[PATCH] gnu: reprotest: Add missing propogated inputs

Hello,

This patch adds some propagated inputs to the reprotest package so that

most tests can run in a pure shell. I observed some issues running this

package in a pure shell when reviewing issue 69991 (which updates

diffoscope). Some of the new propagated inputs have already been listed

as native inputs, but I did not remove them under the assumption that

these programs are actually needed for the build machine. There are 3

tests which still do not work in a pure shell.

The fileordering and user_group tests both require setuid programs. Both

require fusermount and user_group requires sudo. I do not think it is

possible to make these tests work within a pure shell because a package

cannot provide setuid programs (only an operating system can).

The domain_host test does not work in a pure shell due to a dependency

on a program named domainname. I was unable to locate a package which

provides this command. The closest I could find is the inetutils package

which provides dnsdomainname but I do not think this is sufficient for

the test to run correctly. The domainname command is used in

reprotest/build.py line 243:

```python

_ = _.prepend_to_build_command(*"unshare -r --uts".split(),

    "sh", "-ec", r"""

    hostname {1}

    domainname "{2}"

    """.format(build.aux_tree, hostname, domainname) + '"$@"', "-")

```

It looks to me like this is trying to set both the hostname and the

domainname, which makes sense if it wants to test for these settings as

a cause of non-determinism. The hostname command supports setting, but

the dnsdomainname command only supports retrieving the domain name.

Additionally, this patch adds a new build phase to call fusermount3

instead of fusermount, which is the name of the command provided by

Guix's fuse package. This does not make it work in a pure shell but it

does make it work in an operating system that provides the suid version

of the binary.

Summary:

Tests working in pure shell:

- aslr

- build_path

- environment

- exec_path

- home

- kernel

- locales

- num_cpus

- time

- timezone

- umask

Tests work with appropriate suid programs provided:

- fileordering

- user_group

Tests that do not work on any Guix system:

- domain_host

To try reprotest in a pure shell, create the following makefile in an

empty directory:

```make

destination:

    echo foo > destination

clean:

    rm -f destination

```

Then, run the following command to try the working tests. If you want to

see the test report a problem you will need to change the recipe to be

non-deterministic. For example, use `date` instead of `echo foo` in

order to see the time test report a problem. Note that make is an

additional package in the shell because this is the build system that

happens to be used, but it would not make sense to add this as a

mandatory input to the package because some people might use reprotest

without using make. If you remove one of the propagated inputs then at

least one of these tests will crash, with the exception of findutils and

diffoscope which are used by reprotest outside the context of a specific

test (to get the hashes of multiple files and display diffs to the user,

respectively).

```shell

guix shell --pure reprotest make -- bash -c 'cd /tmp/test && reprotest

"make clean && make" destination --variations="+environment,

+build_path, +kernel, +aslr, +num_cpus, +time, +home, +locales,

+exec_path, +timezone, +umask"'

```

You can try the suid-dependent tests with the same Makefile. The sudo

and fuse-2 packages need to be installed; the default %setuid-programs

variable includes the relevant binaries. Remove the `--pure` flag from

the shell invocation and change the `--variations` flag in the reprotest

variations to have the value `"+fileordering, +user_group"`.

Regards,

Skyler

diffoscope, disorderfs, findutils, grep, inetutils, libfaketime,

and util-linux as propagated inputs.

Change-Id: Ibc113558032697be9048bebe54ba7678156667b9

---

gnu/packages/diffoscope.scm | 14 ++++++++++++++

1 file changed, 14 insertions(+)

On 2024-03-25, Skyler Ferris wrote:

Thanks for taking an interest in this!

My understanding is that propagated inputs are generally to be used very

sparingly, as they make it very difficult to install multiple packages

that might contain conflicting propagated inputs.

Most of the proposed inputs are largely optional dependencies of

reprotest, which in guix terms, generally the end-user is expected to

install the additional packages to enable desired optional features.

It might make sense to list the additional dependencies in the

description describing what features each enables?

Adding some of them to native-inputs might also make sense to enable

more tests, and give a clear place to write comments about each. Then

"guix shell --pure --development reprotest reprotest" would pull them in

and reprotest itself to be able to run the relevent tests as you had

tried.

If reprotest behaves quite badly without some of the in the working

environment, such as spitting out an ugly backtrace rather than

reporting what is missing, it should probably be fixed upstream (and

possibly patched in guix in the meantime).

It seems like this is missing from the patch, though I suspect it is

needed to behave correctly with things relying on fuse!

Thanks for the detailed report ... this might be or should be covered by

upstream documentation ... although if there are guix-specific angles on

it, I am not sure where that should be documented.

So, in summary, I think there are some ideas to explore from your patch,

although it needs a bit of rethinking about implementation details.

live well,

vagrant

Hi Vagrant,

Thanks for the feedback!

On 3/25/24 23:02, Vagrant Cascadian wrote:

Fair enough. I haven't personally run into this pain point, but that's probably because people are using them sparingly. =)

I remember running into this before and it has been a pain point for me because it is not clear what the optional dependencies are. I know that package managers like apt have the ability to recommend packages which are optional dependencies, but the problem that I run into is that if I'm looking at a configuration (manifest, operating-system spec, etc) it's not clear to me which items are included as "invisible dependencies" of other packages and which ones are being included for their own sake. So I would like to propose using a different technique which I believe addresses both of our concerns. Basically, the technique is to embed absolute paths into the python code. For example, I tried adding this phase to reprotest (it would need some polishing before actual submission, for reasons mentioned later in this email): ```scheme (add-after 'adjust-locales 'set-taskset-path (lambda _ (define (quote-surround str) (string-append "'" str "'")) (let ((util-linux #$(this-package-input "util-linux"))) (when util-linux (substitute* "reprotest/build.py" (("'taskset'") (quote-surround (string-append util-linux "/bin/taskset"))) (("setarch") (string-append util-linux "/bin/setarch"))))))) ``` This let me run the num_cpus test from a pure shell without adding util-linux to my explicit inputs. Running taskset from the shell resulted in a "command not found" error so the package was not added to the profile. Additionally, I built a `guix pack` with only reprotest as an input, and util-linux was present. I built a pack with the current version of the package and util-linux was not present. So it seems that even though Guix sees the hash and is aware of it as a run-time dependency, it does not automatically add the package to the profile, addressing both of our concerns. If a user does not want an optional dependency then they can delete the input like so (or replace it for a different version, if so desired): ```scheme (use-modules (guix packages) (gnu packages diffoscope)) (package (inherit reprotest) (inputs (modify-inputs (package-inputs reprotest) (delete "util-linux")))) ``` Unfortunately I do not see a way to remove an input using the with-input option in options->transformation procedure or the analogous --with-input command-line flag. If we do go this route then I will look at adding "#f" as a valid value for the with-input transformation to support this case, but that will be handled in a separate issue. We could default to not including optional dependencies, but I don't think that this would be the most useful thing to do. I would expect that most people want all tests to work and that removing the inputs is a niche interest. It seems likely that "all of the tests work out of the box" is closer to what most people are currently experiencing because all of the packages which I added to the propagated inputs, with the exception of libfaketime and diffoscope, is in the %base-packages variable that Guix recommends people use (and the "normal" diff command is provided by %base-packages as well, so the lack of diffoscope would not cause a problem for users with the default configuration). If we do decide to exclude optional dependencies by default then I will look at adding an "add-input" option to the available transformations.

Doing this for every command used by reprotest would require some up-front work, and I'm not sure if it would be possible to uniquely identify every instance of the program names with regular expressions alone (for example, the above build phase replaces instances of setarch in docstrings which is undesirable). So I might end up adding a guix-specific patch that does something like surround each instance of the program name with double percent signs so that they are uniquely identifiable in the build phase. This might add some additional work when updating the package, because new lines that add commands would need to have the double percent signs added and changes to existing lines could cause conflicts with the patch. But I think that the majority of the work would be the up-front work which I am willing to do.

I think this is a useful addition even with the above technique, so that users have awareness about the optional dependencies & how they can manipulate them.

I agree that this should be part of the revision even with the above technique, because if a user chooses to exclude an optional dependency without realizing a desired test depends on it then they will see the backtrace instead of a clean explanation (there is an explanation at the very beginning of the input, but it is buried by the backtrace). Additionally, the setuid packages need to be handled by the operating-system declaration no matter what we do here.

Oops, I meant to delete that paragraph. I wrote it before finding out that there are separate fuse-2 and fuse packages. The fuse-2 package provides a binary named "fusermount", and reprotest used it successfully on my machine without this additional substitution.

Assuming that we go with the above technique, I think that it would make sense to mention the ability to use package transformation options to control optional dependencies in the description of the reprotest package. In either case I think that the setuid requirements should be added to the description.

I appreciate any feedback you have!

Regards,

Skyler

Hi again, sorry, it seems like some of my paragraphs and code blocks got

squashed into one giant paragraph somehow. I'm going to try this again,

hopefully it works this time. This is the text beneath the "Most of the

proposed inputs..." quote:

I remember running into this before and it has been a pain point for me

because it is not clear what the optional dependencies are. I know that

package managers like apt have the ability to recommend packages which

are optional dependencies, but the problem that I run into is that if

I'm looking at a configuration (manifest, operating-system spec, etc)

it's not clear to me which items are included as "invisible

dependencies" of other packages and which ones are being included for

their own sake. So I would like to propose using a different technique

which I believe addresses both of our concerns.

Basically, the technique is to embed absolute paths into the python

code. For example, I tried adding this phase to reprotest (it would need

some polishing before actual submission, for reasons mentioned later in

this email):

```scheme

(add-after 'adjust-locales 'set-taskset-path

  (lambda _

    (define (quote-surround str)

      (string-append "'" str "'"))

    (let ((util-linux #$(this-package-input "util-linux")))

      (when util-linux

        (substitute* "reprotest/build.py"

          (("'taskset'")

           (quote-surround (string-append util-linux "/bin/taskset")))

          (("setarch")

           (string-append util-linux "/bin/setarch")))))))

```

This let me run the num_cpus test from a pure shell without adding

util-linux to my explicit inputs. Running taskset from the shell

resulted in a "command not found" error so the package was not added to

the profile. Additionally, I built a `guix pack` with only reprotest as

an input, and util-linux was present. I built a pack with the current

version of the package and util-linux was not present. So it seems that

even though Guix sees the hash and is aware of it as a run-time

dependency, it does not automatically add the package to the profile,

addressing both of our concerns.

If a user does not want an optional dependency then they can delete the

input like so (or replace it for a different version, if so desired):

```scheme

(use-modules (guix packages) (gnu packages diffoscope))

(package

    (inherit reprotest)

    (inputs (modify-inputs (package-inputs reprotest)

                           (delete "util-linux"))))

```

Unfortunately I do not see a way to remove an input using the with-input

option in options->transformation procedure or the analogous

--with-input command-line flag. If we do go this route then I will look

at adding "#f" as a valid value for the with-input transformation to

support this case, but that will be handled in a separate issue.

We could default to not including optional dependencies, but I don't

think that this would be the most useful thing to do. I would expect

that most people want all tests to work and that removing the inputs is

a niche interest. It seems likely that "all of the tests work out of the

box" is closer to what most people are currently experiencing because

all of the packages which I added to the propagated inputs, with the

exception of libfaketime and diffoscope, is in the %base-packages

variable that Guix recommends people use (and the "normal" diff command

is provided by %base-packages as well, so the lack of diffoscope would

not cause a problem for users with the default configuration). If we do

decide to exclude optional dependencies by default then I will look at

adding an "add-input" option to the available transformations.