[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#49456] [PATCH] gnu: add environment-modules
From: |
Ludovic Courtès |
Subject: |
[bug#49456] [PATCH] gnu: add environment-modules |
Date: |
Tue, 20 Jul 2021 22:34:55 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
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’.