| Message ID | 20230123133217.318-2-maxim.cournoyer@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Enable cross-compilation for the pyproject-build-system | expand |
Hi Maxim, Sorry, This week is still too busy for me with work to review this. Looking forward to reviewing future patches or these if still needed. all best, jgart
Hello, Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > +++ b/guix/packages.scm > @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context) […] > + (let ((builder-name (procedure-name (bag-build bag)))) > + (if (or (bag-target bag) > + (eq? 'pyproject-build builder-name)) > + (bag->cross-derivation bag) This one part is a showstopper to me, for two reasons: 1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s not guaranteed to return something useful). 2. Special-casing build systems here is not okay: the bag and build system abstractions exist to maintain separation of concerns. I understand there’s an actual bug to fix and the desire to fix a more common issue, but I think this one approach is not the way forward. I hope that makes sense! Ludo’.
Hi Ludo,
> I understand there’s an actual bug to fix ...
Is the bug you are referencing exist as an open ticket somewhere on our tracker?
Just trying to get more context on that if it does.
all best,
jgart
> Is the bug you are referencing exist as an open ticket somewhere on our tracker?
Oooops, Ignore, found it: https://issues.guix.gnu.org/issue/25235
Reread Maxim's original message now ;()
Hi Ludo! Ludovic Courtès <ludo@gnu.org> writes: > Hello, > > Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > >> +++ b/guix/packages.scm >> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context) > > […] > >> + (let ((builder-name (procedure-name (bag-build bag)))) >> + (if (or (bag-target bag) >> + (eq? 'pyproject-build builder-name)) >> + (bag->cross-derivation bag) > > This one part is a showstopper to me, for two reasons: > > 1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s > not guaranteed to return something useful). > > 2. Special-casing build systems here is not okay: the bag and build > system abstractions exist to maintain separation of concerns. > > I understand there’s an actual bug to fix and the desire to fix a more > common issue, but I think this one approach is not the way forward. > > I hope that makes sense! I agree this is not "pretty", but it would be a "temporary" kludge until all the build systems can be migrated (and the package adjusted for) the "new" way, which is: native-inputs and inputs always co-exist, whether the build is a native one or a cross one. In light of this, it seems OK to test the water with a not so significant build system (only a handful of package relies on pyproject-build-system thus far). When all the build systems will have been migrated to the new way (a too big undertaking to be done in one shot), this kludge can be removed. Otherwise, could you offer a concrete suggestion as the way forward? I appreciate the "that's not the way", but stopping short of suggesting a better alternative leaves me wanting more :-).
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > Hi Ludo! > > Ludovic Courtès <ludo@gnu.org> writes: > >> Hello, >> >> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: >> >>> +++ b/guix/packages.scm >>> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context) >> >> […] >> >>> + (let ((builder-name (procedure-name (bag-build bag)))) >>> + (if (or (bag-target bag) >>> + (eq? 'pyproject-build builder-name)) >>> + (bag->cross-derivation bag) >> >> This one part is a showstopper to me, for two reasons: >> >> 1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s >> not guaranteed to return something useful). >> >> 2. Special-casing build systems here is not okay: the bag and build >> system abstractions exist to maintain separation of concerns. >> >> I understand there’s an actual bug to fix and the desire to fix a more >> common issue, but I think this one approach is not the way forward. >> >> I hope that makes sense! > > I agree this is not "pretty", but it would be a "temporary" kludge until > all the build systems can be migrated (and the package adjusted for) the > "new" way, which is: native-inputs and inputs always co-exist, whether > the build is a native one or a cross one. > > In light of this, it seems OK to test the water with a not so > significant build system (only a handful of package relies on > pyproject-build-system thus far). When all the build systems will have > been migrated to the new way (a too big undertaking to be done in one > shot), this kludge can be removed. > > Otherwise, could you offer a concrete suggestion as the way forward? I > appreciate the "that's not the way", but stopping short of suggesting a > better alternative leaves me wanting more :-). I think it would be clearer to see other potential ways forward if the end goal was more clearly articulated. Guessing at that here from the changes proposed to guix/packages.scm, once all build systems are adjusted, cross derivations will be produced for all bags, regardless of whether there's a target. That doesn't make much sense to me. One explaination is that the current naming is confusing when thinking about this goal, so maybe bag->cross-derivation happens to do what you want it to do in all circumstances, even when target is #f?
Hi Christopher, Christopher Baines <mail@cbaines.net> writes: > Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > >> Hi Ludo! >> >> Ludovic Courtès <ludo@gnu.org> writes: >> >>> Hello, >>> >>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: >>> >>>> +++ b/guix/packages.scm >>>> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context) >>> >>> […] >>> >>>> + (let ((builder-name (procedure-name (bag-build bag)))) >>>> + (if (or (bag-target bag) >>>> + (eq? 'pyproject-build builder-name)) >>>> + (bag->cross-derivation bag) >>> >>> This one part is a showstopper to me, for two reasons: >>> >>> 1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s >>> not guaranteed to return something useful). >>> >>> 2. Special-casing build systems here is not okay: the bag and build >>> system abstractions exist to maintain separation of concerns. >>> >>> I understand there’s an actual bug to fix and the desire to fix a more >>> common issue, but I think this one approach is not the way forward. >>> >>> I hope that makes sense! >> >> I agree this is not "pretty", but it would be a "temporary" kludge until >> all the build systems can be migrated (and the package adjusted for) the >> "new" way, which is: native-inputs and inputs always co-exist, whether >> the build is a native one or a cross one. >> >> In light of this, it seems OK to test the water with a not so >> significant build system (only a handful of package relies on >> pyproject-build-system thus far). When all the build systems will have >> been migrated to the new way (a too big undertaking to be done in one >> shot), this kludge can be removed. >> >> Otherwise, could you offer a concrete suggestion as the way forward? I >> appreciate the "that's not the way", but stopping short of suggesting a >> better alternative leaves me wanting more :-). > > I think it would be clearer to see other potential ways forward if the > end goal was more clearly articulated. > > Guessing at that here from the changes proposed to guix/packages.scm, > once all build systems are adjusted, cross derivations will be produced > for all bags, regardless of whether there's a target. > > That doesn't make much sense to me. One explaination is that the current > naming is confusing when thinking about this goal, so maybe > bag->cross-derivation happens to do what you want it to do in all > circumstances, even when target is #f? Thanks for tipping in. The end goal is to avoid loosing the information of which inputs are native (build inputs) vs regular in the bag, and yes bag->cross-derivation allows that. It appears to me the distinction in the bag representations (native vs cross) was originally perceived useful as some kind of optimization (there's less variables to worry about, and we can squash the inputs/search-paths together, since they're all native anyway), but this information (currently discarded) ends up being very useful even on the build side (to wrap only the target inputs, say, and not all the native/build inputs). So yes, the change long term would be to integrate the bag->cross-derivation logic into bag->derivation, at which point it would be unified for any type of build (the bag representation would be shared between native and cross builds). When authoring packages, we'd no longer see the odd idiom '(or native-inputs inputs)' which results from this implementation detail; instead we'd always have access to both 'native-inputs' and 'inputs'. I hope that helps understanding the rationale.
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > Hi Christopher, > > Christopher Baines <mail@cbaines.net> writes: > >> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: >> >>> Hi Ludo! >>> >>> Ludovic Courtès <ludo@gnu.org> writes: >>> >>>> Hello, >>>> >>>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: >>>> >>>>> +++ b/guix/packages.scm >>>>> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context) >>>> >>>> […] >>>> >>>>> + (let ((builder-name (procedure-name (bag-build bag)))) >>>>> + (if (or (bag-target bag) >>>>> + (eq? 'pyproject-build builder-name)) >>>>> + (bag->cross-derivation bag) >>>> >>>> This one part is a showstopper to me, for two reasons: >>>> >>>> 1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s >>>> not guaranteed to return something useful). >>>> >>>> 2. Special-casing build systems here is not okay: the bag and build >>>> system abstractions exist to maintain separation of concerns. >>>> >>>> I understand there’s an actual bug to fix and the desire to fix a more >>>> common issue, but I think this one approach is not the way forward. >>>> >>>> I hope that makes sense! >>> >>> I agree this is not "pretty", but it would be a "temporary" kludge until >>> all the build systems can be migrated (and the package adjusted for) the >>> "new" way, which is: native-inputs and inputs always co-exist, whether >>> the build is a native one or a cross one. >>> >>> In light of this, it seems OK to test the water with a not so >>> significant build system (only a handful of package relies on >>> pyproject-build-system thus far). When all the build systems will have >>> been migrated to the new way (a too big undertaking to be done in one >>> shot), this kludge can be removed. >>> >>> Otherwise, could you offer a concrete suggestion as the way forward? I >>> appreciate the "that's not the way", but stopping short of suggesting a >>> better alternative leaves me wanting more :-). >> >> I think it would be clearer to see other potential ways forward if the >> end goal was more clearly articulated. >> >> Guessing at that here from the changes proposed to guix/packages.scm, >> once all build systems are adjusted, cross derivations will be produced >> for all bags, regardless of whether there's a target. >> >> That doesn't make much sense to me. One explaination is that the current >> naming is confusing when thinking about this goal, so maybe >> bag->cross-derivation happens to do what you want it to do in all >> circumstances, even when target is #f? > > Thanks for tipping in. The end goal is to avoid loosing the information > of which inputs are native (build inputs) vs regular in the bag, and yes > bag->cross-derivation allows that. It appears to me the distinction in > the bag representations (native vs cross) was originally perceived > useful as some kind of optimization (there's less variables to worry > about, and we can squash the inputs/search-paths together, since they're > all native anyway), but this information (currently discarded) ends up > being very useful even on the build side (to wrap only the target > inputs, say, and not all the native/build inputs). > > So yes, the change long term would be to integrate the > bag->cross-derivation logic into bag->derivation, at which point it > would be unified for any type of build (the bag representation would be > shared between native and cross builds). Thanks for the explanation, so maybe an alternative to trying to get bag->derivation to function differently for different build systems would be to push combining the inputs down in to each build system. Take the gnu-build-system as an example, gnu-build in (guix build-system gnu) would be changed to take multiple lists of inputs, rather than a single list. It can then combine the lists of inputs as is done in bag->derivation, to avoid affecting any packages. While this does require changing all the build systems, I think it's a bit more forward thinking compared to trying to add a kludge in to bag->derivation, since hopefully the change there can be the longer term one. Does that make sense?
Hi Maxim, Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > Ludovic Courtès <ludo@gnu.org> writes: > >> Hello, >> >> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: >> >>> +++ b/guix/packages.scm >>> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context) >> >> […] >> >>> + (let ((builder-name (procedure-name (bag-build bag)))) >>> + (if (or (bag-target bag) >>> + (eq? 'pyproject-build builder-name)) >>> + (bag->cross-derivation bag) >> >> This one part is a showstopper to me, for two reasons: >> >> 1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s >> not guaranteed to return something useful). >> >> 2. Special-casing build systems here is not okay: the bag and build >> system abstractions exist to maintain separation of concerns. >> >> I understand there’s an actual bug to fix and the desire to fix a more >> common issue, but I think this one approach is not the way forward. >> >> I hope that makes sense! > > I agree this is not "pretty", but it would be a "temporary" kludge until To make sure there’s no misunderstanding, I think that’s not okay even as a “temporary kludge” (I’m not against temporary kludges in general, I’ve done my share of that ;-), but this would be way too brittle.) > all the build systems can be migrated (and the package adjusted for) the > "new" way, which is: native-inputs and inputs always co-exist, whether > the build is a native one or a cross one. Again, I’m not sure about the “new way”. I’m sorry I lack the bandwidth to review this quickly, especially a wide-ranging change like inputs/native-inputs. (Just yesterday I was fixing cross-compilation issues on ‘core-updates’; that gives a lot of insight as to how native-inputs/inputs play out in practice.) [...] > Otherwise, could you offer a concrete suggestion as the way forward? I > appreciate the "that's not the way", but stopping short of suggesting a > better alternative leaves me wanting more :-). Yup, I’m sorry I don’t have any suggestions! Perhaps one suggestion would be: start the native-inputs/inputs change on a new branch, for all the build systems (or at least the main ones), and then try to build the ‘core’ subset (which includes cross-compilation) to get an idea of the kind of code simplification opportunities as well as issues that arise. I feel like it’s hard to anticipate the impact of such a change without actually trying. Thanks, Ludo’.
Hello Christopher, Christopher Baines <mail@cbaines.net> writes: [...] >> Thanks for tipping in. The end goal is to avoid loosing the information >> of which inputs are native (build inputs) vs regular in the bag, and yes >> bag->cross-derivation allows that. It appears to me the distinction in >> the bag representations (native vs cross) was originally perceived >> useful as some kind of optimization (there's less variables to worry >> about, and we can squash the inputs/search-paths together, since they're >> all native anyway), but this information (currently discarded) ends up >> being very useful even on the build side (to wrap only the target >> inputs, say, and not all the native/build inputs). >> >> So yes, the change long term would be to integrate the >> bag->cross-derivation logic into bag->derivation, at which point it >> would be unified for any type of build (the bag representation would be >> shared between native and cross builds). > > Thanks for the explanation, so maybe an alternative to trying to get > bag->derivation to function differently for different build systems > would be to push combining the inputs down in to each build system. > > Take the gnu-build-system as an example, gnu-build in (guix build-system > gnu) would be changed to take multiple lists of inputs, rather than a > single list. It can then combine the lists of inputs as is done in > bag->derivation, to avoid affecting any packages. > > While this does require changing all the build systems, I think it's a > bit more forward thinking compared to trying to add a kludge in to > bag->derivation, since hopefully the change there can be the longer term > one. I'll see if I have a good enough understanding of the code to make sense of your suggestion. I'll definitely try it! Thanks for suggesting.
Hi Ludovic, Ludovic Courtès <ludo@gnu.org> writes: > Hi Maxim, > > Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > >> Ludovic Courtès <ludo@gnu.org> writes: >> >>> Hello, >>> >>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: >>> >>>> +++ b/guix/packages.scm >>>> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context) >>> >>> […] >>> >>>> + (let ((builder-name (procedure-name (bag-build bag)))) >>>> + (if (or (bag-target bag) >>>> + (eq? 'pyproject-build builder-name)) >>>> + (bag->cross-derivation bag) >>> >>> This one part is a showstopper to me, for two reasons: >>> >>> 1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s >>> not guaranteed to return something useful). >>> >>> 2. Special-casing build systems here is not okay: the bag and build >>> system abstractions exist to maintain separation of concerns. >>> >>> I understand there’s an actual bug to fix and the desire to fix a more >>> common issue, but I think this one approach is not the way forward. >>> >>> I hope that makes sense! >> >> I agree this is not "pretty", but it would be a "temporary" kludge until > > To make sure there’s no misunderstanding, I think that’s not okay even > as a “temporary kludge” (I’m not against temporary kludges in general, > I’ve done my share of that ;-), but this would be way too brittle.) "way too brittle" is a strong warning :-). I don't understand what is so brittle about it? It's not like we change the build system names often. >> all the build systems can be migrated (and the package adjusted for) the >> "new" way, which is: native-inputs and inputs always co-exist, whether >> the build is a native one or a cross one. > > Again, I’m not sure about the “new way”. I’m sorry I lack the bandwidth > to review this quickly, especially a wide-ranging change like > inputs/native-inputs. > > (Just yesterday I was fixing cross-compilation issues on ‘core-updates’; > that gives a lot of insight as to how native-inputs/inputs play out in > practice.) > > [...] > >> Otherwise, could you offer a concrete suggestion as the way forward? I >> appreciate the "that's not the way", but stopping short of suggesting a >> better alternative leaves me wanting more :-). > > Yup, I’m sorry I don’t have any suggestions! Thanks, it's useful to state that up front. Otherwise the other side of the line goes like "... so?" :-). > Perhaps one suggestion would be: start the native-inputs/inputs change > on a new branch, for all the build systems (or at least the main > ones), and then try to build the ‘core’ subset (which includes > cross-compilation) to get an idea of the kind of code simplification > opportunities as well as issues that arise. I feel like it’s hard to > anticipate the impact of such a change without actually trying. I can do this; the exercise would be similar to what I've done for pyproject-build-system, where I was able to build complex packages (both natively and cross-compiled) with the new same-bag representation change, but to a larger scale. It's more work, which is why I would have preferred to contain its scope to get started, but the changes were easy, IIRC. Basically, it forces us to review all the (assoc-ref inputs "package-name") or (search-input-file inputs "file") procedure calls and remove the assumption that native-inputs are also inputs for native builds (that'll fix multiple cross-compilation bugs at the same time, so it's a good thing to do anyway). To be continued!
Hi Maxim, Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: [...] >>>>> + (let ((builder-name (procedure-name (bag-build bag)))) >>>>> + (if (or (bag-target bag) >>>>> + (eq? 'pyproject-build builder-name)) >>>>> + (bag->cross-derivation bag) >>>> >>>> This one part is a showstopper to me, for two reasons: >>>> >>>> 1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s >>>> not guaranteed to return something useful). >>>> >>>> 2. Special-casing build systems here is not okay: the bag and build >>>> system abstractions exist to maintain separation of concerns. >>>> >>>> I understand there’s an actual bug to fix and the desire to fix a more >>>> common issue, but I think this one approach is not the way forward. >>>> >>>> I hope that makes sense! >>> >>> I agree this is not "pretty", but it would be a "temporary" kludge until >> >> To make sure there’s no misunderstanding, I think that’s not okay even >> as a “temporary kludge” (I’m not against temporary kludges in general, >> I’ve done my share of that ;-), but this would be way too brittle.) > > "way too brittle" is a strong warning :-). It is; I’m objecting to this change. > I don't understand what is > so brittle about it? It's not like we change the build system names > often. I have little to add to what I wrote above: procedure names don’t mean anything, especially with higher-order functions, and separation of concerns is an important design principle. I’m speaking with 15+ years of experience with Guile; I don’t mind being challenged, it’s healthy, but I think we also need to recognize the expertise of each other and encounter each other halfway. [...] >> Perhaps one suggestion would be: start the native-inputs/inputs change >> on a new branch, for all the build systems (or at least the main >> ones), and then try to build the ‘core’ subset (which includes >> cross-compilation) to get an idea of the kind of code simplification >> opportunities as well as issues that arise. I feel like it’s hard to >> anticipate the impact of such a change without actually trying. > > I can do this; the exercise would be similar to what I've done for > pyproject-build-system, where I was able to build complex packages (both > natively and cross-compiled) with the new same-bag representation > change, but to a larger scale. It's more work, which is why I would > have preferred to contain its scope to get started, but the changes were > easy, IIRC. Yes, I agree it’s more work, but it’s dissimilar in that you’ll be confronted with tons of weird cases, some of which we can already anticipate, but probably others we’ll probably be surprised to discover. > Basically, it forces us to review all the (assoc-ref inputs > "package-name") or (search-input-file inputs "file") procedure calls and > remove the assumption that native-inputs are also inputs for native > builds (that'll fix multiple cross-compilation bugs at the same time, so > it's a good thing to do anyway). I do hope it’ll be this simple, but I just can’t tell. Thank you, Ludo’.
Hi, Ludovic Courtès <ludo@gnu.org> writes: > Hi Maxim, > > Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > > [...] > >>>>>> + (let ((builder-name (procedure-name (bag-build bag)))) >>>>>> + (if (or (bag-target bag) >>>>>> + (eq? 'pyproject-build builder-name)) >>>>>> + (bag->cross-derivation bag) >>>>> >>>>> This one part is a showstopper to me, for two reasons: >>>>> >>>>> 1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s >>>>> not guaranteed to return something useful). >>>>> >>>>> 2. Special-casing build systems here is not okay: the bag and build >>>>> system abstractions exist to maintain separation of concerns. >>>>> >>>>> I understand there’s an actual bug to fix and the desire to fix a more >>>>> common issue, but I think this one approach is not the way forward. >>>>> >>>>> I hope that makes sense! >>>> >>>> I agree this is not "pretty", but it would be a "temporary" kludge until >>> >>> To make sure there’s no misunderstanding, I think that’s not okay even >>> as a “temporary kludge” (I’m not against temporary kludges in general, >>> I’ve done my share of that ;-), but this would be way too brittle.) >> >> "way too brittle" is a strong warning :-). > > It is; I’m objecting to this change. OK. >> I don't understand what is >> so brittle about it? It's not like we change the build system names >> often. > > I have little to add to what I wrote above: procedure names don’t mean > anything, especially with higher-order functions, and separation of > concerns is an important design principle. > > I’m speaking with 15+ years of experience with Guile; I don’t mind being > challenged, it’s healthy, but I think we also need to recognize the > expertise of each other and encounter each other halfway. When I don't understand something, I ask for an explanation. I'm happy (and greatly value) that my correspondent has 15 years of experience with Guile, but I'll still ask if something is unclear. I hope you don't mind. > [...] > >>> Perhaps one suggestion would be: start the native-inputs/inputs change >>> on a new branch, for all the build systems (or at least the main >>> ones), and then try to build the ‘core’ subset (which includes >>> cross-compilation) to get an idea of the kind of code simplification >>> opportunities as well as issues that arise. I feel like it’s hard to >>> anticipate the impact of such a change without actually trying. >> >> I can do this; the exercise would be similar to what I've done for >> pyproject-build-system, where I was able to build complex packages (both >> natively and cross-compiled) with the new same-bag representation >> change, but to a larger scale. It's more work, which is why I would >> have preferred to contain its scope to get started, but the changes were >> easy, IIRC. > > Yes, I agree it’s more work, but it’s dissimilar in that you’ll be > confronted with tons of weird cases, some of which we can already > anticipate, but probably others we’ll probably be surprised to discover. OK. I'll put this to the test, when I'm done with the current Ruby rabbit-hole I've been pursuing. Thanks for the feedback!
Hi, Ludovic Courtès <ludo@gnu.org> writes: > Hi Maxim, > > Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > > [...] > >>>>>> + (let ((builder-name (procedure-name (bag-build bag)))) >>>>>> + (if (or (bag-target bag) >>>>>> + (eq? 'pyproject-build builder-name)) >>>>>> + (bag->cross-derivation bag) >>>>> >>>>> This one part is a showstopper to me, for two reasons: >>>>> >>>>> 1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s >>>>> not guaranteed to return something useful). >>>>> >>>>> 2. Special-casing build systems here is not okay: the bag and build >>>>> system abstractions exist to maintain separation of concerns. >>>>> >>>>> I understand there’s an actual bug to fix and the desire to fix a more >>>>> common issue, but I think this one approach is not the way forward. >>>>> >>>>> I hope that makes sense! >>>> >>>> I agree this is not "pretty", but it would be a "temporary" kludge until >>> >>> To make sure there’s no misunderstanding, I think that’s not okay even >>> as a “temporary kludge” (I’m not against temporary kludges in general, >>> I’ve done my share of that ;-), but this would be way too brittle.) >> >> "way too brittle" is a strong warning :-). > > It is; I’m objecting to this change. OK. >> I don't understand what is >> so brittle about it? It's not like we change the build system names >> often. > > I have little to add to what I wrote above: procedure names don’t mean > anything, especially with higher-order functions, and separation of > concerns is an important design principle. > > I’m speaking with 15+ years of experience with Guile; I don’t mind being > challenged, it’s healthy, but I think we also need to recognize the > expertise of each other and encounter each other halfway. When I don't understand something, I ask for an explanation. I'm happy (and greatly value) that my correspondent has 15 years of experience with Guile, but I'll still ask if something is unclear. I hope you don't mind. > [...] > >>> Perhaps one suggestion would be: start the native-inputs/inputs change >>> on a new branch, for all the build systems (or at least the main >>> ones), and then try to build the ‘core’ subset (which includes >>> cross-compilation) to get an idea of the kind of code simplification >>> opportunities as well as issues that arise. I feel like it’s hard to >>> anticipate the impact of such a change without actually trying. >> >> I can do this; the exercise would be similar to what I've done for >> pyproject-build-system, where I was able to build complex packages (both >> natively and cross-compiled) with the new same-bag representation >> change, but to a larger scale. It's more work, which is why I would >> have preferred to contain its scope to get started, but the changes were >> easy, IIRC. > > Yes, I agree it’s more work, but it’s dissimilar in that you’ll be > confronted with tons of weird cases, some of which we can already > anticipate, but probably others we’ll probably be surprised to discover. OK. I'll put this to the test, when I'm done with the current Ruby rabbit-hole I've been pursuing. Thanks for the feedback!
diff --git a/guix/build-system/pyproject.scm b/guix/build-system/pyproject.scm index 8f3b562ca3..74d739023f 100644 --- a/guix/build-system/pyproject.scm +++ b/guix/build-system/pyproject.scm @@ -1,6 +1,7 @@ ;;; GNU Guix --- Functional package management for GNU ;;; Copyright © 2021 Lars-Dominik Braun <lars@6xq.net> ;;; Copyright © 2022 Marius Bakke <marius@gnu.org> +;;; Copyright © 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com> ;;; ;;; This file is part of GNU Guix. ;;; @@ -69,28 +70,37 @@ (define* (lower name #:rest arguments) "Return a bag for NAME." (define private-keywords - '(#:target #:python #:inputs #:native-inputs)) - - (and (not target) ;XXX: no cross-compilation - (bag - (name name) - (system system) - (host-inputs `(,@(if source - `(("source" ,source)) - '()) - ,@inputs - - ;; Keep the standard inputs of 'gnu-build-system'. - ,@(standard-packages))) - (build-inputs `(("python" ,python) - ("sanity-check.py" ,(local-file sanity-check.py)) - ,@native-inputs)) - (outputs (append outputs '(wheel))) - (build pyproject-build) - (arguments (strip-keyword-arguments private-keywords arguments))))) - -(define* (pyproject-build name inputs - #:key source + `(#:python #:inputs #:native-inputs + ,@(if target '() '(#:target)))) + + (bag + (name name) + (system system) + (target target) + (build-inputs `(,@(if source + `(("source" ,source)) + '()) + ("python" ,python) + ("sanity-check.py" ,(local-file sanity-check.py)) + ,@native-inputs + ,@(if target + (standard-cross-packages target 'host) + '()) + ;; Keep the standard inputs of 'gnu-build-system'. + ,@(standard-packages))) + (host-inputs inputs) + (target-inputs (if target + (standard-cross-packages target 'target) + '())) + (outputs (append outputs '(wheel))) + (build pyproject-build) + (arguments (strip-keyword-arguments private-keywords arguments)))) + +(define* (pyproject-build name + #:key + target + build-inputs target-inputs host-inputs + source (tests? #t) (configure-flags ''()) (build-backend #f) @@ -98,44 +108,63 @@ (define* (pyproject-build name inputs (test-flags ''()) (phases '%standard-phases) (outputs '("out" "wheel")) + (native-search-paths '()) (search-paths '()) (system (%current-system)) (guile #f) (imported-modules %pyproject-build-system-modules) (modules '((guix build pyproject-build-system) (guix build utils)))) - "Build SOURCE using PYTHON, and with INPUTS." + "Build SOURCE using PYTHON, and with BUILD-INPUTS, HOST-INPUTS and +TARGET-INPUTS (if available)." (define build (with-imported-modules imported-modules #~(begin (use-modules #$@(sexp->gexp modules)) - #$(with-build-variables inputs outputs - #~(pyproject-build - #:name #$name - #:source #+source - #:configure-flags #$configure-flags - #:system #$system - #:build-backend #$build-backend - #:test-backend #$test-backend - #:test-flags #$test-flags - #:tests? #$tests? - #:phases #$(if (pair? phases) - (sexp->gexp phases) - phases) - #:outputs %outputs - #:search-paths '#$(sexp->gexp - (map search-path-specification->sexp - search-paths)) - #:inputs %build-inputs))))) + (define %build-host-inputs + #+(input-tuples->gexp build-inputs)) + + (define %build-target-inputs + (append #$(input-tuples->gexp host-inputs) + #+(input-tuples->gexp target-inputs))) + + (define %build-inputs + (append %build-host-inputs %build-target-inputs)) + + (define %outputs + #$(outputs->gexp outputs)) + (pyproject-build #:name #$name + #:source #+source + #:configure-flags #$configure-flags + #:system #$system + #:target #$target + #:build-backend #$build-backend + #:test-backend #$test-backend + #:test-flags #$test-flags + #:tests? #$tests? + #:phases #$(if (pair? phases) + (sexp->gexp phases) + phases) + #:outputs %outputs + #:search-paths + '#$(sexp->gexp + (map search-path-specification->sexp + search-paths)) + #:native-search-paths + '#$(sexp->gexp + (map search-path-specification->sexp + native-search-paths)) + #:native-inputs %build-host-inputs + #:inputs %build-target-inputs)))) (mlet %store-monad ((guile (package->derivation (or guile (default-guile)) system #:graft? #f))) (gexp->derivation name build #:system system - #:graft? #f ;consistent with 'gnu-build' - #:target #f + #:target target + #:graft? #f ;consistent with 'gnu-build' #:guile-for-build guile))) (define pyproject-build-system diff --git a/guix/build/pyproject-build-system.scm b/guix/build/pyproject-build-system.scm index c69ccc9d64..e51b5cfc43 100644 --- a/guix/build/pyproject-build-system.scm +++ b/guix/build/pyproject-build-system.scm @@ -86,6 +86,58 @@ (define-condition-type &cannot-extract-multiple-wheels &python-build-error ;; Raised, when no wheel has been built by the build system. (define-condition-type &no-wheels-built &python-build-error no-wheels-built?) +;;; XXX: This is the same as in (guix build gnu-build-system), except adjusted +;;; for the fact that native-inputs always exist now, whether cross-compiling +;;; or not. When not cross-compiling, input-directories are appended to +;;; native-input-directories to so that native-search-paths are computed for +;;; all inputs. +(define* (set-paths #:key target inputs native-inputs + search-paths native-search-paths + #:allow-other-keys) + (define input-directories + ;; The "source" input can be a directory, but we don't want it for search + ;; paths. See <https://issues.guix.gnu.org/44924>. + (match (alist-delete "source" inputs) + (((_ . dir) ...) + dir))) + + (define native-input-directories + (match (alist-delete "source" native-inputs) + (((_ . dir) ...) + dir))) + + ;; Tell 'ld-wrapper' to disallow non-store libraries. + (setenv "GUIX_LD_WRAPPER_ALLOW_IMPURITIES" "no") + + ;; When cross building, $PATH must refer only to native (host) inputs since + ;; target inputs are not executable. + (set-path-environment-variable "PATH" '("bin" "sbin") + (append native-input-directories + (if target + '() + input-directories))) + + (for-each (match-lambda + ((env-var (files ...) separator type pattern) + (set-path-environment-variable env-var files + input-directories + #:separator separator + #:type type + #:pattern pattern))) + search-paths) + + (for-each (match-lambda + ((env-var (files ...) separator type pattern) + (set-path-environment-variable env-var files + (append native-input-directories + (if target + '() + input-directories)) + #:separator separator + #:type type + #:pattern pattern))) + native-search-paths)) + (define* (build #:key outputs build-backend configure-flags #:allow-other-keys) "Build a given Python package." @@ -135,9 +187,9 @@ (define (pyproject.toml->build-backend file) wheel-dir config-settings))) -(define* (check #:key tests? test-backend test-flags #:allow-other-keys) +(define* (check #:key target tests? test-backend test-flags #:allow-other-keys) "Run the test suite of a given Python package." - (if tests? + (if (and tests? (not target)) ;; Unfortunately with PEP 517 there is no common method to specify test ;; systems. Guess test system based on inputs instead. (let* ((pytest (which "pytest")) @@ -172,11 +224,11 @@ (define* (check #:key tests? test-backend test-flags #:allow-other-keys) (else (raise (condition (&test-system-not-found)))))) (format #t "test suite not run~%"))) -(define* (install #:key inputs outputs #:allow-other-keys) +(define* (install #:key native-inputs outputs #:allow-other-keys) "Install a wheel file according to PEP 427" ;; See https://www.python.org/dev/peps/pep-0427/#installing-a-wheel-distribution-1-0-py32-none-any-whl - (let ((site-dir (site-packages inputs outputs)) - (python (assoc-ref inputs "python")) + (let ((site-dir (site-packages native-inputs outputs)) + (python (assoc-ref native-inputs "python")) (out (assoc-ref outputs "out"))) (define (extract file) "Extract wheel (ZIP file) into site-packages directory" @@ -262,10 +314,10 @@ (define (list-directories base predicate) (expand-data-directory directory) (rmdir directory)) datadirs)))) -(define* (compile-bytecode #:key inputs outputs #:allow-other-keys) +(define* (compile-bytecode #:key native-inputs outputs #:allow-other-keys) "Compile installed byte-code in site-packages." - (let* ((site-dir (site-packages inputs outputs)) - (python (assoc-ref inputs "python")) + (let* ((site-dir (site-packages native-inputs outputs)) + (python (assoc-ref native-inputs "python")) (major-minor (map string->number (take (string-split (python-version python) #\.) 2))) (<3.7? (match major-minor @@ -281,7 +333,7 @@ (define* (compile-bytecode #:key inputs outputs #:allow-other-keys) (invoke "python" "-m" "compileall" "--invalidation-mode=unchecked-hash" site-dir)))) -(define* (create-entrypoints #:key inputs outputs #:allow-other-keys) +(define* (create-entrypoints #:key native-inputs outputs #:allow-other-keys) "Implement Entry Points Specification (https://packaging.python.org/specifications/entry-points/) by PyPa, which creates runnable scripts in bin/ from entry point specification @@ -337,8 +389,7 @@ (define (create-script path name module function) import ~a as mod sys.exit (mod.~a ())~%" interpreter module function))) (chmod file-path #o755))) - - (let* ((site-dir (site-packages inputs outputs)) + (let* ((site-dir (site-packages native-inputs outputs)) (out (assoc-ref outputs "out")) (bin-dir (string-append out "/bin")) (entry-point-files (find-files site-dir "^entry_points.txt$"))) @@ -358,6 +409,51 @@ (define* (set-SOURCE-DATE-EPOCH* #:rest _) ;; not support timestamps before 1980. (setenv "SOURCE_DATE_EPOCH" "315619200")) +(define* (set-setuptools-env #:key target #:allow-other-keys) + "Set environment variables such as LDSHARED, LDXXSHARED, etc. used by +setuptools when building native extensions. This is particularly useful for +cross-compilation." + (define cc-for-target (if target + (string-append target "-gcc") + "gcc")) + (define cxx-for-target (if target + (string-append target "-g++") + "g++")) + ;; The variables defined here are taken from CPython's configure.ac file. + ;; The explanations are those of the Poky (from Yocto) project. + (setenv "CC" cc-for-target) + (setenv "CXX" cxx-for-target) + ;; LDSHARED is the ld *command* used to create shared library. + (setenv "LDSHARED" (string-append cc-for-target " -shared")) + ;; LDXXSHARED is the ld *command* used to create shared library of C++ + ;; objects. + (setenv "LDCXXSHARED" (string-append cxx-for-target " -shared")) + ;; CCSHARED are the C *flags* used to create objects to go into a shared + ;; library (module). + (setenv "CCSHARED" "-fPIC") + ;; LINKFORSHARED are the flags passed to the $(CC) command that links the + ;; python executable. + (setenv "LINKFORSHARED" "-Xlinker -export-dynamic")) + +(define* (sanity-check #:key tests? native-inputs outputs #:allow-other-keys + #:rest args) + (apply (assoc-ref python:%standard-phases 'sanity-check) + (append args (list #:inputs native-inputs)))) + +(define* (add-install-to-pythonpath #:key native-inputs outputs + #:allow-other-keys) + "A phase that just wraps the 'add-installed-pythonpath' procedure." + (add-installed-pythonpath native-inputs outputs)) + +(define* (wrap #:key native-inputs outputs #:allow-other-keys #:rest args) + (apply (assoc-ref python:%standard-phases 'wrap) + (append args (list #:inputs native-inputs)))) + +(define* (rename-pth-file #:key name native-inputs outputs #:allow-other-keys + #:rest args) + (apply (assoc-ref python:%standard-phases 'rename-pth-file) + (append args (list #:inputs native-inputs)))) + (define %standard-phases ;; The build phase only builds C extensions and copies the Python sources, ;; while the install phase copies then byte-compiles the sources to the @@ -365,13 +461,19 @@ (define %standard-phases ;; to ease testing the built package. (modify-phases python:%standard-phases (replace 'set-SOURCE-DATE-EPOCH set-SOURCE-DATE-EPOCH*) + (replace 'set-paths set-paths) + (add-after 'set-paths 'set-setuptools-env set-setuptools-env) (replace 'build build) (replace 'install install) + (replace 'add-install-to-pythonpath add-install-to-pythonpath) + (replace 'wrap wrap) (delete 'check) ;; Must be before tests, so they can use installed packages’ entry points. (add-before 'wrap 'create-entrypoints create-entrypoints) (add-after 'wrap 'check check) - (add-before 'check 'compile-bytecode compile-bytecode))) + (add-before 'check 'compile-bytecode compile-bytecode) + (replace 'sanity-check sanity-check) + (replace 'rename-pth-file rename-pth-file))) (define* (pyproject-build #:key inputs (phases %standard-phases) #:allow-other-keys #:rest args) diff --git a/guix/packages.scm b/guix/packages.scm index 041a872f9d..6d7df17fc3 100644 --- a/guix/packages.scm +++ b/guix/packages.scm @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context) "Return the derivation to build BAG for SYSTEM. Optionally, CONTEXT can be a package object describing the context in which the call occurs, for improved error reporting." - (if (bag-target bag) - (bag->cross-derivation bag) - (mlet* %store-monad ((system -> (bag-system bag)) - (inputs -> (bag-transitive-inputs bag)) - (input-drvs (mapm %store-monad - (cut expand-input context <> system) - inputs)) - (paths -> (delete-duplicates - (append-map (match-lambda - ((_ (? package? p) _ ...) - (package-native-search-paths - p)) - (_ '())) - inputs)))) - ;; It's possible that INPUTS contains packages that are not 'eq?' but - ;; that lead to the same derivation. Delete those duplicates to avoid - ;; issues down the road, such as duplicate entries in '%build-inputs'. - (apply (bag-build bag) (bag-name bag) - (delete-duplicates input-drvs input=?) - #:search-paths paths - #:outputs (bag-outputs bag) #:system system - (bag-arguments bag))))) + (let ((builder-name (procedure-name (bag-build bag)))) + (if (or (bag-target bag) + (eq? 'pyproject-build builder-name)) + (bag->cross-derivation bag) + (mlet* %store-monad ((system -> (bag-system bag)) + (inputs -> (bag-transitive-inputs bag)) + (input-drvs (mapm %store-monad + (cut expand-input context <> system) + inputs)) + (paths -> (delete-duplicates + (append-map (match-lambda + ((_ (? package? p) _ ...) + (package-native-search-paths + p)) + (_ '())) + inputs)))) + ;; It's possible that INPUTS contains packages that are not 'eq?' but + ;; that lead to the same derivation. Delete those duplicates to avoid + ;; issues down the road, such as duplicate entries in '%build-inputs'. + (apply (bag-build bag) (bag-name bag) + (delete-duplicates input-drvs input=?) + #:search-paths paths + #:outputs (bag-outputs bag) #:system system + (bag-arguments bag)))))) (define* (bag->cross-derivation bag #:optional context) "Return the derivation to build BAG, which is actually a cross build.