| Message ID | 20230524215526.323283-2-charlie.johnston@ni.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] package-index: Convert package-index into a bbclass. | expand |
What happens if several recipes inherit this class, and are processed at the same time? Won't they step on each other, writing the same index files into the deploy directory? I think package-index recipe is deliberately standalone for that reason: you have to build it in a separate step. Alex On Wed, 24 May 2023 at 23:55, Charlie Johnston <charlie.johnston@ni.com> wrote: > > Added a new packagefeed class that contains both a > packagegroup and the package index builds. This allows a > one-line bitbake for a packagegroup that defines a feed > without explicitly needing to run the package-index recipe. > > Signed-off-by: Charlie Johnston <charlie.johnston@ni.com> > --- > meta/classes/packagefeed.bbclass | 5 +++++ > 1 file changed, 5 insertions(+) > create mode 100644 meta/classes/packagefeed.bbclass > > diff --git a/meta/classes/packagefeed.bbclass b/meta/classes/packagefeed.bbclass > new file mode 100644 > index 0000000000..b83ac54f21 > --- /dev/null > +++ b/meta/classes/packagefeed.bbclass > @@ -0,0 +1,5 @@ > +# > +# Special case of packagegroup that includes package index builds. > +# > + > +inherit packagegroup package_index > -- > 2.30.2 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#181693): https://lists.openembedded.org/g/openembedded-core/message/181693 > Mute This Topic: https://lists.openembedded.org/mt/99118888/1686489 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On Wed, 2023-05-24 at 16:55 -0500, Charlie Johnston wrote: > Added a new packagefeed class that contains both a > packagegroup and the package index builds. This allows a > one-line bitbake for a packagegroup that defines a feed > without explicitly needing to run the package-index recipe. > > Signed-off-by: Charlie Johnston <charlie.johnston@ni.com> > --- > meta/classes/packagefeed.bbclass | 5 +++++ > 1 file changed, 5 insertions(+) > create mode 100644 meta/classes/packagefeed.bbclass > > diff --git a/meta/classes/packagefeed.bbclass b/meta/classes/packagefeed.bbclass > new file mode 100644 > index 0000000000..b83ac54f21 > --- /dev/null > +++ b/meta/classes/packagefeed.bbclass > @@ -0,0 +1,5 @@ > +# > +# Special case of packagegroup that includes package index builds. > +# > + > +inherit packagegroup package_index A new class with two inherits in it doesn't really seem worthwhile to me? Was there more you planned to add here? Cheers, Richard
Yes, having two recipes that inherit this running at the same time would potentially write the same index files. I'm not sure that's actually a problem other than the same thing being done twice. As long as at least one of the package index steps runs after all packages have been written, the index files will be up-to-date. Since I've made the task recursively depend on the tasks that create packages, I think it's fine. That being said, if there's a way to both add it to recipes and ensure that the task only runs once per bitbake command I would be open to moving towards that. Is there anything like that? Thanks, Charlie On Thu, May 25, 2023 at 03:47 AM, Alexander Kanavin wrote: > > What happens if several recipes inherit this class, and are processed > at the same time? Won't they step on each other, writing the same > index files into the deploy directory? I think package-index recipe is > deliberately standalone for that reason: you have to build it in a > separate step. > > Alex > > On Wed, 24 May 2023 at 23:55, Charlie Johnston <charlie.johnston@ni.com> > wrote: > >> >> Added a new packagefeed class that contains both a >> packagegroup and the package index builds. This allows a >> one-line bitbake for a packagegroup that defines a feed >> without explicitly needing to run the package-index recipe. >> >> Signed-off-by: Charlie Johnston <charlie.johnston@ni.com> >> --- >> meta/classes/packagefeed.bbclass | 5 +++++ >> 1 file changed, 5 insertions(+) >> create mode 100644 meta/classes/packagefeed.bbclass >> >> diff --git a/meta/classes/packagefeed.bbclass >> b/meta/classes/packagefeed.bbclass >> new file mode 100644 >> index 0000000000..b83ac54f21 >> --- /dev/null >> +++ b/meta/classes/packagefeed.bbclass >> @@ -0,0 +1,5 @@ >> +# >> +# Special case of packagegroup that includes package index builds. >> +# >> + >> +inherit packagegroup package_index >> -- >> 2.30.2 >> >> >> >> > >
I did not plan to add more currently. I liked this as it makes it clear the intention of a specific packagegroup to represent a feed. I don't mind dropping the class and just having our "packagefeed" packagegroups add the extra inherit. For my own knowledge, what makes a class worthwhile? Is there an expectation that a bbclass has a certain amount of functionality to it? Thanks, Charlie On Thu, May 25, 2023 at 05:52 AM, Richard Purdie wrote: > > On Wed, 2023-05-24 at 16:55 -0500, Charlie Johnston wrote: > >> Added a new packagefeed class that contains both a >> packagegroup and the package index builds. This allows a >> one-line bitbake for a packagegroup that defines a feed >> without explicitly needing to run the package-index recipe. >> >> Signed-off-by: Charlie Johnston <charlie.johnston@ni.com> >> --- >> meta/classes/packagefeed.bbclass | 5 +++++ >> 1 file changed, 5 insertions(+) >> create mode 100644 meta/classes/packagefeed.bbclass >> >> diff --git a/meta/classes/packagefeed.bbclass >> b/meta/classes/packagefeed.bbclass >> new file mode 100644 >> index 0000000000..b83ac54f21 >> --- /dev/null >> +++ b/meta/classes/packagefeed.bbclass >> @@ -0,0 +1,5 @@ >> +# >> +# Special case of packagegroup that includes package index builds. >> +# >> + >> +inherit packagegroup package_index > > A new class with two inherits in it doesn't really seem worthwhile to > me? Was there more you planned to add here? > > Cheers, > > Richard
It would not be the same thing done twice. It is not clear how the rpm/dep/opkg-specific tooling would behave if there's two or more instances of it indexing the files at the same time, or one starts earlier than the other, and the later ones see incomplete index files or half-deployed packages etc. It's well possible such situations are not handled well, and you still need to somehow ensure the indexing indeed runs once, and only after everything else. How would you ensure that? I worry this change opens the door to misuse, and cryptic, hard to reproduce or diagnose errors. The output would also be non-deterministic: it would contain everything from the packagegroup and an unknown amount of additional packages, depending on what else happened to be in deploy directory at the time. Alex On Thu, 25 May 2023 at 17:55, Charlie Johnston <charlie.johnston@ni.com> wrote: > > Yes, having two recipes that inherit this running at the same time would potentially write the same index files. I'm not sure that's actually a problem other than the same thing being done twice. > As long as at least one of the package index steps runs after all packages have been written, the index files will be up-to-date. Since I've made the task recursively depend on the tasks that create packages, I think it's fine. > > That being said, if there's a way to both add it to recipes and ensure that the task only runs once per bitbake command I would be open to moving towards that. Is there anything like that? > > Thanks, > Charlie > > On Thu, May 25, 2023 at 03:47 AM, Alexander Kanavin wrote: > > What happens if several recipes inherit this class, and are processed > at the same time? Won't they step on each other, writing the same > index files into the deploy directory? I think package-index recipe is > deliberately standalone for that reason: you have to build it in a > separate step. > > Alex > > On Wed, 24 May 2023 at 23:55, Charlie Johnston <charlie.johnston@ni.com> wrote: > > > Added a new packagefeed class that contains both a > packagegroup and the package index builds. This allows a > one-line bitbake for a packagegroup that defines a feed > without explicitly needing to run the package-index recipe. > > Signed-off-by: Charlie Johnston <charlie.johnston@ni.com> > --- > meta/classes/packagefeed.bbclass | 5 +++++ > 1 file changed, 5 insertions(+) > create mode 100644 meta/classes/packagefeed.bbclass > > diff --git a/meta/classes/packagefeed.bbclass b/meta/classes/packagefeed.bbclass > new file mode 100644 > index 0000000000..b83ac54f21 > --- /dev/null > +++ b/meta/classes/packagefeed.bbclass > @@ -0,0 +1,5 @@ > +# > +# Special case of packagegroup that includes package index builds. > +# > + > +inherit packagegroup package_index > -- > 2.30.2 > > > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#181715): https://lists.openembedded.org/g/openembedded-core/message/181715 > Mute This Topic: https://lists.openembedded.org/mt/99118888/1686489 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
I see your point. Please drop this patch set from consideration. Taking a step back, my intention was to make it so that a "packagefeed" was a simple buildable target the way an image or packagegroup is. That is, I'd like it to be possible to just "bitbake packagefeed" and get all the packages and indexes. I think the non-deterministic problem exists today too since someone could run "bitbake packagegroup package-index" and nothing is stopping them from doing that other than noticing it doesn't work. I can see a couple of potential options: 1. Come up with a way to ensure that the do_package_index task is only run once per bitbake invocation after all other packaging tasks. As far as I can tell, there's not a way to enforce this with my current design. It also doesn't address the problem of packages already in the deploy directories. 2. Add more scaffolding to create a packagefeed class with potentially its own deploy directory and which only includes index files for its dependencies. That seems like a lot more effort than my change but might address the concerns more completely. Any feedback or thoughts on those ideas? Thanks, Charlie On Thu, May 25, 2023 at 11:11 AM, Alexander Kanavin wrote: > > It would not be the same thing done twice. It is not clear how the > rpm/dep/opkg-specific tooling would behave if there's two or more > instances of it indexing the files at the same time, or one starts > earlier than the other, and the later ones see incomplete index files > or half-deployed packages etc. It's well possible such situations are > not handled well, and you still need to somehow ensure the indexing > indeed runs once, and only after everything else. How would you ensure > that? I worry this change opens the door to misuse, and cryptic, hard > to reproduce or diagnose errors. > > The output would also be non-deterministic: it would contain > everything from the packagegroup and an unknown amount of additional > packages, depending on what else happened to be in deploy directory at > the time. > > Alex > > On Thu, 25 May 2023 at 17:55, Charlie Johnston <charlie.johnston@ni.com> > wrote: > >> >> Yes, having two recipes that inherit this running at the same time would >> potentially write the same index files. I'm not sure that's actually a >> problem other than the same thing being done twice. >> As long as at least one of the package index steps runs after all packages >> have been written, the index files will be up-to-date. Since I've made the >> task recursively depend on the tasks that create packages, I think it's >> fine. >> >> That being said, if there's a way to both add it to recipes and ensure >> that the task only runs once per bitbake command I would be open to moving >> towards that. Is there anything like that? >> >> Thanks, >> Charlie >> >> On Thu, May 25, 2023 at 03:47 AM, Alexander Kanavin wrote: >> >> What happens if several recipes inherit this class, and are processed >> at the same time? Won't they step on each other, writing the same >> index files into the deploy directory? I think package-index recipe is >> deliberately standalone for that reason: you have to build it in a >> separate step. >> >> Alex >> >> On Wed, 24 May 2023 at 23:55, Charlie Johnston <charlie.johnston@ni.com> >> wrote: >> >> >> Added a new packagefeed class that contains both a >> packagegroup and the package index builds. This allows a >> one-line bitbake for a packagegroup that defines a feed >> without explicitly needing to run the package-index recipe. >> >> Signed-off-by: Charlie Johnston <charlie.johnston@ni.com> >> --- >> meta/classes/packagefeed.bbclass | 5 +++++ >> 1 file changed, 5 insertions(+) >> create mode 100644 meta/classes/packagefeed.bbclass >> >> diff --git a/meta/classes/packagefeed.bbclass >> b/meta/classes/packagefeed.bbclass >> new file mode 100644 >> index 0000000000..b83ac54f21 >> --- /dev/null >> +++ b/meta/classes/packagefeed.bbclass >> @@ -0,0 +1,5 @@ >> +# >> +# Special case of packagegroup that includes package index builds. >> +# >> + >> +inherit packagegroup package_index >> -- >> 2.30.2 >> >> >> >> >> >> > >
On Thu, 25 May 2023 at 23:54, Charlie Johnston <charlie.johnston@ni.com> wrote: > I can see a couple of potential options: > > 1. Come up with a way to ensure that the do_package_index task is only run once per bitbake invocation after all other packaging tasks. As far as I can tell, there's not a way to enforce this with my current design. It also doesn't address the problem of packages already in the deploy directories. > 2. Add more scaffolding to create a packagefeed class with potentially its own deploy directory and which only includes index files for its dependencies. That seems like a lot more effort than my change but might address the concerns more completely. > Any feedback or thoughts on those ideas? You could try doing something like what buildhistory and similar classes do, that is, hook into bb.event.BuildCompleted: https://docs.yoctoproject.org/bitbake/bitbake-user-manual/bitbake-user-manual-metadata.html#events Alex
By the way, image recipes already do something like point 2 (copy only the needed packages out of global deploy into recipe's $WORKDIR, and then run an indexer only on that prior to composing the rootfs from those private feeds), so you could as well reuse that code. See create_packages_dir() in lib/oe/package_manager/__init__.py I think that's a better option than option 1. Alex On Fri, 26 May 2023 at 09:58, Alexander Kanavin via lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org> wrote: > > On Thu, 25 May 2023 at 23:54, Charlie Johnston <charlie.johnston@ni.com> wrote: > > I can see a couple of potential options: > > > > 1. Come up with a way to ensure that the do_package_index task is only run once per bitbake invocation after all other packaging tasks. As far as I can tell, there's not a way to enforce this with my current design. It also doesn't address the problem of packages already in the deploy directories. > > 2. Add more scaffolding to create a packagefeed class with potentially its own deploy directory and which only includes index files for its dependencies. That seems like a lot more effort than my change but might address the concerns more completely. > > Any feedback or thoughts on those ideas? > > You could try doing something like what buildhistory and similar > classes do, that is, hook into bb.event.BuildCompleted: > https://docs.yoctoproject.org/bitbake/bitbake-user-manual/bitbake-user-manual-metadata.html#events > > Alex > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#181763): https://lists.openembedded.org/g/openembedded-core/message/181763 > Mute This Topic: https://lists.openembedded.org/mt/99118888/1686489 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
diff --git a/meta/classes/packagefeed.bbclass b/meta/classes/packagefeed.bbclass new file mode 100644 index 0000000000..b83ac54f21 --- /dev/null +++ b/meta/classes/packagefeed.bbclass @@ -0,0 +1,5 @@ +# +# Special case of packagegroup that includes package index builds. +# + +inherit packagegroup package_index
Added a new packagefeed class that contains both a packagegroup and the package index builds. This allows a one-line bitbake for a packagegroup that defines a feed without explicitly needing to run the package-index recipe. Signed-off-by: Charlie Johnston <charlie.johnston@ni.com> --- meta/classes/packagefeed.bbclass | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 meta/classes/packagefeed.bbclass