Subject: Re: [bug#49934] [PATCH 1/3] gnu: Add libx86emu

From: "pelzflorian (Florian Pelz)" <[email-masked]>
Date: Mon, 9 Aug 2021 19:06:28 +0200

On Mon, Aug 09, 2021 at 12:00:00PM +0000, phodina wrote:
> Thanks. I didn't know there's a patch already. The aim of this one is to get the hwinfo and hw-prober to get the hardware details.

I think it is good to get your patch into shape and merged.  While I
haven’t been involved in package review (shame on me), I’ll give it a
try.

You should, after applying each of your patches, cd to the guix repo
and run `./etc/indent-code.el gnu/packages/hardware.scm` as described
in `info '(guix)Formatting Code'`.

`./pre-inst-env guix lint libx86emu` warns that your custom check
phase does not consider #:tests?  (i.e. it does not work with the
--without-tests transform option).

As for the package libx86emu proper:

> (define-public libx86emu
>   (package
>     (name "libx86emu")
>     (version "3.1")
>     (source (origin
>               (method git-fetch)
>               (uri (git-reference
>                     (url "https://github.com/wfeldt/libx86emu")
>                     (commit version)))
>               (file-name (git-file-name name version))
>               (sha256
>                (base32
>                 "104xqc6nj9rpi7knl3dfqvasf087hlz2n5yndb1iycw35a6j509b"))))
>     (build-system gnu-build-system)
>     (arguments
>      `(#:make-flags (list
>                      (string-append "CC=" ,(cc-for-target))
>                      (string-append "DESTDIR=" %output)
>                      (string-append "LIBDIR=/lib"))
>        #:phases (modify-phases %standard-phases
>                   (delete 'configure)
>                   (add-after 'unpack 'fix-version-and-usr
>                     (lambda* (#:key inputs #:allow-other-keys)
>                       (delete-file "git2log")

I am not sure, would it be better to delete this git2log in an origin
snippet?  I have not checked.

>                       (let* ((file (open-file "VERSION" "a")))
>                         (display ,version file)
>                         (close-port file))
>                       (substitute* "Makefile"
>                         (("/usr") "/"))))
>                   (replace 'check
>                     (lambda* _
>                       (invoke "make" "test"))))))
>     (native-inputs `(("git" ,git) ("perl" ,perl) ("nasm" ,nasm)))
>     (synopsis "Library for x86 emulation")
>     (description "Small library to emulate x86 instructions.  The focus here
> is not a complete emulation (go for qemu for this) but to cover enough for
> typical firmware blobs.

The start of the sentence is missing before @enumerate.  “You can”?

> @enumerate
> @item intercept any memory access or directly map real memory ranges
> @item intercept any i/o access, map real i/o ports, or block any real i/o
> @item intercept any interrupt
> @item provides hook to run after each instruction

Either “provides a hook” or “provides hooks”.

> @item recognizes a special x86 instruction that can trigger logging

s/recognizes/recognize/

> @item has integrated logging

Maybe s/has/use/

> @end enumerate")
>     (home-page "https://github.com/wfeldt/libx86emu")
>     (license license:bsd-1)))

The license is not bsd-1 but x11-style, I think.

Regards,
Florian