Subject: Re: [bug#69131] [PATCH 1/2] gnu: Add passt.

From: Skyler Ferris <[email-masked]>
Date: Fri, 16 Feb 2024 03:02:43 +0000

"Add passt" review notes:

The following checklist is based on the items listed in the manual (22.7 
"Submitting Patches"). Each checklist item may have one of the following 
statuses:

C: Commitable. This item looks as expected for a final commit.
CC: Commitable with Context. Like commitable, but some notes have been 
added providing context about anything notable or unusual. These notes 
are provided for transparency or to set expectations for the committer 
and should not be interpreted as meaning that I object to the patch.
MR: Minor Repair. There is something about the patch which I think 
should ideally be changed, but I do not think that the commit should be 
held up for this reason alone.
A: Attention. Something in the patch needs attention from the author, 
the committer, or both. Resolving the matter may or may not require a 
change to the patch.
Blank: I did not review this aspect of the patch.

[CC] Applies cleanly (commit bd87416648929f38c0173f047776d7675ea8a10d)
[CC] Cryptographic hash
[C ] Synopsis & Description
[C ] Package license matches source license
[C ] No compiler warnings
[CC] `guix lint` output
[MR] `guix style` output
[C ] Package builds (my host is an x86_64 Xen guest)
[  ] Builds for other architectures
[C ] No bundled software
[  ] Works in container with minimal inputs
[C ] `guix size` output
[C ] Dependent packages (`guix refresh --list-dependent PACKAGE`)
[C ] `guix build --rounds=2`
[C ] Gender-neutral wording
[C ] One set of related changes
[  ] Simulated guix pull

# Patch Application
The copyright statement in the first patch does not apply cleanly 
because commit 6002e9e7711136373de5a6325769e9c7d11032c9 recently added 
it already, but git successfully processed that diff after manually 
deleting the chunk from the patch.

# Cryptographic Signatures
I was not able to locate signatures for either the source tarball or the 
git commits.

# Guix Lint Output
The linter printed two notes, one that the 'generic-html' updater failed 
to find upstream releases and another indicated that archives are not 
available from either Software Heritage or Disarchive. As I understand 
it, these issues issues should not block the package. Not every package 
has an automatic updater (though it would be nice if they did) and the 
release page for this project is atypical. The 'archival' section in the 
"Invoking 'guix lint'" chapter (9.8) indicates that the Software 
Heritage will pick up the source at some point in the future.

# Guix Style output (Minor repair)
The tool made some reasonable-looking changes. I'm not sure if it would 
be appropriate for me to add a new patch to this thread that applies 
these changes.

# Runs in a container with minimal inputs
This package wants to use QEMU, and my machine is already a Xen guest. I 
know that there are some complications when it comes to nested 
virtualization. The demo script (at #$output/share/doc/passt/demo.sh) 
did not "just work" for me, but it's not clear to me that this is a 
problem with the package. I tried running it with the following:

```
$ # Extra inputs are for the shell script
$ ./pre-inst-env guix shell -C iproute2 coreutils util-linux-with-udev 
passd
[env]$ 
/gnu/store/wgzlv2chrxmk4y4m4fxx8g7x0z59cb98-passt-2023_12_30.f091893/share/doc/passt/demo.sh
```