| Message ID | 20231207133913.443-2-alex.kiernan@gmail.com |
|---|---|
| State | Accepted, archived |
| Commit | 740374a13ad5359767b421666decf50c158ea0df |
| Headers | show |
| Series | [1/3] cargo: Rename MANIFEST_PATH -> CARGO_MANIFEST_PATH | expand |
On Thu, 2023-12-07 at 13:39 +0000, Alex Kiernan wrote: > cargo_common_do_configure uses CARGO_MANIFEST_PATH (which depends on > CARGO_SRC_DIR), but their definition was in cargo.bbclass. > > Match the other variables here and change to default values, rather > than weak defaults. FWIW "single value" class variables tend to work much better as ??= (which I'd call default value) since than it doesn't matter if the recipe setting comes before or after the inherit and the inherit position in the recipe doesn't matter. I'd call ?= a weak assignment or weak default. Thanks for working on cleaning some of these things up. I got so far with it originally and then ran out of time (and too frustrated with the long build/test cycles!). Cheers, Richard > > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com> > --- > > meta/classes-recipe/cargo.bbclass | 7 ------- > meta/classes-recipe/cargo_common.bbclass | 7 +++++++ > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/meta/classes-recipe/cargo.bbclass b/meta/classes-recipe/cargo.bbclass > index 96a74e2ef1ec..0829a58dd90f 100644 > --- a/meta/classes-recipe/cargo.bbclass > +++ b/meta/classes-recipe/cargo.bbclass > @@ -30,13 +30,6 @@ B = "${WORKDIR}/build" > # where the issue occured > export RUST_BACKTRACE = "1" > > -# The directory of the Cargo.toml relative to the root directory, per default > -# assume there's a Cargo.toml directly in the root directory > -CARGO_SRC_DIR ??= "" > - > -# The actual path to the Cargo.toml > -CARGO_MANIFEST_PATH ??= "${S}/${CARGO_SRC_DIR}/Cargo.toml" > - > RUSTFLAGS ??= "" > BUILD_MODE = "${@['--release', ''][d.getVar('DEBUG_BUILD') == '1']}" > # --frozen flag will prevent network access (which is required since only > diff --git a/meta/classes-recipe/cargo_common.bbclass b/meta/classes-recipe/cargo_common.bbclass > index bf298e96c745..c330c122a9d3 100644 > --- a/meta/classes-recipe/cargo_common.bbclass > +++ b/meta/classes-recipe/cargo_common.bbclass > @@ -33,6 +33,13 @@ CARGO_DISABLE_BITBAKE_VENDORING ?= "0" > # Used by libstd-rs to point to the vendor dir included in rustc src > CARGO_VENDORING_DIRECTORY ?= "${CARGO_HOME}/bitbake" > > +# The directory of the Cargo.toml relative to the root directory, per default > +# assume there's a Cargo.toml directly in the root directory > +CARGO_SRC_DIR ?= "" > + > +# The actual path to the Cargo.toml > +CARGO_MANIFEST_PATH ?= "${S}/${CARGO_SRC_DIR}/Cargo.toml" > + > CARGO_RUST_TARGET_CCLD ?= "${RUST_TARGET_CCLD}" > cargo_common_do_configure () { > mkdir -p ${CARGO_HOME}/bitbake > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#191955): https://lists.openembedded.org/g/openembedded-core/message/191955 > Mute This Topic: https://lists.openembedded.org/mt/103034028/1686473 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [richard.purdie@linuxfoundation.org] > -=-=-=-=-=-=-=-=-=-=-=- >
On Thu, Dec 7, 2023 at 10:40 PM Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > On Thu, 2023-12-07 at 13:39 +0000, Alex Kiernan wrote: > > cargo_common_do_configure uses CARGO_MANIFEST_PATH (which depends on > > CARGO_SRC_DIR), but their definition was in cargo.bbclass. > > > > Match the other variables here and change to default values, rather > > than weak defaults. > > FWIW "single value" class variables tend to work much better as ??= > (which I'd call default value) since than it doesn't matter if the > recipe setting comes before or after the inherit and the inherit > position in the recipe doesn't matter. > Ah, I guess that makes sense - for "single value" :append/:remove aren't really useful, so leaving the actual assignment as late as possible works? TBH I've never really been very clear what the "right" thing to use is between ?= and ??= > I'd call ?= a weak assignment or weak default. > > Thanks for working on cleaning some of these things up. I got so far > with it originally and then ran out of time (and too frustrated with > the long build/test cycles!). > I'll go back and do the opposite cleanup and send a v2. > Cheers, > > Richard > > > > > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com> > > --- > > > > meta/classes-recipe/cargo.bbclass | 7 ------- > > meta/classes-recipe/cargo_common.bbclass | 7 +++++++ > > 2 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/meta/classes-recipe/cargo.bbclass b/meta/classes-recipe/cargo.bbclass > > index 96a74e2ef1ec..0829a58dd90f 100644 > > --- a/meta/classes-recipe/cargo.bbclass > > +++ b/meta/classes-recipe/cargo.bbclass > > @@ -30,13 +30,6 @@ B = "${WORKDIR}/build" > > # where the issue occured > > export RUST_BACKTRACE = "1" > > > > -# The directory of the Cargo.toml relative to the root directory, per default > > -# assume there's a Cargo.toml directly in the root directory > > -CARGO_SRC_DIR ??= "" > > - > > -# The actual path to the Cargo.toml > > -CARGO_MANIFEST_PATH ??= "${S}/${CARGO_SRC_DIR}/Cargo.toml" > > - > > RUSTFLAGS ??= "" > > BUILD_MODE = "${@['--release', ''][d.getVar('DEBUG_BUILD') == '1']}" > > # --frozen flag will prevent network access (which is required since only > > diff --git a/meta/classes-recipe/cargo_common.bbclass b/meta/classes-recipe/cargo_common.bbclass > > index bf298e96c745..c330c122a9d3 100644 > > --- a/meta/classes-recipe/cargo_common.bbclass > > +++ b/meta/classes-recipe/cargo_common.bbclass > > @@ -33,6 +33,13 @@ CARGO_DISABLE_BITBAKE_VENDORING ?= "0" > > # Used by libstd-rs to point to the vendor dir included in rustc src > > CARGO_VENDORING_DIRECTORY ?= "${CARGO_HOME}/bitbake" > > > > +# The directory of the Cargo.toml relative to the root directory, per default > > +# assume there's a Cargo.toml directly in the root directory > > +CARGO_SRC_DIR ?= "" > > + > > +# The actual path to the Cargo.toml > > +CARGO_MANIFEST_PATH ?= "${S}/${CARGO_SRC_DIR}/Cargo.toml" > > + > > CARGO_RUST_TARGET_CCLD ?= "${RUST_TARGET_CCLD}" > > cargo_common_do_configure () { > > mkdir -p ${CARGO_HOME}/bitbake > > -=-=-=-=-=-=-=-=-=-=-=- > > Links: You receive all messages sent to this group. > > View/Reply Online (#191955): https://lists.openembedded.org/g/openembedded-core/message/191955 > > Mute This Topic: https://lists.openembedded.org/mt/103034028/1686473 > > Group Owner: openembedded-core+owner@lists.openembedded.org > > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [richard.purdie@linuxfoundation.org] > > -=-=-=-=-=-=-=-=-=-=-=- > > >
On Fri, Dec 8, 2023 at 8:15 AM Alex Kiernan via lists.openembedded.org <alex.kiernan=gmail.com@lists.openembedded.org> wrote: > > On Thu, Dec 7, 2023 at 10:40 PM Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > > On Thu, 2023-12-07 at 13:39 +0000, Alex Kiernan wrote: > > > cargo_common_do_configure uses CARGO_MANIFEST_PATH (which depends on > > > CARGO_SRC_DIR), but their definition was in cargo.bbclass. > > > > > > Match the other variables here and change to default values, rather > > > than weak defaults. > > > > FWIW "single value" class variables tend to work much better as ??= > > (which I'd call default value) since than it doesn't matter if the > > recipe setting comes before or after the inherit and the inherit > > position in the recipe doesn't matter. > > > > Ah, I guess that makes sense - for "single value" :append/:remove > aren't really useful, so leaving the actual assignment as late as > possible works? > And I've just read the bitbake docs and I clearly still don't really understand... its += / -= rather than append/remove which are the things which are important here. > TBH I've never really been very clear what the "right" thing to use is > between ?= and ??= > > > I'd call ?= a weak assignment or weak default. > > > > Thanks for working on cleaning some of these things up. I got so far > > with it originally and then ran out of time (and too frustrated with > > the long build/test cycles!). > > > > I'll go back and do the opposite cleanup and send a v2. > > > Cheers, > > > > Richard > > > > > > > > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com> > > > --- > > > > > > meta/classes-recipe/cargo.bbclass | 7 ------- > > > meta/classes-recipe/cargo_common.bbclass | 7 +++++++ > > > 2 files changed, 7 insertions(+), 7 deletions(-) > > > > > > diff --git a/meta/classes-recipe/cargo.bbclass b/meta/classes-recipe/cargo.bbclass > > > index 96a74e2ef1ec..0829a58dd90f 100644 > > > --- a/meta/classes-recipe/cargo.bbclass > > > +++ b/meta/classes-recipe/cargo.bbclass > > > @@ -30,13 +30,6 @@ B = "${WORKDIR}/build" > > > # where the issue occured > > > export RUST_BACKTRACE = "1" > > > > > > -# The directory of the Cargo.toml relative to the root directory, per default > > > -# assume there's a Cargo.toml directly in the root directory > > > -CARGO_SRC_DIR ??= "" > > > - > > > -# The actual path to the Cargo.toml > > > -CARGO_MANIFEST_PATH ??= "${S}/${CARGO_SRC_DIR}/Cargo.toml" > > > - > > > RUSTFLAGS ??= "" > > > BUILD_MODE = "${@['--release', ''][d.getVar('DEBUG_BUILD') == '1']}" > > > # --frozen flag will prevent network access (which is required since only > > > diff --git a/meta/classes-recipe/cargo_common.bbclass b/meta/classes-recipe/cargo_common.bbclass > > > index bf298e96c745..c330c122a9d3 100644 > > > --- a/meta/classes-recipe/cargo_common.bbclass > > > +++ b/meta/classes-recipe/cargo_common.bbclass > > > @@ -33,6 +33,13 @@ CARGO_DISABLE_BITBAKE_VENDORING ?= "0" > > > # Used by libstd-rs to point to the vendor dir included in rustc src > > > CARGO_VENDORING_DIRECTORY ?= "${CARGO_HOME}/bitbake" > > > > > > +# The directory of the Cargo.toml relative to the root directory, per default > > > +# assume there's a Cargo.toml directly in the root directory > > > +CARGO_SRC_DIR ?= "" > > > + > > > +# The actual path to the Cargo.toml > > > +CARGO_MANIFEST_PATH ?= "${S}/${CARGO_SRC_DIR}/Cargo.toml" > > > + > > > CARGO_RUST_TARGET_CCLD ?= "${RUST_TARGET_CCLD}" > > > cargo_common_do_configure () { > > > mkdir -p ${CARGO_HOME}/bitbake > > > > > > > > > > > -- > Alex Kiernan > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#192014): https://lists.openembedded.org/g/openembedded-core/message/192014 > Mute This Topic: https://lists.openembedded.org/mt/103034028/3618097 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kiernan@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On Fri, 2023-12-08 at 08:15 +0000, Alex Kiernan wrote: > On Thu, Dec 7, 2023 at 10:40 PM Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > > On Thu, 2023-12-07 at 13:39 +0000, Alex Kiernan wrote: > > > cargo_common_do_configure uses CARGO_MANIFEST_PATH (which depends on > > > CARGO_SRC_DIR), but their definition was in cargo.bbclass. > > > > > > Match the other variables here and change to default values, rather > > > than weak defaults. > > > > FWIW "single value" class variables tend to work much better as ??= > > (which I'd call default value) since than it doesn't matter if the > > recipe setting comes before or after the inherit and the inherit > > position in the recipe doesn't matter. > > > > Ah, I guess that makes sense - for "single value" :append/:remove > aren't really useful, so leaving the actual assignment as late as > possible works? ??= means "fall back to this value if nothing else is ever set". For a single value, append or += never makes sense so the fallback is probably what we want from a class regardless of order. If you use ?= it just adds some ordering constraints. > TBH I've never really been very clear what the "right" thing to use is > between ?= and ??= For the above case it is one of the few "clearer" cases. As soon as you have multiple values, it becomes fuzzy sadly. I'm not sure how we improve things either. Cheers, Richard
On Fri, 2023-12-08 at 08:23 +0000, Alex Kiernan wrote: > On Fri, Dec 8, 2023 at 8:15 AM Alex Kiernan via lists.openembedded.org > <alex.kiernan=gmail.com@lists.openembedded.org> wrote: > > > > On Thu, Dec 7, 2023 at 10:40 PM Richard Purdie > > <richard.purdie@linuxfoundation.org> wrote: > > > > > > On Thu, 2023-12-07 at 13:39 +0000, Alex Kiernan wrote: > > > > cargo_common_do_configure uses CARGO_MANIFEST_PATH (which depends on > > > > CARGO_SRC_DIR), but their definition was in cargo.bbclass. > > > > > > > > Match the other variables here and change to default values, rather > > > > than weak defaults. > > > > > > FWIW "single value" class variables tend to work much better as ??= > > > (which I'd call default value) since than it doesn't matter if the > > > recipe setting comes before or after the inherit and the inherit > > > position in the recipe doesn't matter. > > > > > > > Ah, I guess that makes sense - for "single value" :append/:remove > > aren't really useful, so leaving the actual assignment as late as > > possible works? > > > > And I've just read the bitbake docs and I clearly still don't really > understand... its += / -= rather than append/remove which are the > things which are important here. > > > TBH I've never really been very clear what the "right" thing to use is > > between ?= and ??= For something which isn't a single value, you'd need to use += or append and that gets much more unclear. My comments were only for single value variables as we don't have a good answer for the rest, much as I wish we had. Cheers, Richard
diff --git a/meta/classes-recipe/cargo.bbclass b/meta/classes-recipe/cargo.bbclass index 96a74e2ef1ec..0829a58dd90f 100644 --- a/meta/classes-recipe/cargo.bbclass +++ b/meta/classes-recipe/cargo.bbclass @@ -30,13 +30,6 @@ B = "${WORKDIR}/build" # where the issue occured export RUST_BACKTRACE = "1" -# The directory of the Cargo.toml relative to the root directory, per default -# assume there's a Cargo.toml directly in the root directory -CARGO_SRC_DIR ??= "" - -# The actual path to the Cargo.toml -CARGO_MANIFEST_PATH ??= "${S}/${CARGO_SRC_DIR}/Cargo.toml" - RUSTFLAGS ??= "" BUILD_MODE = "${@['--release', ''][d.getVar('DEBUG_BUILD') == '1']}" # --frozen flag will prevent network access (which is required since only diff --git a/meta/classes-recipe/cargo_common.bbclass b/meta/classes-recipe/cargo_common.bbclass index bf298e96c745..c330c122a9d3 100644 --- a/meta/classes-recipe/cargo_common.bbclass +++ b/meta/classes-recipe/cargo_common.bbclass @@ -33,6 +33,13 @@ CARGO_DISABLE_BITBAKE_VENDORING ?= "0" # Used by libstd-rs to point to the vendor dir included in rustc src CARGO_VENDORING_DIRECTORY ?= "${CARGO_HOME}/bitbake" +# The directory of the Cargo.toml relative to the root directory, per default +# assume there's a Cargo.toml directly in the root directory +CARGO_SRC_DIR ?= "" + +# The actual path to the Cargo.toml +CARGO_MANIFEST_PATH ?= "${S}/${CARGO_SRC_DIR}/Cargo.toml" + CARGO_RUST_TARGET_CCLD ?= "${RUST_TARGET_CCLD}" cargo_common_do_configure () { mkdir -p ${CARGO_HOME}/bitbake
cargo_common_do_configure uses CARGO_MANIFEST_PATH (which depends on CARGO_SRC_DIR), but their definition was in cargo.bbclass. Match the other variables here and change to default values, rather than weak defaults. Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com> --- meta/classes-recipe/cargo.bbclass | 7 ------- meta/classes-recipe/cargo_common.bbclass | 7 +++++++ 2 files changed, 7 insertions(+), 7 deletions(-)