[bug#59073] [PATCH] gnu: r-minimal: Avoid referencing /gnu/store/[^-]+-glibc-[^-]+-static.

[bug#59073] [PATCH] gnu: r-minimal: Avoid referencing /gnu/store/[^-]+-glibc-[^-]+-static.

[Christopher Baines]

As this causes the following error when computing a cross-compilation
derivation for r-minimal and related packages.

I think this is coming from the string appearing in the sources section of the
derivation, but not being a valid store path.

* gnu/packages/statistics.scm (r-minimal)[arguments]: Split string involving
/gnu/store.
---
 gnu/packages/statistics.scm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/statistics.scm b/gnu/packages/statistics.scm
index 2d4dbe4a31..43988ad00e 100644
--- a/gnu/packages/statistics.scm
+++ b/gnu/packages/statistics.scm
@@ -470,7 +470,11 @@ (define-public r-minimal
                                     "util-macros"
                                     "graphite2"))
                           "|"
-                          "/gnu/store/[^-]+-glibc-[^-]+-static"
+                          ;; Be careful when including store paths in the
+                          ;; build script, since they might be treated as
+                          ;; references
+                          "/gnu/store"
+                          "/[^-]+-glibc-[^-]+-static"
                           ")/lib")) ""))))))))))))
 
 (define-public rmath-standalone
-- 
2.37.3

[bug#59073] [PATCH] gnu: r-minimal: Avoid referencing /gnu/store/[^-]+-glibc-[^-]+-static.

[zimoun]

Hi,

On dim., 06 nov. 2022 at 10:05, Christopher Baines <mail@cbaines.net> wrote:

> * gnu/packages/statistics.scm (r-minimal)[arguments]: Split string involving
> /gnu/store.

It would imply rebuild many packages, no?  master, staging or
core-updates?

> ---
>  gnu/packages/statistics.scm | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/statistics.scm b/gnu/packages/statistics.scm
> index 2d4dbe4a31..43988ad00e 100644
> --- a/gnu/packages/statistics.scm
> +++ b/gnu/packages/statistics.scm
> @@ -470,7 +470,11 @@ (define-public r-minimal
>                                      "util-macros"
>                                      "graphite2"))
>                            "|"
> -                          "/gnu/store/[^-]+-glibc-[^-]+-static"
> +                          ;; Be careful when including store paths in the
> +                          ;; build script, since they might be treated as
> +                          ;; references
> +                          "/gnu/store"
> +                          "/[^-]+-glibc-[^-]+-static"
>                            ")/lib")) ""))))))))))))

LGTM.


Cheers,
simon

[bug#59073] [PATCH] gnu: r-minimal: Avoid referencing /gnu/store/[^-]+-glibc-[^-]+-static.

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 440 bytes --]


zimoun <zimon.toutoune@gmail.com> writes:

> Hi,
>
> On dim., 06 nov. 2022 at 10:05, Christopher Baines <mail@cbaines.net> wrote:
>
>> * gnu/packages/statistics.scm (r-minimal)[arguments]: Split string involving
>> /gnu/store.
>
> It would imply rebuild many packages, no?  master, staging or
> core-updates?

It does, my plan was/is to ask Ricardo if he could maybe include it when
pushing the next big batch of r related changes?

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

[bug#59073] [PATCH] gnu: r-minimal: Avoid referencing /gnu/store/[^-]+-glibc-[^-]+-static.

[zimoun]

Hi Chris,

On lun., 07 nov. 2022 at 10:32, Christopher Baines <mail@cbaines.net> wrote:

> It does, my plan was/is to ask Ricardo if he could maybe include it when
> pushing the next big batch of r related changes?

I am preparing a Bioconductor upgrade, so it could be part of it.
Ricardo, WDYT?


Cheers,
simon

[bug#59073] [PATCH] gnu: r-minimal: Avoid referencing /gnu/store/[^-]+-glibc-[^-]+-static.

[Ludovic Courtès]

Christopher Baines <mail@cbaines.net> skribis:

> As this causes the following error when computing a cross-compilation
> derivation for r-minimal and related packages.

Which error?  :-)

Ludo’.

[bug#59073] [PATCH v2] gnu: r-minimal: Avoid referencing /gnu/store/[^-]+-glibc-[^-]+-static.

[Christopher Baines]

As this causes the following error when computing a cross-compilation
derivation for r-minimal and related packages:

  guix build: error: path ‘/gnu/store/[^-]+-glibc-[^-]+-static’ is not valid

I think this is coming from the string appearing in the builder and thus the
sources section of the derivation, but not being a valid store path.

* gnu/packages/statistics.scm (r-minimal)[arguments]: Split string involving
/gnu/store.
---
 gnu/packages/statistics.scm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/statistics.scm b/gnu/packages/statistics.scm
index 2d4dbe4a31..43988ad00e 100644
--- a/gnu/packages/statistics.scm
+++ b/gnu/packages/statistics.scm
@@ -470,7 +470,11 @@ (define-public r-minimal
                                     "util-macros"
                                     "graphite2"))
                           "|"
-                          "/gnu/store/[^-]+-glibc-[^-]+-static"
+                          ;; Be careful when including store paths in the
+                          ;; build script, since they might be treated as
+                          ;; references
+                          "/gnu/store"
+                          "/[^-]+-glibc-[^-]+-static"
                           ")/lib")) ""))))))))))))
 
 (define-public rmath-standalone
-- 
2.37.3

[bug#59073] [PATCH] gnu: r-minimal: Avoid referencing /gnu/store/[^-]+-glibc-[^-]+-static.

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]


Ludovic Courtès <ludo@gnu.org> writes:

> Christopher Baines <mail@cbaines.net> skribis:
>
>> As this causes the following error when computing a cross-compilation
>> derivation for r-minimal and related packages.
>
> Which error?  :-)

Good spot, I've updated the patch/commit message.

But it's this:

  → guix build --target=aarch64-linux-gnu r-minimal
  guix build: error: path ‘/gnu/store/[^-]+-glibc-[^-]+-static’ is not valid

I'm not sure why this seems to only affect cross derivations. It's not
the only problem that's happened for cross derivations though, so it
might be good to expand the linting to cover them (maybe even for just
one common target like aarch64-linux-gnu).

Thanks,

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

[bug#59073] [PATCH] gnu: r-minimal: Avoid referencing /gnu/store/[^-]+-glibc-[^-]+-static.

[Ludovic Courtès]

Hi,

Christopher Baines <mail@cbaines.net> skribis:

>   → guix build --target=aarch64-linux-gnu r-minimal
>   guix build: error: path ‘/gnu/store/[^-]+-glibc-[^-]+-static’ is not valid

Interesting bug!

I pushed a fix and explanation:

--8<---------------cut here---------------start------------->8---
commit 3bd4b92f55f40119349e39902a9b800de98040d2

    build-system/gnu: Turn #:phases into a gexp when cross-compiling.
    
    Previously, we'd get this error:
    
      $ guix build --target=aarch64-linux-gnu r-minimal -d --no-grafts
      guix build: error: path ‘/gnu/store/[^-]+-glibc-[^-]+-static’ is not valid
    
    This is because the sexp would be passed as an input of the surrounding
    gexp in 'gnu-cross-build', and thus
    "/gnu/store/[^-]+-glibc-[^-]+-static" would be interpreted as a source
    file name, as in this example:
    
      scheme@(guix gexp)> #~(foo #$(list 'whatever "/gnu/store/[^-]+-glibc-[^-]+-static"))
      $11 = #<gexp (foo #<gexp-input (whatever "/gnu/store/[^-]+-glibc-[^-]+-static"):out>) 7f098badec30>
      scheme@(guix gexp)> (gexp-inputs $11)
      $12 = (#<gexp-input "/gnu/store/[^-]+-glibc-[^-]+-static":out>)
    
    Fixes <https://issues.guix.gnu.org/59073>.
    Reported by Christopher Baines <mail@cbaines.net>.
    
    * guix/build-system/gnu.scm (gnu-cross-build): When PHASES is a pair,
    pass it through 'sexp->gexp'.
--8<---------------cut here---------------end--------------->8---

That said, this code is bogus:

  1. We should refrain from using non-literal strings as patterns in
     ‘substitute*’.

  2. We should call ‘%store-directory’ from (guix build utils) instead
     of hardcoding “/gnu/store”.

Both of these should be fixed at some point.

Thanks,
Ludo’.

[bug#59073] [PATCH] gnu: r-minimal: Avoid referencing /gnu/store/[^-]+-glibc-[^-]+-static.

[zimoun]

Hi,

On Tue, 08 Nov 2022 at 14:09, Ludovic Courtès <ludo@gnu.org> wrote:

> Interesting bug!
>
> I pushed a fix and explanation:

Oh! :-)

> That said, this code is bogus:
>
>   1. We should refrain from using non-literal strings as patterns in
>      ‘substitute*’.

What does it mean “non-literal string” here?


Cheers,
simon

[bug#59073] [PATCH] gnu: r-minimal: Avoid referencing /gnu/store/[^-]+-glibc-[^-]+-static.

[Ludovic Courtès]

Hi,

zimoun <zimon.toutoune@gmail.com> skribis:

>> That said, this code is bogus:
>>
>>   1. We should refrain from using non-literal strings as patterns in
>>      ‘substitute*’.
>
> What does it mean “non-literal string” here?

A string that is not a literal (or constant, if you prefer).

This is OK:

  (substitute* f
    ((".*") …))

This is not:

  (substitute* f
    (((string-append "." (star))) …))

Ludo’.

[bug#59073] [PATCH] gnu: r-minimal: Avoid referencing /gnu/store/[^-]+-glibc-[^-]+-static.

[zimoun]

Hi,

On jeu., 10 nov. 2022 at 15:07, Ludovic Courtès <ludo@gnu.org> wrote:

> A string that is not a literal (or constant, if you prefer).
>
> This is OK:
>
>   (substitute* f
>     ((".*") …))
>
> This is not:
>
>   (substitute* f
>     (((string-append "." (star))) …))

Thanks for explaining.

Cheers,
simon