| Message ID | 20250211150034.18696-11-stefan.herbrechtsmeier-oss@weidmueller.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add vendor support for go, npm and rust | expand |
> -----Original Message----- > From: bitbake-devel@lists.openembedded.org <bitbake-devel@lists.openembedded.org> On Behalf Of Stefan Herbrechtsmeier via lists.openembedded.org > Sent: den 11 februari 2025 16:00 > To: openembedded-core@lists.openembedded.org > Cc: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>; bitbake-devel@lists.openembedded.org > Subject: [bitbake-devel] [RFC PATCH 10/30] conf: bitbake: add SRC_URI_FILES variable > > From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > > Add the variable SRC_URI_FILES to collect files whichs contains > additional SRC_URI lines. > > Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > --- > > meta/conf/bitbake.conf | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf > index 8b607088c6..ed67500ba7 100644 > --- a/meta/conf/bitbake.conf > +++ b/meta/conf/bitbake.conf > @@ -745,6 +745,7 @@ AUTOREV = "${@bb.fetch2.get_autorev(d)}" > SRCPV = "" > > SRC_URI = "" > +SRC_URI_FILES = "" Do we really need a new variable for this? Can't you add some parameter instead to the URIs that should be handled separately? If a parameter is not appropriate, then the name of the variable should be reconsidered. Based on the name it is unclear what is expected to go in SRC_URI and what should go in SRC_URI_FILES. > > # Use pseudo as the fakeroot implementation > PSEUDO_LOCALSTATEDIR ?= "${WORKDIR}/pseudo/" > -- > 2.39.5 //Peter
Am 11.02.2025 um 17:22 schrieb Peter Kjellerstedt: >> -----Original Message----- >> From:bitbake-devel@lists.openembedded.org <bitbake-devel@lists.openembedded.org> On Behalf Of Stefan Herbrechtsmeier via lists.openembedded.org >> Sent: den 11 februari 2025 16:00 >> To:openembedded-core@lists.openembedded.org >> Cc: Stefan Herbrechtsmeier<stefan.herbrechtsmeier@weidmueller.com>;bitbake-devel@lists.openembedded.org >> Subject: [bitbake-devel] [RFC PATCH 10/30] conf: bitbake: add SRC_URI_FILES variable >> >> From: Stefan Herbrechtsmeier<stefan.herbrechtsmeier@weidmueller.com> >> >> Add the variable SRC_URI_FILES to collect files whichs contains >> additional SRC_URI lines. >> >> Signed-off-by: Stefan Herbrechtsmeier<stefan.herbrechtsmeier@weidmueller.com> >> --- >> >> meta/conf/bitbake.conf | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf >> index 8b607088c6..ed67500ba7 100644 >> --- a/meta/conf/bitbake.conf >> +++ b/meta/conf/bitbake.conf >> @@ -745,6 +745,7 @@ AUTOREV ="${@bb.fetch2.get_autorev(d)}" >> SRCPV = "" >> >> SRC_URI = "" >> +SRC_URI_FILES = "" > Do we really need a new variable for this? Can't you add some parameter > instead to the URIs that should be handled separately? The variable holds paths to files which contains dynamic generated SRC_URIs. The dynamic SRC_URIs are unknown at parse time and only the file paths are fix. The files are filled with generated SRC_URIs via a separate task. > If a parameter is not appropriate, then the name of the variable should > be reconsidered. Based on the name it is unclear what is expected to go > in SRC_URI and what should go in SRC_URI_FILES. Would SRC_URI_MANIFESTS or SRC_URI_MANIFEST_FILES fit better?
On Wed, 12 Feb 2025 at 09:55, Stefan Herbrechtsmeier via
lists.openembedded.org
<stefan.herbrechtsmeier-oss=weidmueller.com@lists.openembedded.org>
wrote:
> The variable holds paths to files which contains dynamic generated SRC_URIs. The dynamic SRC_URIs are unknown at parse time and only the file paths are fix. The files are filled with generated SRC_URIs via a separate task.
This doesn't explain why we need this separate new variable to begin
with. In what scenarios it would be useful? How does the
implementation rely on it?
I'd suggest SRC_URI_DYNAMIC, we have a precedent in PACKAGES_DYNAMIC.
Alex
On Wed, 12 Feb 2025 at 10:50, Alexander Kanavin via lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org> wrote: > This doesn't explain why we need this separate new variable to begin > with. In what scenarios it would be useful? How does the > implementation rely on it? Ok, I'm reading the patchset further and it becomes somewhat more clear. Stefan, please do write long, descriptive commit messages, so that the purpose of each change doesn't need to be deduced from reading the whole patchset. There's only so much my head and other people's heads can fit in. Alex
Am 12.02.2025 um 10:49 schrieb Alexander Kanavin via lists.openembedded.org: > On Wed, 12 Feb 2025 at 09:55, Stefan Herbrechtsmeier via > lists.openembedded.org > <stefan.herbrechtsmeier-oss=weidmueller.com@lists.openembedded.org> > wrote: > >> The variable holds paths to files which contains dynamic generated SRC_URIs. The dynamic SRC_URIs are unknown at parse time and only the file paths are fix. The files are filled with generated SRC_URIs via a separate task. > This doesn't explain why we need this separate new variable to begin > with. In what scenarios it would be useful? How does the > implementation rely on it? We can't manipulate a global variable value inside a task. We need a file to exchange values between task. The vendor classes resolve the SRC_URIs of the dependency in the resolve task. The new variable contains the list of files with SRC_URIs inside it. The later tasks read the additional SRC_URIs from the files and merge them with the recipe SRC_URIs. > I'd suggest SRC_URI_DYNAMIC, we have a precedent in PACKAGES_DYNAMIC. The variable doesn't contain a dynamic for the SRC_URI but instead a list of files with SRC_URIs inside the file.
On Thu, 13 Feb 2025 at 08:43, Stefan Herbrechtsmeier via lists.openembedded.org <stefan.herbrechtsmeier-oss=weidmueller.com@lists.openembedded.org> wrote: > The variable holds paths to files which contains dynamic generated SRC_URIs. The dynamic SRC_URIs are unknown at parse time and only the file paths are fix. The files are filled with generated SRC_URIs via a separate task. > > This doesn't explain why we need this separate new variable to begin > with. In what scenarios it would be useful? How does the > implementation rely on it? > > We can't manipulate a global variable value inside a task. We need a file to exchange values between task. The vendor classes resolve the SRC_URIs of the dependency in the resolve task. The new variable contains the list of files with SRC_URIs inside it. The later tasks read the additional SRC_URIs from the files and merge them with the recipe SRC_URIs. > > I'd suggest SRC_URI_DYNAMIC, we have a precedent in PACKAGES_DYNAMIC. > > The variable doesn't contain a dynamic for the SRC_URI but instead a list of files with SRC_URIs inside the file. Thanks. But all of this should be in a commit message. And actually even that is not enough: you need to tell where the variable is set, show an example value, show an example content of the file(s) it references, and also tell where the value of the variable is used to do further processing, and what that processing is, and refer to commits doing it. The code is obvious to you because you've worked with it for several weeks now. It's not obvious to any of the reviewers and you need to help them at every step, otherwise few people will look and even fewer will comment, and the whole thing is at risk of fizzling out due to lack of interest. Alex
diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf index 8b607088c6..ed67500ba7 100644 --- a/meta/conf/bitbake.conf +++ b/meta/conf/bitbake.conf @@ -745,6 +745,7 @@ AUTOREV = "${@bb.fetch2.get_autorev(d)}" SRCPV = "" SRC_URI = "" +SRC_URI_FILES = "" # Use pseudo as the fakeroot implementation PSEUDO_LOCALSTATEDIR ?= "${WORKDIR}/pseudo/"