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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 49456 in the body.
You can then email your comments to 49456 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ivan Gankevich <i.gankevich <at> spbu.ru>
To: guix-patches <at> gnu.org
Cc: Ivan Gankevich <i.gankevich <at> spbu.ru>
Subject: [PATCH] gnu: add environment-modules
Date: Wed,  7 Jul 2021 11:59:33 +0300

---
 gnu/packages/parallel.scm | 64 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

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

Message #8 received at 49456 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ivan Gankevich <i.gankevich <at> spbu.ru>
Cc: 49456 <at> debbugs.gnu.org
Subject: Re: bug#49456: [PATCH] gnu: add environment-modules
Date: Tue, 20 Jul 2021 22:34:55 +0200

Hello,

Nice!  Overall LGTM, modulo cosmetic issues described below.

Ivan Gankevich <i.gankevich <at> 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’.

Message #11 received at 49456 <at> debbugs.gnu.org (full text, mbox):

From: Ivan Gankevich <i.gankevich <at> spbu.ru>
To: 49456 <at> debbugs.gnu.org
Cc: Ivan Gankevich <i.gankevich <at> spbu.ru>
Subject: [PATCH] gnu: Add modules.
Date: Wed, 21 Jul 2021 15:27:38 +0300

* gnu/packages/package-management.scm (modules): New variable.
---
 gnu/packages/package-management.scm | 109 ++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)

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 <at> gmail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
 ;;; Copyright © 2020 Vincent Legoll <vincent.legoll <at> gmail.com>
+;;; Copyright © 2021 Ivan Gankevich <i.gankevich <at> 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

Message #14 received at 49456 <at> debbugs.gnu.org (full text, mbox):

From: Ivan Gankevich <i.gankevich <at> spbu.ru>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 49456 <at> debbugs.gnu.org
Subject: Re: bug#49456: [PATCH] gnu: add environment-modules
Date: Wed, 21 Jul 2021 15:46:47 +0300

>> +++ 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.


>> +(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”.


>> +          (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”).


>
>> +    (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.


>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!

Message #19 received at 49456-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ivan Gankevich <i.gankevich <at> spbu.ru>
Cc: 49456-done <at> debbugs.gnu.org
Subject: Re: bug#49456: [PATCH] gnu: add environment-modules
Date: Fri, 23 Jul 2021 17:04:53 +0200

Hi,

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

> * 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.