bug#62181: Gnulib package keeps references to clisp, Python, Perl, and Bash

bug#62181: Gnulib package keeps references to clisp, Python, Perl, and Bash

[Ludovic Courtès]

Hello,

The Gnulib package, which is meant to be source, retains references to
several packages:

--8<---------------cut here---------------start------------->8---
$ guix describe
Generation 250	Mar 12 2023 23:58:03	(current)
  guix 1ed227d
    repository URL: https://git.savannah.gnu.org/git/guix.git
    branch: master
    commit: 1ed227d7952af48efe50a2f6c9537e17c356daa1
$ guix gc --references $(guix build gnulib)
/gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8
/gnu/store/76p04alhjimlv4cgijl670byf2m50qh8-clisp-2.49-92
/gnu/store/bvnzi0z7i9qk31a03y64rs8sxrckkinr-python-3.9.9
/gnu/store/hy6abswwv4d89zp464fw52z65fkzr7h5-perl-5.34.0
--8<---------------cut here---------------end--------------->8---

We should probably fix that and add #:allowed-references '().

WDYT, Vivien?

Thanks,
Ludo’.

bug#62181: [PATCH 0/2] Re: bug#62181: Gnulib package keeps references to clisp, Python, Perl, and Bash

[Vivien Kraus via Bug reports for GNU Guix]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]

Hi!

Le mardi 14 mars 2023 à 09:34 +0100, Ludovic Courtès a écrit :
> The Gnulib package, which is meant to be source, retains references
> to
> several packages:
> 
> --8<---------------cut here---------------start------------->8---
> $ guix describe
> Generation 250  Mar 12 2023 23:58:03    (current)
>   guix 1ed227d
>     repository URL: https://git.savannah.gnu.org/git/guix.git
>     branch: master
>     commit: 1ed227d7952af48efe50a2f6c9537e17c356daa1
> $ guix gc --references $(guix build gnulib)
> /gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8
> /gnu/store/76p04alhjimlv4cgijl670byf2m50qh8-clisp-2.49-92
> /gnu/store/bvnzi0z7i9qk31a03y64rs8sxrckkinr-python-3.9.9
> /gnu/store/hy6abswwv4d89zp464fw52z65fkzr7h5-perl-5.34.0
> --8<---------------cut here---------------end--------------->8---
> 
> We should probably fix that and add #:allowed-references '().

These were shebangs in the build auxiliary scripts. It’s a tough problem,
because if I undo all the patch shebangs after installation, then gnulib is
not usable in the gnu-build-system bootstrap phase without a few more
tweaks. That’s not really a problem, because we can fix all our gnulib users
(guile-gnutls), but we should expect issues if gnulib bootstrap scripts call
themselves without an explicit interpreter.

In any case, #:allowed-reference is a gnu-build-system thing, so we can’t use that for gnulib.

Vivien

Vivien Kraus (2):
  gnu: gnulib: Reset the shebangs.
  gnu: guile-gnutls: Do not expect gnulib shebangs to work.

 gnu/packages/build-tools.scm | 28 ++++++++++++++++++++++++----
 gnu/packages/tls.scm         |  6 ++++++
 2 files changed, 30 insertions(+), 4 deletions(-)


base-commit: 302680dbce784a53bfddb4ce5e6a9505cd0c477a
-- 
2.39.2

bug#62181: [PATCH 1/2] gnu: gnulib: Reset the shebangs.

[Vivien Kraus via Bug reports for GNU Guix]

* gnu/packages/build-tools.scm (gnulib) [phase reset-shebangs]: After
installation, rewrite the /gnu/store shebangs in the distributed build-aux
files.
[inputs, native-inputs]: Add bash-minimal.
[phase let-autogen-execute-gnulib-tool]: Specify a shell to execute
gnulib-tool from autogen.sh.
---
 gnu/packages/build-tools.scm | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/build-tools.scm b/gnu/packages/build-tools.scm
index 28ab77bbb4..f93a72f2f5 100644
--- a/gnu/packages/build-tools.scm
+++ b/gnu/packages/build-tools.scm
@@ -973,12 +973,32 @@ (define (find-ucd-files . names)
                    ("NormalizationTest.txt" . "uninorm")
                    ("auxiliary/GraphemeBreakTest.txt" . "unigbrk")
                    ("auxiliary/WordBreakTest.txt" . "uniwbrk")))
-                (delete-file "gen-uni-tables")))))))
-    (inputs ;; Shebangs for some auxiliary build files.
-     (list python perl clisp))
+                (delete-file "gen-uni-tables"))))
+          (add-after 'install 'reset-shebangs
+            (lambda _
+              (for-each
+               (lambda (file)
+                 (false-if-exception
+                  ;; Might fail on binary files, this is not a problem.
+                  (substitute* file
+                    ((#$(file-append (this-package-native-input "bash-minimal")
+                                     "/bin/sh"))
+                     "/bin/sh")
+                    ((#$(file-append (this-package-native-input "python")
+                                     "/bin/python3"))
+                     "/usr/bin/env python3")
+                    ((#$(file-append (this-package-native-input "perl")
+                                     "/bin/perl"))
+                     "/usr/bin/perl")
+                    ((#$(file-append (this-package-native-input "clisp")
+                                     "/bin/clisp"))
+                     "/usr/bin/clisp"))))
+               (find-files (string-append #$output "/src/gnulib"))))))))
+    (inputs ;; Shebang for gnulib-tool
+     (list bash-minimal))
     (native-inputs
      (list
-      python perl clisp
+      bash-minimal python perl clisp
       ;; Unicode data:
       ucd-next
       ;; Programs for the tests:
-- 
2.39.2

bug#62181: [PATCH 1/2] gnu: gnulib: Reset the shebangs.

[Ludovic Courtès]

Vivien Kraus <vivien@planete-kraus.eu> skribis:

> * gnu/packages/build-tools.scm (gnulib) [phase reset-shebangs]: After
> installation, rewrite the /gnu/store shebangs in the distributed build-aux
> files.
> [inputs, native-inputs]: Add bash-minimal.
> [phase let-autogen-execute-gnulib-tool]: Specify a shell to execute
> gnulib-tool from autogen.sh.

[...]

> +          (add-after 'install 'reset-shebangs

s/reset/restore/ ?

> +            (lambda _
> +              (for-each
> +               (lambda (file)
> +                 (false-if-exception
> +                  ;; Might fail on binary files, this is not a problem.
> +                  (substitute* file
> +                    ((#$(file-append (this-package-native-input "bash-minimal")
> +                                     "/bin/sh"))
> +                     "/bin/sh")
> +                    ((#$(file-append (this-package-native-input "python")
> +                                     "/bin/python3"))
> +                     "/usr/bin/env python3")
> +                    ((#$(file-append (this-package-native-input "perl")
> +                                     "/bin/perl"))
> +                     "/usr/bin/perl")
> +                    ((#$(file-append (this-package-native-input "clisp")
> +                                     "/bin/clisp"))
> +                     "/usr/bin/clisp"))))
> +               (find-files (string-append #$output "/src/gnulib"))))))))

How about:

  (substitute* (find-files …)
    (("^#! ?(.*)/bin/([a-zA-Z0-9-]+)" _ prefix program)
     (string-append "#!/bin/" program "\n")))

?

Ludo’.

bug#62181: [PATCH 2/2] gnu: guile-gnutls: Do not expect gnulib shebangs to work.

[Vivien Kraus via Bug reports for GNU Guix]

* gnu/packages/tls.scm (guile-gnutls) [phase patch-more-shebangs]: Fix how
autogen.sh invokes gnulib-tool, and how configure.ac invokes git-version-gen,
so as not to rely on shebangs.
---
 gnu/packages/tls.scm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gnu/packages/tls.scm b/gnu/packages/tls.scm
index 81d90c86ae..5a44fa89dc 100644
--- a/gnu/packages/tls.scm
+++ b/gnu/packages/tls.scm
@@ -417,6 +417,12 @@ (define-public guile-gnutls
       #~(modify-phases %standard-phases
           (add-after 'unpack 'patch-more-shebangs
             (lambda _
+              (substitute* "autogen.sh"
+                (("\\$gnulib_tool \\$gnulib_tool_options")
+                 "sh $gnulib_tool $gnulib_tool_options"))
+              (substitute* "configure.ac"
+                (("build-aux/git-version-gen")
+                 "sh build-aux/git-version-gen"))
               (for-each patch-shebang
                         '("autopull.sh" "autogen.sh"))))
           (replace 'bootstrap
-- 
2.39.2

bug#62181: [PATCH 2/2] gnu: guile-gnutls: Do not expect gnulib shebangs to work.

[Ludovic Courtès]

Vivien Kraus <vivien@planete-kraus.eu> skribis:

> * gnu/packages/tls.scm (guile-gnutls) [phase patch-more-shebangs]: Fix how
> autogen.sh invokes gnulib-tool, and how configure.ac invokes git-version-gen,
> so as not to rely on shebangs.

[...]

> +              (substitute* "configure.ac"
> +                (("build-aux/git-version-gen")
> +                 "sh build-aux/git-version-gen"))
>                (for-each patch-shebang
>                          '("autopull.sh" "autogen.sh"))))

Maybe you can instead add ‘build-aux/git-version-gen’ to the ‘for-each’
list above?

Ludo’.

bug#62181: [PATCH 2/2] gnu: guile-gnutls: Do not expect gnulib shebangs to work.

[Vivien Kraus via Bug reports for GNU Guix]

Le jeudi 16 mars 2023 à 11:48 +0100, Ludovic Courtès a écrit :
> > +              (substitute* "configure.ac"
> > +                (("build-aux/git-version-gen")
> > +                 "sh build-aux/git-version-gen"))
> >                (for-each patch-shebang
> >                          '("autopull.sh" "autogen.sh"))))
> 
> Maybe you can instead add ‘build-aux/git-version-gen’ to the ‘for-
> each’
> list above?

Unfortunately, no, because it is imported from gnulib (so it does not
exist before this early patch shebang phase) and *immediately* invoked
by the same bootstrap script, so I have no time window when I can fix
it.

Vivien

bug#62181: [PATCH 0/2] Re: bug#62181: Gnulib package keeps references to clisp, Python, Perl, and Bash

[Ludovic Courtès]

Hello!

Vivien Kraus <vivien@planete-kraus.eu> skribis:

> These were shebangs in the build auxiliary scripts. It’s a tough problem,
> because if I undo all the patch shebangs after installation, then gnulib is
> not usable in the gnu-build-system bootstrap phase without a few more
> tweaks.

Is it really a problem?  Apart from ‘gnulib-tool’, every script is first
copied into the source tree, and that happens before the
‘patch-source-shebangs’ phase no?

> In any case, #:allowed-reference is a gnu-build-system thing, so we can’t use that for gnulib.

True; we should add it to other build systems.

Thanks,
Ludo’.

bug#62181: [PATCH 0/2] Re: bug#62181: Gnulib package keeps references to clisp, Python, Perl, and Bash

[Vivien Kraus via Bug reports for GNU Guix]

Le jeudi 16 mars 2023 à 11:41 +0100, Ludovic Courtès a écrit :
> Apart from ‘gnulib-tool’, every script is first
> copied into the source tree, and that happens before the
> ‘patch-source-shebangs’ phase no?

Precisely, so if they are executed during the bootstrap phase, we can’t
rely on their shebangs being correct yet. It depends on individual
packages which build-aux scripts are actually called (although I’m
pretty sure noone will call the clisp thing). However, top/bootstrap-
funclib.sh is intended to be included in the top-level directory of the
package, to be invoked at bootstrap time, and at least this script
calls gnulib-tool (the one in /src, so the unpatched copy) incorrectly.

Vivien

bug#62181: [PATCH v2 0/2] Drop references to other store items in /src, with better style

[Vivien Kraus via Bug reports for GNU Guix]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

So I applied Ludo’s proposed style improvement with a few more tweaks to
respect the gnulib-intended shebangs, and making sure that substitute* failure
on /lib/javaversion.class and the .mo test data can be recovered.

This v2 is still not a silver bullet, because gnulib users need to patch a few
files before bootstrap.

Vivien Kraus (2):
  gnu: gnulib: Reset the shebangs.
  gnu: guile-gnutls: Do not expect gnulib shebangs to work.

 gnu/packages/build-tools.scm | 21 +++++++++++++++++----
 gnu/packages/tls.scm         |  6 ++++++
 2 files changed, 23 insertions(+), 4 deletions(-)


base-commit: 1820dc7ce6945569db4ef6e265764705e6aabc48
-- 
2.39.2

bug#62181: [PATCH v2 1/2] gnu: gnulib: Reset the shebangs.

[Vivien Kraus via Bug reports for GNU Guix]

* gnu/packages/build-tools.scm (gnulib) [phase reset-shebangs]: After
installation, rewrite the /gnu/store shebangs in the distributed build-aux
files.
[inputs, native-inputs]: Add bash-minimal.
[phase let-autogen-execute-gnulib-tool]: Specify a shell to execute
gnulib-tool from autogen.sh.
---
 gnu/packages/build-tools.scm | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/build-tools.scm b/gnu/packages/build-tools.scm
index 28ab77bbb4..caf0f3fc1c 100644
--- a/gnu/packages/build-tools.scm
+++ b/gnu/packages/build-tools.scm
@@ -973,12 +973,25 @@ (define (find-ucd-files . names)
                    ("NormalizationTest.txt" . "uninorm")
                    ("auxiliary/GraphemeBreakTest.txt" . "unigbrk")
                    ("auxiliary/WordBreakTest.txt" . "uniwbrk")))
-                (delete-file "gen-uni-tables")))))))
-    (inputs ;; Shebangs for some auxiliary build files.
-     (list python perl clisp))
+                (delete-file "gen-uni-tables"))))
+          (add-after 'install 'restore-shebangs
+            (lambda _
+              (substitute* (find-files
+                            (string-append #$output "/src/gnulib")
+                            (lambda (fname stat)
+                              (and (not (string-suffix? "/lib/javaversion.class" fname))
+                                   (not (string-suffix? ".mo" fname)))))
+                (("^#! ?(.*)/bin/sh" _ prefix)
+                 "#!/bin/sh")
+                (("^#! ?(.*)/bin/python3" _ prefix)
+                 "#!/usr/bin/env python3")
+                (("^#! ?(.*)/bin/([a-zA-Z0-9-]+)" _ prefix program)
+                 (string-append "#!/usr/bin/" program))))))))
+    (inputs ;; Shebang for gnulib-tool
+     (list bash-minimal))
     (native-inputs
      (list
-      python perl clisp
+      bash-minimal python perl clisp
       ;; Unicode data:
       ucd-next
       ;; Programs for the tests:
-- 
2.39.2

bug#62181: [PATCH v2 1/2] gnu: gnulib: Reset the shebangs.

[Ludovic Courtès]

Hi,

Vivien Kraus <vivien@planete-kraus.eu> skribis:

> * gnu/packages/build-tools.scm (gnulib) [phase reset-shebangs]: After
> installation, rewrite the /gnu/store shebangs in the distributed build-aux
> files.
> [inputs, native-inputs]: Add bash-minimal.
> [phase let-autogen-execute-gnulib-tool]: Specify a shell to execute
> gnulib-tool from autogen.sh.

Applied with tweaks to the commit log.

> +              (substitute* (find-files
> +                            (string-append #$output "/src/gnulib")
> +                            (lambda (fname stat)
> +                              (and (not (string-suffix? "/lib/javaversion.class" fname))
> +                                   (not (string-suffix? ".mo" fname)))))

This leaves the shebang on ‘bin/gnulib-tool’, such that ‘gnulib’ keeps a
reference to ‘bash-minimal’ (and nothing else).

It’s not even convenient though because…

> * gnu/packages/tls.scm (guile-gnutls) [phase patch-more-shebangs]: Fix how
> autogen.sh invokes gnulib-tool, and how configure.ac invokes git-version-gen,
> so as not to rely on shebangs.

[...]

> +              (substitute* "autogen.sh"
> +                (("\\$gnulib_tool \\$gnulib_tool_options")
> +                 "sh $gnulib_tool $gnulib_tool_options"))

… this is still needed: GNULIB/bin/gnulib-tool, which has a correct
shebang, is *not* used, and instead GNULIB/src/gnulib/gnulib-tool is
used.

Should we remove GNULIB/bin/gnulib-tool?  Or should we remove the other
one?

Anyway, applied as well, thank you!

Ludo’.

bug#62181: [PATCH v2 1/2] gnu: gnulib: Reset the shebangs.

[Vivien Kraus via Bug reports for GNU Guix]

Hi!

Le vendredi 17 mars 2023 à 22:48 +0100, Ludovic Courtès a écrit :
> > * gnu/packages/tls.scm (guile-gnutls) [phase patch-more-shebangs]:
> > Fix how
> > autogen.sh invokes gnulib-tool, and how configure.ac invokes git-
> > version-gen,
> > so as not to rely on shebangs.
> 
> [...]
> 
> > +              (substitute* "autogen.sh"
> > +                (("\\$gnulib_tool \\$gnulib_tool_options")
> > +                 "sh $gnulib_tool $gnulib_tool_options"))
> 
> … this is still needed: GNULIB/bin/gnulib-tool, which has a correct
> shebang, is *not* used, and instead GNULIB/src/gnulib/gnulib-tool is
> used.
> 
> Should we remove GNULIB/bin/gnulib-tool?  Or should we remove the
> other
> one?

If we could have different outputs, one for bin/gnulib-tool and one for
the gnulib source, then it would be better: the default output would
retain no references, and the :bin output would only need a bash-
minimal to run gnulib-tool. So packages that call gnulib-tool directly
would add both outputs as native-inputs, and packages that use
bootstrap.conf would only need the default output.

Would that help solve the problem introduced by the bash-minimal
reference?

Anyway, I’m not sure the copy-build-system allows for different
outputs.

What do you think?

Vivien

bug#62181: [PATCH v2 2/2] gnu: guile-gnutls: Do not expect gnulib shebangs to work.

[Vivien Kraus via Bug reports for GNU Guix]

* gnu/packages/tls.scm (guile-gnutls) [phase patch-more-shebangs]: Fix how
autogen.sh invokes gnulib-tool, and how configure.ac invokes git-version-gen,
so as not to rely on shebangs.
---
 gnu/packages/tls.scm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gnu/packages/tls.scm b/gnu/packages/tls.scm
index 81d90c86ae..5a44fa89dc 100644
--- a/gnu/packages/tls.scm
+++ b/gnu/packages/tls.scm
@@ -417,6 +417,12 @@ (define-public guile-gnutls
       #~(modify-phases %standard-phases
           (add-after 'unpack 'patch-more-shebangs
             (lambda _
+              (substitute* "autogen.sh"
+                (("\\$gnulib_tool \\$gnulib_tool_options")
+                 "sh $gnulib_tool $gnulib_tool_options"))
+              (substitute* "configure.ac"
+                (("build-aux/git-version-gen")
+                 "sh build-aux/git-version-gen"))
               (for-each patch-shebang
                         '("autopull.sh" "autogen.sh"))))
           (replace 'bootstrap
-- 
2.39.2