[bug#40267] [PATCH 1/2] gnu: Add unicorn.

["Jakub Kądziołka" <kuba@kadziolka.net>]

[-- Attachment #1: Type: text/plain, Size: 3225 bytes --]

On Sat, Mar 28, 2020 at 11:48:11PM -0400, Leo Famulari wrote:
> On Sat, Mar 28, 2020 at 01:50:52AM +0100, Jakub Kądziołka wrote:
> > If I package a -rc version, should it have a -next suffix in its name
> > even though the "stable" version isn't packaged?
> 
> I think it's best to just call it unicorn. The version says -rc and we
> mention it in the synopsis and description. And it's useful under the
> hood for guix lint to match the upstream name.

Fair enough.

> > Maybe I should also package the non-rc unicorn? The test suite for that
> > version fails to compile, so it's not entirely trivial.
> 
> Is the previous release useful?

As far as I am aware, the non-rc release is not useful beyond avoiding
any potential uneasyness about running -rc releases ;)

> We normally don't package betas or
> release candidates... it depends. Do you have an idea of the release
> timeline?

Sadly, I have no idea.

> Do you think upstream would mind if we packaged the RC?

I don't think so? As a datapoint, FreeBSD packages the -rc.

> > +    ;; NOTE: unicorn ships a bundled QEMU, but with custom modifications.
> 
> Can you add more detail to this comment? Is it just a patch on a QEMU
> tarball or is this not really QEMU anymore?

The documentation suggests the changes go quite deep:

| Internally, Unicorn reuses the CPU emulation component of QEMU as its
| core (with quite a lot of changes to adapt to our design).

What do you think about a comment like this?

;; NOTE: unicorn ships a bundled QEMU, but heavily modified.

> > +         (add-after 'unpack 'install-bindings-to-python-output
> > +           (lambda* (#:key outputs #:allow-other-keys)
> > +             ;; python-build-system will build the bindings and install them to
> > +             ;; the "out" output, so change the build-internal names of the
> > +             ;; outputs.
> > +             (set-car! (assoc "out" outputs) "lib")
> > +             (set-car! (assoc "python" outputs) "out")
> > +             #t))
> 
> I would wait for advice here. The manual requests we write everything in
> a functional style. But I don't know of another way to make
> python-build-system install things to alternate outputs without changing
> the build system or replacing the install phase. It would be nice to
> have a parameter for this somewhere...

Yeah, it's not the nicest thing. I think I'll submit a patch to c-u that
would add such a parameter (does #:python-output sound good?), and then come
back here when it lands. Would this imperative hack be ok as a temporary solution?

> > +         (add-before 'check 'check-library
> > +           (lambda* (#:key outputs #:allow-other-keys)
> > +             ;; TODO: running the tests on non-x86 requires a cross-binutils
> > +             ;; with x86 as target.
> > +             ,@(if (member (%current-system) '("x86_64-linux" "i686-linux"))
> 
> I think the 'when' procedure is more clear than 'if' in cases where the
> else branch is empty.

The issue is that the else branch contains '(), and is not itself empty.
Do you happen to know how to get the value of (%current-system)
build-side?

Thanks for your review!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]