[PATCH core-updates 0/1] Specify output in input label when it's not "out".

Open

Details

3 participants

Josselin Poiret
Hilton Chain
Ludovic Courtès

Owner: unassigned

Submitted by: Hilton Chain

Severity: normal

Hilton Chain wrote on 5 Aug 04:50 +0200

Recipients:(address . guix-patches@gnu.org)(name . Hilton Chain)(address . hako@ultrarare.space)

Message-ID:cover.1691202289.git.hako@ultrarare.space

Hello Guix,

Recently I found it not possible to find `(,gcc "lib") in inputs with

`this-package-input' since it has the label "gcc" and there're other "gcc"s

in the build environment.

As we should avoid direct use on input labels, I think the solution is to

modify `add-input-label', hence the patch.

Taking `aide' from (gnu packages admin) as an example, the current behavior is

that both `pcre:static' and `pcre' have the label "pcre", this affects

`this-package-input' and `modify-inputs':

Toggle snippet (10 lines)

scheme@(guix-user)> ,use (guix packages)

scheme@(guix-user)> ,use (gnu packages admin)

scheme@(guix-user)> ((@@ (guix packages) add-input-label) (package-inputs aide))

$1 = ("_" ([...]

("pcre" #<package pcre@8.45 gnu/packages/pcre.scm:41 7f59cd759bb0> "static")

("pcre" #<package pcre@8.45 gnu/packages/pcre.scm:41 7f59cd759bb0>)

("zlib" #<package zlib@1.2.13 gnu/packages/compression.scm:106 7f59c130bd10> "static")

("zlib" #<package zlib@1.2.13 gnu/packages/compression.scm:106 7f59c130bd10>)))

With the patch appiled, `pcre:static' has the label "pcre:static", while

`pcre' stays "pcre":

Toggle snippet (10 lines)

scheme@(guix-user)> ,use (guix packages)

scheme@(guix-user)> ,use (gnu packages admin)

scheme@(guix-user)> ((@@ (guix packages) add-input-label) (package-inputs aide))

$1 = ("_" ([...]

("pcre:static" #<package pcre@8.45 gnu/packages/pcre.scm:41 7f6fe32efe70> "static")

("pcre" #<package pcre@8.45 gnu/packages/pcre.scm:41 7f6fe32efe70>)

("zlib:static" #<package zlib@1.2.13 gnu/packages/compression.scm:106 7f6fd244a000> "static")

("zlib" #<package zlib@1.2.13 gnu/packages/compression.scm:106 7f6fd244a000>)))

Thanks

Hilton Chain (1):

packages: Specify output in input label when it's not "out".

guix/packages.scm | 8 +++++++-

1 file changed, 7 insertions(+), 1 deletion(-)

base-commit: 8852e6bb5521edca099d6f346efc92db3244584c

2.41.0

Hilton Chain wrote on 5 Aug 04:53 +0200

[PATCH core-updates 1/1] packages: Specify output in input label when it's not "out".

Recipients:(address . 65062@debbugs.gnu.org)(name . Hilton Chain)(address . hako@ultrarare.space)

Message-ID:b6c9adca21cc4418219b51532c2f0a9bddb208f0.1691202289.git.hako@ultrarare.space

* guix/packages.scm (add-input-label): Specify output when it's not "out".

---

guix/packages.scm | 8 +++++++-

1 file changed, 7 insertions(+), 1 deletion(-)

Toggle diff (21 lines)diff --git a/guix/packages.scm b/guix/packages.scm
index ba98bb0fb4..d0e6e16cbb 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -626,7 +626,13 @@ (define (add-input-label input)
     ((? package? package)
      (list (package-name package) package))
     (((? package? package) output)                ;XXX: ugly?
-     (list (package-name package) package output))
+     (if (string=? output "out")
+         ;; (package "out") => ("package" package "out")
+         (list (package-name package) package output)
+         ;; (package "output") => ("package:output" package "output")
+         (list (string-append (package-name package) ":" output)
+               package
+               output)))
     ((? gexp-input?)       ;XXX: misplaced because 'native?' field is ignored?
      (let ((obj    (gexp-input-thing input))
            (output (gexp-input-output input)))
-- 
2.41.0

Hilton Chain wrote on 5 Aug 05:01 +0200

Re: [PATCH core-updates 0/1] Specify output in input label when it's not "out".

Recipients:(address . 65062@debbugs.gnu.org)

Message-ID:87il9ui44r.wl-hako@ultrarare.space

On Sat, 05 Aug 2023 10:50:32 +0800,

Hilton Chain wrote:

Toggle quote (14 lines)> 
> Hello Guix,
> 
> Recently I found it not possible to find `(,gcc "lib") in inputs with
> `this-package-input' since it has the label "gcc" and there're other "gcc"s
> in the build environment.
> 
> As we should avoid direct use on input labels, I think the solution is to
> modify `add-input-label', hence the patch.
> 
> Taking `aide' from (gnu packages admin) as an example, the current behavior is
> that both `pcre:static' and `pcre' have the label "pcre", this affects
> `this-package-input' and `modify-inputs':

Ahh sorry, I haven't checked `lookup-input', it seems that it doesn't

use input labels, so this patch only applies to `modify-inputs'.

Hilton Chain wrote on 5 Aug 05:19 +0200

Recipients:

Message-ID:87h6pei3bh.wl-hako@ultrarare.space

tags 65062 moreinfo

thanks

On Sat, 05 Aug 2023 11:01:40 +0800,

Hilton Chain wrote:

Toggle quote (3 lines)

> Ahh sorry, I haven't checked `lookup-input', it seems that it doesn't

> use input labels, so this patch only applies to `modify-inputs'.

Sorry for the noise, I have checked `lookup-input' and it uses labels,

but returns unwanted result with this patch (searching for "gcc:lib"

returns a "gcc").

I'll check that out.

Ludovic Courtès wrote on 22 Aug 18:00 +0200

Re: [bug#65062] [PATCH core-updates 1/1] packages: Specify output in input label when it's not "out".

Recipients:(name . Hilton Chain)(address . hako@ultrarare.space)

Message-ID:875y575apr.fsf@gnu.org

Hi,

Hilton Chain <hako@ultrarare.space> skribis:

Toggle quote (2 lines)

> * guix/packages.scm (add-input-label): Specify output when it's not "out".

[...]

Toggle quote (4 lines)

> + (list (string-append (package-name package) ":" output)

> + package

> + output)))

The Grand Plan¹ is to eventually get rid of labels entirely (or almost:

there’d still be input alists on the build side). As such, I thought we

shouldn’t worry too much about what the actual label is. But perhaps

you stumbled upon situations where this is a problem? Could you

describe them?

Thanks,

Ludo’.

¹ https://guix.gnu.org/en/blog/2021/the-big-change/

Hilton Chain wrote on 24 Aug 05:42 +0200

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID:87msyhumwj.wl-hako@ultrarare.space

Hi Ludo,

On Wed, 23 Aug 2023 00:00:00 +0800,

Ludovic Courtès wrote:

Toggle quote (24 lines)>
> Hi,
>
> Hilton Chain <hako@ultrarare.space> skribis:
>
> > * guix/packages.scm (add-input-label): Specify output when it's not "out".
>
> [...]
>
> > +         (list (string-append (package-name package) ":" output)
> > +               package
> > +               output)))
>
> The Grand Plan¹ is to eventually get rid of labels entirely (or almost:
> there’d still be input alists on the build side).  As such, I thought we
> shouldn’t worry too much about what the actual label is.  But perhaps
> you stumbled upon situations where this is a problem?  Could you
> describe them?
>
> Thanks,
> Ludo’.
>
> ¹ https://guix.gnu.org/en/blog/2021/the-big-change/

My main concern is that currently modify-inputs, this-package-input

and this-package-native-input operate on input labels and there would

be duplicated labels if adding multiple outputs of a package.

For modify-inputs, I think there's no approach to solve this without

also specifying labels in inputs.

Although this-package-* can be replaced by search-input-*, I'd like to

avoid (dirname (dirname (search-input-file inputs "/lib/..."))) when

(this-package-input "...") is available.

For current this-package-* vs. search-input-*, I have other points:

1. In the context of build system arguments, like #:configure-flags,

inputs and native-inputs as variables aren't available, one may need

to use %build-inputs, %build-host-inputs and %build-target-inputs for

search-input-*, which is inconsistent with other parts.

2. It might be a bit confusing when, for example, adding

tzdata-for-test to native-inputs, and referencing it with proper

cross-compilation support:

Toggle snippet (6 lines)

(setenv "TZDIR"

(search-input-directory

(if #$(%current-target-system) native-inputs inputs)

"/share/zoneinfo"))

In such cases I may prefer this-package-*, but it would be unreliable

when there're duplicated labels.

There's also issue referencing a package when multiple versions of it

under a same name are added to the inputs, which may not fall under

this "Subject:".

Thanks

Josselin Poiret wrote on 25 Aug 13:10 +0200

Recipients:

Message-ID:87a5ufv0mv.fsf@jpoiret.xyz

Hi everyone,

Hilton Chain <hako@ultrarare.space> writes:

Toggle quote (10 lines)> 2. It might be a bit confusing when, for example, adding
> tzdata-for-test to native-inputs, and referencing it with proper
> cross-compilation support:
> --8<---------------cut here---------------start------------->8---
> (setenv "TZDIR"
>         (search-input-directory
>          (if #$(%current-target-system) native-inputs inputs)
>          "/share/zoneinfo"))
> --8<---------------cut here---------------end--------------->8---

FWIW, the idiomatic way in Guix is to use `(or native-inputs inputs)`
instead of that if.

HTH,
-- 
Josselin Poiret

Ludovic Courtès wrote on 9 Sep 00:03 +0200

Recipients:(name . Hilton Chain)(address . hako@ultrarare.space)

Message-ID:87msxw1fw6.fsf@gnu.org

Hi,

Hilton Chain <hako@ultrarare.space> skribis:

Toggle quote (16 lines)>> Hilton Chain <hako@ultrarare.space> skribis:
>>
>> > * guix/packages.scm (add-input-label): Specify output when it's not "out".
>>
>> [...]
>>
>> > +         (list (string-append (package-name package) ":" output)
>> > +               package
>> > +               output)))
>>
>> The Grand Plan¹ is to eventually get rid of labels entirely (or almost:
>> there’d still be input alists on the build side).  As such, I thought we
>> shouldn’t worry too much about what the actual label is.  But perhaps
>> you stumbled upon situations where this is a problem?  Could you
>> describe them?

[...]

Toggle quote (7 lines)

> My main concern is that currently modify-inputs, this-package-input

> and this-package-native-input operate on input labels and there would

> be duplicated labels if adding multiple outputs of a package.

> For modify-inputs, I think there's no approach to solve this without

> also specifying labels in inputs.

Yes, good point.

Another, more radical approach, would be to change semantics, whereby

(inputs (list p)) would mean that all the outputs of ‘p’, not just

“out”, are taken as inputs. That’d simplify inputs at the expense of

precision, and (this-package-input NAME) would always be unambiguous.

But maybe that’s too radical and uncertain.

So all things considered, I guess you’re right and we should do what you

propose.

Minor issues:

Toggle quote (15 lines)> --- a/guix/packages.scm
> +++ b/guix/packages.scm
> @@ -626,7 +626,13 @@ (define (add-input-label input)
>      ((? package? package)
>       (list (package-name package) package))
>      (((? package? package) output)                ;XXX: ugly?
> -     (list (package-name package) package output))
> +     (if (string=? output "out")
> +         ;; (package "out") => ("package" package "out")
> +         (list (package-name package) package output)
> +         ;; (package "output") => ("package:output" package "output")
> +         (list (string-append (package-name package) ":" output)
> +               package
> +               output)))

Rather write it as two separate clauses, without comments:

(((? package? package) "out")

…)

(((? package? package) output)

…)

Could you also add a test case in ‘tests/packages.scm’ that would look

up inputs by those labels?

Thanks,

Ludo’.

Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 65062@debbugs.gnu.org

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date