[PATCH] gnu: add environment-modules

  • Done
  • quality assurance status badge
Details
2 participants
  • Ivan Gankevich
  • Ludovic Courtès
Owner
unassigned
Submitted by
Ivan Gankevich
Severity
normal

Debbugs page

Ivan Gankevich wrote 4 years ago
(address . guix-patches@gnu.org)(name . Ivan Gankevich)(address . i.gankevich@spbu.ru)
20210707085932.20751-1-i.gankevich@spbu.ru
---
gnu/packages/parallel.scm | 64 +++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)

Toggle diff (83 lines)
diff --git a/gnu/packages/parallel.scm b/gnu/packages/parallel.scm
index 42826f49d6..f07ce02d33 100644
--- a/gnu/packages/parallel.scm
+++ b/gnu/packages/parallel.scm
@@ -41,8 +41,10 @@
#:use-module (gnu packages admin)
#:use-module (gnu packages autotools)
#:use-module (gnu packages base)
+ #:use-module (gnu packages dejagnu)
#:use-module (gnu packages flex)
#:use-module (gnu packages freeipmi)
+ #:use-module (gnu packages less)
#:use-module (gnu packages linux)
#:use-module (gnu packages mpi)
#:use-module (gnu packages perl)
@@ -378,3 +380,65 @@ and output captured in the notebook. Whatever arguments are accepted by a
SLURM command line executable are also accepted by the corresponding magic
command---e.g., @code{%salloc}, @code{%sbatch}, etc.")
(license license:bsd-3))))
+
+(define-public environment-modules
+ (package
+ (name "environment-modules")
+ (version "4.7.1")
+ (source
+ (origin
+ (method url-fetch)
+ (uri (string-append "mirror://sourceforge/modules/Modules/modules-"
+ version "/modules-" version ".tar.bz2"))
+ (sha256 (base32 "07r03vqskjxyjy5m2b6p2px42djnd2z1k4b5j9dxqv8prin01ax6"))))
+ (build-system gnu-build-system)
+ (arguments
+ `(#:configure-flags
+ (list (string-append "--with-bin-search-path="
+ (assoc-ref %build-inputs "tcl") "/bin" ":"
+ (assoc-ref %build-inputs "procps") "/bin" ":"
+ (assoc-ref %build-inputs "less") "/bin" ":"
+ (assoc-ref %build-inputs "coreutils") "/bin")
+ (string-append "--with-tcl=" (assoc-ref %build-inputs "tcl") "/lib")
+ "--disable-compat-version")
+ #:test-target "test"
+ #:phases
+ (modify-phases %standard-phases
+ (add-before 'configure 'patch-scripts-for-python-3
+ (lambda _
+ ;; patch the script for python-3
+ (substitute* "script/createmodule.py.in"
+ (("pathkeys.sort\\(\\)") "pathkeys = sorted(pathkeys)")
+ (("print\\(\"\\\\t\"\\*") "print(\"\\t\"*int")
+ (("@PYTHON@") (which "python3")))
+ #t))
+ (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")
+ ))
+ '("testsuite/modules.00-init/005-init_ts.exp"
+ "testsuite/install.00-init/005-init_ts.exp"))
+ #t)))))
+ (native-inputs
+ `(("dejagnu" ,dejagnu)
+ ("autoconf" ,autoconf)
+ ("which" ,which)))
+ (inputs
+ `(("tcl" ,tcl)
+ ("less" ,less)
+ ("procps" ,procps)
+ ("coreutils" ,coreutils)
+ ("python" ,python-3)))
+ (home-page "http://modules.sourceforge.net/")
+ (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.")
+ (license license:gpl2+)))
--
2.32.0
Ludovic Courtès wrote 4 years ago
(name . Ivan Gankevich)(address . i.gankevich@spbu.ru)(address . 49456@debbugs.gnu.org)
87o8awk9lc.fsf@gnu.org
Hello,

Nice! Overall LGTM, modulo cosmetic issues described below.

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

Toggle quote (2 lines)
> +++ b/gnu/packages/parallel.scm

How about ‘package-management.scm’ instead?

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

Toggle quote (4 lines)
> +(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?

Toggle quote (4 lines)
> + (add-before 'configure 'patch-scripts-for-python-3
> + (lambda _
> + ;; patch the script for python-3

Nitpick: please capitalize sentences and end with a period.

Toggle quote (6 lines)
> + (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).

Toggle quote (12 lines)
> + (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’?

Toggle quote (2 lines)
> + ))

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

Toggle quote (4 lines)
> + (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. :-)


Thanks!

Ludo’.
Ivan Gankevich wrote 4 years ago
[PATCH] gnu: Add modules.
(address . 49456@debbugs.gnu.org)(name . Ivan Gankevich)(address . i.gankevich@spbu.ru)
20210721122736.17604-1-i.gankevich@spbu.ru
* gnu/packages/package-management.scm (modules): New variable.
---
gnu/packages/package-management.scm | 109 ++++++++++++++++++++++++++++
1 file changed, 109 insertions(+)

Toggle diff (147 lines)
diff --git a/gnu/packages/package-management.scm b/gnu/packages/package-management.scm
index c2c7846630..d3dc8e593b 100644
--- a/gnu/packages/package-management.scm
+++ b/gnu/packages/package-management.scm
@@ -17,6 +17,7 @@
;;; Copyright © 2020 Jesse Gibbons <jgibbons2357+guix@gmail.com>
;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
;;; Copyright © 2020 Vincent Legoll <vincent.legoll@gmail.com>
+;;; Copyright © 2021 Ivan Gankevich <i.gankevich@spbu.ru>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -52,6 +53,7 @@
#:use-module (gnu packages crypto)
#:use-module (gnu packages curl)
#:use-module (gnu packages databases)
+ #:use-module (gnu packages dejagnu)
#:use-module (gnu packages dbm)
#:use-module (gnu packages docbook)
#:use-module (gnu packages file)
@@ -64,6 +66,7 @@
#:use-module (gnu packages guile)
#:use-module (gnu packages guile-xyz)
#:use-module (gnu packages hurd)
+ #:use-module (gnu packages less)
#:use-module (gnu packages libedit)
#:use-module (gnu packages linux)
#:use-module (gnu packages lisp)
@@ -83,6 +86,7 @@
#:use-module (gnu packages serialization)
#:use-module (gnu packages sqlite)
#:use-module (gnu packages ssh)
+ #:use-module (gnu packages tcl)
#:use-module (gnu packages texinfo)
#:use-module (gnu packages time)
#:use-module (gnu packages tls)
@@ -1494,3 +1498,108 @@ It is mainly meant for programmers who develop portable programs or libraries in
but could potentially work for end-users of those programs. It also has a translator
from R7RS, which allows most R7RS code to run on R6RS implementations.")
(license license:gpl3+)))
+
+(define-public modules
+ (package
+ (name "modules")
+ (version "4.8.0")
+ (source
+ (origin
+ (method url-fetch)
+ (uri (string-append "mirror://sourceforge/modules/Modules/modules-"
+ version "/modules-" version ".tar.bz2"))
+ (sha256 (base32 "1amz8qdqbvfdc8jv0j4720vywbz2gi7l3sr1lh37ilfbxy9lq9g9"))))
+ (build-system gnu-build-system)
+ (arguments
+ `(#:configure-flags
+ (list (string-append "--with-bin-search-path="
+ (assoc-ref %build-inputs "tcl") "/bin" ":"
+ (assoc-ref %build-inputs "procps") "/bin" ":"
+ (assoc-ref %build-inputs "less") "/bin" ":"
+ (assoc-ref %build-inputs "coreutils") "/bin")
+ (string-append "--with-tcl=" (assoc-ref %build-inputs "tcl") "/lib")
+ "--disable-compat-version")
+ #:test-target "test"
+ #:phases
+ (modify-phases %standard-phases
+ (add-before 'configure 'patch-scripts-for-python-3
+ (lambda _
+ ;; Patch the script for python-3.
+ (substitute* "script/createmodule.py.in"
+ (("pathkeys.sort\\(\\)") "pathkeys = sorted(pathkeys)")
+ (("print\\(\"\\\\t\"\\*") "print(\"\\t\"*int")
+ (("@PYTHON@") (which "python3")))))
+ (add-before 'check 'patch-/bin/sh-and-nixbld-groups-in-tests
+ (lambda _
+ (use-modules (srfi srfi-1))
+ (let* ((groups-file (string-append (getcwd) "/nixbld-groups"))
+ (groups-file-z (string-append groups-file "-z"))
+ (nixbld-groups
+ (fold
+ (lambda (id prev)
+ (catch #t
+ (lambda () (cons (group:name (getgrnam id)) prev))
+ (lambda _ prev)))
+ '()
+ (vector->list (getgroups)))))
+ ;; Simulate "id -G -n" command output.
+ (call-with-output-file groups-file
+ (lambda (port)
+ (display (string-join nixbld-groups " ") port)
+ (display #\newline port)))
+ ;; Simulate "id -G -n -z" command output.
+ (call-with-output-file groups-file-z
+ (lambda (port)
+ (for-each
+ (lambda (group-name)
+ (display group-name port)
+ (display #\null port))
+ nixbld-groups)))
+ ;; Generate "modulecmd-test.tcl" before running "make test".
+ (invoke "make" "modulecmd-test.tcl")
+ ;; Substitute shell.
+ (substitute*
+ '("modulecmd-test.tcl"
+ "modulecmd.tcl"
+ "testsuite/modules.70-maint/380-edit.exp"
+ "compat/init/filter")
+ (("/bin/sh") (which "sh")))
+ ;; Skip tests that use supplementary groups.
+ (for-each
+ delete-file
+ '("testsuite/modules.20-locate/112-hide-user-group.exp"
+ "testsuite/modules.20-locate/117-forbid-user-group.exp"
+ "testsuite/modules.20-locate/119-hide-cascading.exp"
+ "testsuite/modules.50-cmds/140-system.exp"
+ "testsuite/modules.50-cmds/287-info-usergroups.exp"
+ "testsuite/modules.50-cmds/440-module-tag.exp"
+ "testsuite/modules.70-maint/220-config.exp"))
+ (for-each
+ (lambda (file)
+ (substitute* file
+ (("/bin/sh") (which "bash"))
+ ;; For some reason "kvm" group cannot be resolved for
+ ;; "nixbld" user. We replace "id ..." commands with
+ ;; "cat ..." that simulates them.
+ (("exec id -G -n -z") (string-append "exec cat " groups-file-z))
+ (("exec id -G -n") (string-append "exec cat " groups-file))))
+ '("testsuite/modules.00-init/005-init_ts.exp"
+ "testsuite/install.00-init/005-init_ts.exp"
+ "modulecmd-test.tcl"))))))))
+ (native-inputs
+ `(("dejagnu" ,dejagnu)
+ ("autoconf" ,autoconf)
+ ("which" ,which)))
+ (inputs
+ `(("tcl" ,tcl)
+ ("less" ,less)
+ ("procps" ,procps)
+ ("coreutils" ,coreutils)
+ ("python" ,python-3)))
+ (home-page "http://modules.sourceforge.net/")
+ (synopsis "Shell environment variables and aliases management")
+ (description "Modules simplify shell initialization and let users
+modify their environment during the session with modulefiles. Modules are
+used on high-performance clusters to dynamically add and remove paths
+to specific versions of applications.")
+ (license license:gpl2+)))
--
2.32.0
Ivan Gankevich wrote 4 years ago
Re: bug#49456: [PATCH] gnu: add environment-modules
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 49456@debbugs.gnu.org)
YPgXNxQVZR+3hLwl@surge
Toggle quote (6 lines)
>> +++ b/gnu/packages/parallel.scm
>
>How about ‘package-management.scm’ instead?
>
>You probably need to add a copyright line for you too.

Moved to ‘package-management.scm’, added copyright line.


Toggle quote (7 lines)
>> +(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?

Renamed to “modules”.


Toggle quote (18 lines)
>> + (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’?

This change is needed for tests only, main programme uses different
configuration.

I have updated to the version 4.8.0 and unfortunately these changes no longer
work (developers replaced calls to “id” with Tcl extensions). Now I disabled
tests that use group information.

Can we add all supplementary groups to /etc/groups? Not adding them to
/etc/group makes some shell commands return an error (“groups”, “id -G -n”).


Toggle quote (7 lines)
>
>> + (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.

Changed description.


Toggle quote (5 lines)
>Could you send an updated patch?
>
>Bonus points if you can provide a commit log that follows our
>conventions. :-)

I’ve sent an updated patch in a separate email. Thank you for the corrections!
Ludovic Courtès wrote 4 years ago
(name . Ivan Gankevich)(address . i.gankevich@spbu.ru)(address . 49456-done@debbugs.gnu.org)
87lf5xax62.fsf_-_@gnu.org
Hi,

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

Toggle quote (2 lines)
> * gnu/packages/package-management.scm (modules): New variable.

Pushed as 8bcd920c71ffa3bcd1f4290e6c252cc59bc9be52. I followed up with
a patch that removes FHS assumptions from the ‘add.modules’ script.

Thanks!

Ludo’.
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 49456@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 49456
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help