bug#25235: [PATCH 1/1] build-system/pyproject: Do not wrap native-inputs.

[Maxim Cournoyer]

Hi Efraim,

Efraim Flashner  writes:

> I'm still on my first cup of coffee...
>
> If a python input is in inputs and in native-inputs would it be included
> in the wrapper or not? At first glance I'd say no. Does the search path
> do deduplication? If it doesn't then it'd work to only remove the first
> instance matching from native-input-dirs.

It wouldn't be included with the current simple logic, which simply
remove any native input from the items of GUIX_PYTHONPATH.  Good catch!
I'll send a v2 with the refined check.

-- 
Thanks,
Maxim

bug#25235: [PATCH 1/1] build-system/pyproject: Do not wrap native-inputs.

[Efraim Flashner]

I'm still on my first cup of coffee...

If a python input is in inputs and in native-inputs would it be included
in the wrapper or not? At first glance I'd say no. Does the search path
do deduplication? If it doesn't then it'd work to only remove the first
instance matching from native-input-dirs.

On Mon, Jan 16, 2023 at 04:29:19PM -0500, Maxim Cournoyer wrote:
> Fixes .
> 
> * guix/build/pyproject-build-system.scm (wrap) [native-inputs]: New argument.
> Filter out native inputs from the values in GUIX_PYTHONPATH.
> 
> ---
> 
>  guix/build/pyproject-build-system.scm | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/guix/build/pyproject-build-system.scm 
> b/guix/build/pyproject-build-system.scm
> index a66c1fb34a..cd418f7ec9 100644
> --- a/guix/build/pyproject-build-system.scm
> +++ b/guix/build/pyproject-build-system.scm
> @@ -445,7 +445,7 @@ (define* (add-install-to-pythonpath #:key native-inputs 
> outputs
>"A phase that just wraps the 'add-installed-pythonpath' procedure."
>(add-installed-pythonpath native-inputs outputs))
>  
> -(define* (wrap #:key inputs outputs #:allow-other-keys)
> +(define* (wrap #:key native-inputs inputs outputs #:allow-other-keys)
>(define (list-of-files dir)
>  (find-files dir (lambda (file stat)
>(and (eq? 'regular (stat:type stat))
> @@ -464,9 +464,17 @@ (define bindirs
>(define %guile (delay (search-input-file inputs "bin/guile")))
>(define (guile) (force %guile))
>  
> -  (let* ((var `("GUIX_PYTHONPATH" prefix
> -,(search-path-as-string->list
> -  (or (getenv "GUIX_PYTHONPATH") "")
> +  ;; Use the same strategy to compute the native-input file names.
> +  (define %native-input-dirs (delay (match native-inputs
> +  (((_ . dir) ...)
> +   dir
> +  (define (native-input-dirs) (force %native-input-dirs))
> +
> +  (let ((var `("GUIX_PYTHONPATH" prefix
> +   ,(remove (lambda (x)
> +  (any (cut string-prefix? <> x) 
> (native-input-dirs)))
> +(search-path-as-string->list
> + (or (getenv "GUIX_PYTHONPATH") ""))
>  (for-each (lambda (dir)
>  (let ((files (list-of-files dir)))
>(for-each (cut wrap-script <> #:guile (guile) var)
> -- 
> 2.38.1
> 
> 
> 
> 

-- 
Efraim Flashner  אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted


signature.asc
Description: PGP signature

bug#25235: [PATCH 1/1] build-system/pyproject: Do not wrap native-inputs.

[Maxim Cournoyer]

Hi Ludovic,

Ludovic Courtès  writes:

> Hi,
>
> Maxim Cournoyer  skribis:
>
>> Fixes .
>>
>> * guix/build/pyproject-build-system.scm (wrap) [native-inputs]: New argument.
>> Filter out native inputs from the values in GUIX_PYTHONPATH.
>
> Yay, great to see this fixed!
>
>>  guix/build/pyproject-build-system.scm | 16 
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/guix/build/pyproject-build-system.scm 
>> b/guix/build/pyproject-build-system.scm
>> index a66c1fb34a..cd418f7ec9 100644
>> --- a/guix/build/pyproject-build-system.scm
>> +++ b/guix/build/pyproject-build-system.scm
>> @@ -445,7 +445,7 @@ (define* (add-install-to-pythonpath #:key native-inputs 
>> outputs
>>"A phase that just wraps the 'add-installed-pythonpath' procedure."
>>(add-installed-pythonpath native-inputs outputs))
>>
>> -(define* (wrap #:key inputs outputs #:allow-other-keys)
>> +(define* (wrap #:key native-inputs inputs outputs #:allow-other-keys)
>>(define (list-of-files dir)
>>  (find-files dir (lambda (file stat)
>>(and (eq? 'regular (stat:type stat))
>> @@ -464,9 +464,17 @@ (define bindirs
>>(define %guile (delay (search-input-file inputs "bin/guile")))
>>(define (guile) (force %guile))
>>
>> -  (let* ((var `("GUIX_PYTHONPATH" prefix
>> -,(search-path-as-string->list
>> -  (or (getenv "GUIX_PYTHONPATH") "")
>> +  ;; Use the same strategy to compute the native-input file names.
>> +  (define %native-input-dirs (delay (match native-inputs
>> +  (((_ . dir) ...)
>> +   dir
>> +  (define (native-input-dirs) (force %native-input-dirs))
>> +
>> +  (let ((var `("GUIX_PYTHONPATH" prefix
>> +   ,(remove (lambda (x)
>> +  (any (cut string-prefix? <> x) 
>> (native-input-dirs)))
>> +(search-path-as-string->list
>> + (or (getenv "GUIX_PYTHONPATH") ""))
>
> I think there’s no need to delay+thunk things since we’re using them
> right away.  How about:
>
>   (define native-input-directories
> (match native-inputs
>   …))
>
>   (let ((var `("GUIX_PYTHONPATH" prefix
>,(remove (lambda (x) … native-input-directories …) …
> …)
>
> ?

It's a (small) optimization, that was intended to delay things until
they are really needed that was already used for %bash (now %guile) in
the wrap phase of (guix build python-build-system).

The idea is to delay the evaluation of native-input-directories or the
lookup for guile in the inputs until it's really necessary.  For
example, the computations are avoided when there are no "bin" or "sbin"
sub-directories in the outputs.

> (Same for ‘%guile’ above, but I don’t see it in ‘master’; maybe that’s
> from another branch?)

Yes, the change build on both #60847 (Enable cross-compilation for the
pyproject-build-system) and #60868 (Use wrap-script instead of
wrap-program), which should be applied first.  %guile appears in #60868,
and changes that make the above easy (native-inputs always appearing as
native-inputs, even for native builds) are from #60847.

-- 
Thanks,
Maxim

bug#25235: [PATCH 1/1] build-system/pyproject: Do not wrap native-inputs.

[Ludovic Courtès]

Hi,

Maxim Cournoyer  skribis:

> Fixes .
>
> * guix/build/pyproject-build-system.scm (wrap) [native-inputs]: New argument.
> Filter out native inputs from the values in GUIX_PYTHONPATH.

Yay, great to see this fixed!

>  guix/build/pyproject-build-system.scm | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/guix/build/pyproject-build-system.scm 
> b/guix/build/pyproject-build-system.scm
> index a66c1fb34a..cd418f7ec9 100644
> --- a/guix/build/pyproject-build-system.scm
> +++ b/guix/build/pyproject-build-system.scm
> @@ -445,7 +445,7 @@ (define* (add-install-to-pythonpath #:key native-inputs 
> outputs
>"A phase that just wraps the 'add-installed-pythonpath' procedure."
>(add-installed-pythonpath native-inputs outputs))
>  
> -(define* (wrap #:key inputs outputs #:allow-other-keys)
> +(define* (wrap #:key native-inputs inputs outputs #:allow-other-keys)
>(define (list-of-files dir)
>  (find-files dir (lambda (file stat)
>(and (eq? 'regular (stat:type stat))
> @@ -464,9 +464,17 @@ (define bindirs
>(define %guile (delay (search-input-file inputs "bin/guile")))
>(define (guile) (force %guile))
>  
> -  (let* ((var `("GUIX_PYTHONPATH" prefix
> -,(search-path-as-string->list
> -  (or (getenv "GUIX_PYTHONPATH") "")
> +  ;; Use the same strategy to compute the native-input file names.
> +  (define %native-input-dirs (delay (match native-inputs
> +  (((_ . dir) ...)
> +   dir
> +  (define (native-input-dirs) (force %native-input-dirs))
> +
> +  (let ((var `("GUIX_PYTHONPATH" prefix
> +   ,(remove (lambda (x)
> +  (any (cut string-prefix? <> x) 
> (native-input-dirs)))
> +(search-path-as-string->list
> + (or (getenv "GUIX_PYTHONPATH") ""))

I think there’s no need to delay+thunk things since we’re using them
right away.  How about:

  (define native-input-directories
(match native-inputs
  …))

  (let ((var `("GUIX_PYTHONPATH" prefix
   ,(remove (lambda (x) … native-input-directories …) …
…)

?

(Same for ‘%guile’ above, but I don’t see it in ‘master’; maybe that’s
from another branch?)

Thanks,
Ludo’.

bug#25235: [PATCH 1/1] build-system/pyproject: Do not wrap native-inputs.

[Maxim Cournoyer]

Fixes .

* guix/build/pyproject-build-system.scm (wrap) [native-inputs]: New argument.
Filter out native inputs from the values in GUIX_PYTHONPATH.

---

 guix/build/pyproject-build-system.scm | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/guix/build/pyproject-build-system.scm 
b/guix/build/pyproject-build-system.scm
index a66c1fb34a..cd418f7ec9 100644
--- a/guix/build/pyproject-build-system.scm
+++ b/guix/build/pyproject-build-system.scm
@@ -445,7 +445,7 @@ (define* (add-install-to-pythonpath #:key native-inputs 
outputs
   "A phase that just wraps the 'add-installed-pythonpath' procedure."
   (add-installed-pythonpath native-inputs outputs))
 
-(define* (wrap #:key inputs outputs #:allow-other-keys)
+(define* (wrap #:key native-inputs inputs outputs #:allow-other-keys)
   (define (list-of-files dir)
 (find-files dir (lambda (file stat)
   (and (eq? 'regular (stat:type stat))
@@ -464,9 +464,17 @@ (define bindirs
   (define %guile (delay (search-input-file inputs "bin/guile")))
   (define (guile) (force %guile))
 
-  (let* ((var `("GUIX_PYTHONPATH" prefix
-,(search-path-as-string->list
-  (or (getenv "GUIX_PYTHONPATH") "")
+  ;; Use the same strategy to compute the native-input file names.
+  (define %native-input-dirs (delay (match native-inputs
+  (((_ . dir) ...)
+   dir
+  (define (native-input-dirs) (force %native-input-dirs))
+
+  (let ((var `("GUIX_PYTHONPATH" prefix
+   ,(remove (lambda (x)
+  (any (cut string-prefix? <> x) (native-input-dirs)))
+(search-path-as-string->list
+ (or (getenv "GUIX_PYTHONPATH") ""))
 (for-each (lambda (dir)
 (let ((files (list-of-files dir)))
   (for-each (cut wrap-script <> #:guile (guile) var)
-- 
2.38.1