[bug#49456] [PATCH] gnu: add environment-modules

["Ludovic Courtès" <ludo@gnu.org>]

Hello,

Nice!  Overall LGTM, modulo cosmetic issues described below.

Ivan Gankevich <i.gankevich@spbu.ru> skribis:

> +++ b/gnu/packages/parallel.scm

How about ‘package-management.scm’ instead?

You probably need to add a copyright line for you too.

> +(define-public environment-modules
> +  (package
> +    (name "environment-modules")

Should the package name be “modules”, since that’s the name that
upstream seems to be using?

> +          (add-before 'configure 'patch-scripts-for-python-3
> +            (lambda _
> +              ;; patch the script for python-3

Nitpick: please capitalize sentences and end with a period.

> +              (substitute* "script/createmodule.py.in"
> +                (("pathkeys.sort\\(\\)") "pathkeys = sorted(pathkeys)")
> +                (("print\\(\"\\\\t\"\\*") "print(\"\\t\"*int")
> +                (("@PYTHON@") (which "python3")))
> +              #t))

You can omit the trailing #t (here and elsewhere).

> +          (add-after 'configure 'patch-/bin/sh-in-tests
> +            (lambda _
> +              (for-each
> +                (lambda (file)
> +                  (substitute* file
> +                    (("/bin/sh") (which "bash"))
> +                    ;; For some reason "kvm" group cannot be resolved for
> +                    ;; "nixbld" user. We remove "-n" switch here to not
> +                    ;; resolve the groups at all.
> +                    (("exec id -G -n -z") "exec id -G -z")
> +                    (("exec id -G -n") "exec id -G")

Is this change made for tests?  In the build environment, the build user
is potentially in the “kvm” group if it exists, but indeed, /etc/group
lacks “kvm” (see nix/libstore/build.cc:1777).

Should a post-check phase reinstate ‘-n’?

> +                    ))

Consider moving the parents on the previous line, as ‘guix lint’
suggests.  :-)

> +    (synopsis "Shell environment variables and aliases management")
> +    (description "A tool that simplify shell initialization and lets users
> +easily modify their environment during the session with modulefiles.")

Please write full sentences for the description.

Could you send an updated patch?

Bonus points if you can provide a commit log that follows our
conventions.  :-)

  https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html

Thanks!

Ludo’.