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

[Leo Famulari <leo@famulari.name>]

On Sat, Mar 28, 2020 at 01:50:52AM +0100, Jakub Kądziołka wrote:
> * gnu/packages/emulators.scm (unicorn-next): New variable.

Sounds like a cool package!

> 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.

> 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? We normally don't package betas or
release candidates... it depends. Do you have an idea of the release
timeline? Do you think upstream would mind if we packaged 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 main library is not written in Python, but the build process has
> +    ;; little in common with any defined build system, so we might as well
> +    ;; build on top of python-build-system and make use of all
> +    ;; the Python-specific phases that can be reused.

Okay

> +         (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...

> +         (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.