GNU bug report logs - #28660
[PATCH] gnu: python-numpy: Give sh store location instead of setting $SHELL.

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 28660 in the body.
You can then email your comments to 28660 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: Thomas Danckaert <post <at> thomasdanckaert.be>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: python-numpy: Give sh store location instead of setting
 $SHELL.
Date: Sun, 01 Oct 2017 14:44:04 +0200 (CEST)

[Message part 1 (text/plain, inline)]

Hi Guix,

I slightly botched the last numpy update: setting $SHELL works, but 
then we have to set $SHELL for every package which uses numpy's 
distutils.  I don't know how many packages there are, but it includes 
scipy and python-hdf (and therefore those builds are currently broken 
on master...).

The attached patch should fix the issue for all packages, by setting 
the default shell to the “/bin/sh” of the build environment's bash 
package (i.e. bash-minimal).

Thomas

[0001-gnu-python-numpy-Give-sh-store-location-instead-of-s.patch (text/x-patch, inline)]

From cfaebf1b09c71585b5513629005e7cf3c5d17508 Mon Sep 17 00:00:00 2001
From: Thomas Danckaert <thomas.danckaert <at> gmail.com>
Date: Sun, 1 Oct 2017 14:32:04 +0200
Subject: [PATCH] gnu: python-numpy: Give sh store location instead of setting
 $SHELL.

* gnu/packages/python.scm (python-numpy): [arguments] Don't set $SHELL in the
  environment, but embed the location of bash as a default shell.  Otherwise,
  we have to set $SHELL for every package which uses numpy's distutils.
---
 gnu/packages/python.scm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm
index e95c22de1..267715b0f 100644
--- a/gnu/packages/python.scm
+++ b/gnu/packages/python.scm
@@ -3742,8 +3742,6 @@ between language specification and implementation aspects.")
        (modify-phases %standard-phases
         (add-before 'build 'set-environment-variables
          (lambda* (#:key inputs #:allow-other-keys)
-           ;; numpy's distutils uses $SHELL to run external commands.
-          (setenv "SHELL" "bash")
           (call-with-output-file "site.cfg"
             (lambda (port)
               (format port
@@ -3762,6 +3760,10 @@ include_dirs = ~a/include
                       (assoc-ref inputs "openblas")
                       (assoc-ref inputs "lapack")
                       (assoc-ref inputs "lapack"))))
+          ;; Insert bash store location for default shell /bin/sh.
+          (substitute* "numpy/distutils/exec_command.py"
+            (("(os.environ.get\\('SHELL', ')(/bin/sh'\\))" match match-start match-end)
+            (string-append match-start (assoc-ref inputs "bash") match-end)))
           ;; Use "gcc" executable, not "cc".
           (substitute* "numpy/distutils/system_info.py"
             (("c = distutils\\.ccompiler\\.new_compiler\\(\\)")
-- 
2.14.1

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

From: Christopher Baines <mail <at> cbaines.net>
To: Thomas Danckaert <post <at> thomasdanckaert.be>
Cc: 28660 <at> debbugs.gnu.org
Subject: Re: [bug#28660] [PATCH] gnu: python-numpy: Give sh store location
 instead of setting $SHELL.
Date: Wed, 4 Oct 2017 07:30:48 +0100

[Message part 1 (text/plain, inline)]

On Sun, 01 Oct 2017 14:44:04 +0200 (CEST)
Thomas Danckaert <post <at> thomasdanckaert.be> wrote:

> Hi Guix,
> 
> I slightly botched the last numpy update: setting $SHELL works, but 
> then we have to set $SHELL for every package which uses numpy's 
> distutils.  I don't know how many packages there are, but it includes 
> scipy and python-hdf (and therefore those builds are currently broken 
> on master...).
> 
> The attached patch should fix the issue for all packages, by setting 
> the default shell to the “/bin/sh” of the build environment's bash 
> package (i.e. bash-minimal).

Could bash be specified as an input, and that used instead? This would
mean that the behaviour of the package is more isolated from the
environment.

For example, on non GuixSD systems, /bin/sh could be something other
than bash. But, if the numpy package depends on some bash in the store,
and uses that, then it would be isolated from this.

What do you think?

[Message part 2 (application/pgp-signature, inline)]

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

From: Thomas Danckaert <post <at> thomasdanckaert.be>
To: mail <at> cbaines.net
Cc: 28660 <at> debbugs.gnu.org
Subject: Re: [bug#28660] [PATCH] gnu: python-numpy: Give sh store location
 instead of setting $SHELL.
Date: Wed, 04 Oct 2017 09:04:36 +0200 (CEST)

From: Christopher Baines <mail <at> cbaines.net>
Subject: Re: [bug#28660] [PATCH] gnu: python-numpy: Give sh store 
location instead of setting $SHELL.
Date: Wed, 4 Oct 2017 07:30:48 +0100

> Could bash be specified as an input, and that used instead? This 
> would
> mean that the behaviour of the package is more isolated from the
> environment.

Sounds good, but I'm afraid I don't understand what you mean (maybe 
it's still too early in the morning).  If we add a "bash" to the 
package inputs, it will still come from the store.  So probably you 
mean something else by “specified as an input”?

> For example, on non GuixSD systems, /bin/sh could be something other
> than bash. But, if the numpy package depends on some bash in the 
> store,
> and uses that, then it would be isolated from this.

That's true, though users on such systems could set $SHELL to 
override the default one from the store.  But maybe avoiding having 
to do that is what you mean by “more isolated from the environment”?

The shell is used for very limited purposes anyway (essentially to 
run C and Fortran compilers to build extenstions, AFAIU), but I'm 
happy to improve the patch if it's useful.  If you have a solution 
that allows users to use their own /bin/sh if they want, but still 
works for all builds of dependent packages on Guix, please explain :-)

Thomas

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

From: Christopher Baines <mail <at> cbaines.net>
To: Thomas Danckaert <post <at> thomasdanckaert.be>
Cc: 28660 <at> debbugs.gnu.org
Subject: Re: [bug#28660] [PATCH] gnu: python-numpy: Give sh store location
 instead of setting $SHELL.
Date: Wed, 4 Oct 2017 08:49:44 +0100

[Message part 1 (text/plain, inline)]

On Wed, 04 Oct 2017 09:04:36 +0200 (CEST)
Thomas Danckaert <post <at> thomasdanckaert.be> wrote:

> From: Christopher Baines <mail <at> cbaines.net>
> Subject: Re: [bug#28660] [PATCH] gnu: python-numpy: Give sh store 
> location instead of setting $SHELL.
> Date: Wed, 4 Oct 2017 07:30:48 +0100
> 
> > Could bash be specified as an input, and that used instead? This 
> > would
> > mean that the behaviour of the package is more isolated from the
> > environment.  
> 
> Sounds good, but I'm afraid I don't understand what you mean (maybe 
> it's still too early in the morning).  If we add a "bash" to the 
> package inputs, it will still come from the store.  So probably you 
> mean something else by “specified as an input”?
> 
> > For example, on non GuixSD systems, /bin/sh could be something other
> > than bash. But, if the numpy package depends on some bash in the 
> > store,
> > and uses that, then it would be isolated from this.  
> 
> That's true, though users on such systems could set $SHELL to 
> override the default one from the store.  But maybe avoiding having 
> to do that is what you mean by “more isolated from the environment”?
> 
> The shell is used for very limited purposes anyway (essentially to 
> run C and Fortran compilers to build extenstions, AFAIU), but I'm 
> happy to improve the patch if it's useful.  If you have a solution 
> that allows users to use their own /bin/sh if they want, but still 
> works for all builds of dependent packages on Guix, please explain :-)

Sorry Thomas, ignore what I originally said. I thought from reading the
patch that this was patching numpy to use /bin/sh . Now after building
it and looking at the resulting files, I can see that it makes the
default use bash from the store [1]. Which was exactly what I was
suggesting, but you were already doing that.

With this new, hopefully more correct interpretation, this patch looks
fine to me :)

1: 
sh = os.environ.get('SHELL', '/gnu/store/kpxi8h3669afr9r1bgvaf9ij3y4wdyyn-bash-minimal-4.4.12/bin/sh')

[Message part 2 (application/pgp-signature, inline)]

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

From: Thomas Danckaert <post <at> thomasdanckaert.be>
To: mail <at> cbaines.net, 28660-done <at> debbugs.gnu.org
Cc: 28660 <at> debbugs.gnu.org
Subject: Re: [bug#28660] [PATCH] gnu: python-numpy: Give sh store location
 instead of setting $SHELL.
Date: Wed, 04 Oct 2017 14:12:27 +0200 (CEST)

From: Christopher Baines <mail <at> cbaines.net>
Subject: Re: [bug#28660] [PATCH] gnu: python-numpy: Give sh store 
location instead of setting $SHELL.
Date: Wed, 4 Oct 2017 08:49:44 +0100

> Sorry Thomas, ignore what I originally said. I thought from reading 
> the
> patch that this was patching numpy to use /bin/sh . Now after 
> building
> it and looking at the resulting files, I can see that it makes the
> default use bash from the store. Which was exactly what I was
> suggesting, but you were already doing that.
>
> With this new, hopefully more correct interpretation, this patch 
> looks
> fine to me :)

Yes, that's the purpose exactly :)  I've slightly reworded the commit 
message & comment to hopefully make it less confusing, and pushed.

Thanks for taking a look!

Thomas

Previous Next

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