| Message ID | 20230910194648.942409-2-rs@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] bitbake-worker: remove the network flag | expand |
Please stop sending the patch. You are just winding everyone up. If you want to have a discussion, open a thread with the comment about icecc and friends, and we'll take it from there, otherwise any further submission from you will be treated with suspicion and mistrust, which is not the outcome you want. Alex On Sun, 10 Sept 2023 at 21:54, <rs@ti.com> wrote: > > From: Randolph Sapp <rs@ti.com> > > The network flag does not currently respect proxies by default as the > fetch stage does [1]. As such it only works for some users at the whim > of the recipe writer. > > This introduces an issue downstream as the build results are now > dependent on the network architecture of the builder, the recipe in > question, and the combination of proxy variables they choose to export > in addition to the normal host variables that affect builds (host tool > version, build environment, etc). > > This is far more indeterminate than a build system that properly exports > proxy variables. If abuse of the network tag is an issue, then the > network tag should be removed in it's current state and networking > should depend entirely on whether the current task is 'do_fetch'. > > [1] https://lists.openembedded.org/g/bitbake-devel/topic/100924096 > > Signed-off-by: Randolph Sapp <rs@ti.com> > --- > > What I feel would be the proper solution to the aforementioned issues. > > Examples of layers that currently use the network flag outside of the do_fetch > stage: > > oe-core (icecc.bbclass) > meta-openembedded (go recipes) > meta-flutter (all flutter recipes and classes) > > This is not an attempt to shame maintainers. I'm just curious why the network > task expects every maintainer to handle proxy variables (something pretty much > required if you are actually using the network in that task) independently. It > just results in needless debugging and a bunch of patches attempting to export > variables in their own way. Sorry if this comes off as antagonistic in any way. > > bin/bitbake-worker | 2 +- > doc/bitbake-user-manual/bitbake-user-manual-metadata.rst | 6 ------ > 2 files changed, 1 insertion(+), 7 deletions(-) > > diff --git a/bin/bitbake-worker b/bin/bitbake-worker > index 451e6926..f25880c1 100755 > --- a/bin/bitbake-worker > +++ b/bin/bitbake-worker > @@ -270,7 +270,7 @@ def fork_off_task(cfg, data, databuilder, workerdata, extraconfigdata, runtask): > > bb.utils.set_process_name("%s:%s" % (the_data.getVar("PN"), taskname.replace("do_", ""))) > > - if not bb.utils.to_boolean(the_data.getVarFlag(taskname, 'network')): > + if taskname != 'do_fetch': > if bb.utils.is_local_uid(uid): > logger.debug("Attempting to disable network for %s" % taskname) > bb.utils.disable_network(uid, gid) > diff --git a/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst b/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst > index 58975f4c..b35c332c 100644 > --- a/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst > +++ b/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst > @@ -1519,12 +1519,6 @@ functionality of the task: > released. You can use this variable flag to accomplish mutual > exclusion. > > -- ``[network]``: When set to "1", allows a task to access the network. By > - default, only the ``do_fetch`` task is granted network access. Recipes > - shouldn't access the network outside of ``do_fetch`` as it usually > - undermines fetcher source mirroring, image and licence manifests, software > - auditing and supply chain security. > - > - ``[noexec]``: When set to "1", marks the task as being empty, with > no execution required. You can use the ``[noexec]`` flag to set up > tasks as dependency placeholders, or to disable tasks defined > -- > 2.42.0 >
On 9/10/23 14:57, Alexander Kanavin wrote: > Please stop sending the patch. You are just winding everyone up. If > you want to have a discussion, open a thread with the comment about > icecc and friends, and we'll take it from there, otherwise any further > submission from you will be treated with suspicion and mistrust, which > is not the outcome you want. > > Alex > [snip] Fair enough. It was my intent to actually submit a usable patch with this discussion but the first revision was a bit too hastily sent. Rest assured, this will be the last revision. I'm not here to push the matter. I just figured this was the next step if the network flag didn't behave the same for all users. Seems like people like the current behavior though. That's fine. I just think this should be documented somewhere if that's the case. I can submit a patch for that documentation update instead, if that's preferred.
On Sun, 10 Sept 2023 at 22:11, Randolph Sapp <rs@ti.com> wrote: > Seems like people like the current behavior though. That's fine. I just > think this should be documented somewhere if that's the case. I can > submit a patch for that documentation update instead, if that's preferred. I've briefly looked at meta-flutter meanwhile. It's generally full of horrible things such as AUTOREV, and does abuse the network flag, but the issue is it's a badly written layer, and so you should take it up with the layer maintainer: https://github.com/jwinarske https://github.com/meta-flutter/meta-flutter/issues/new Go recipes (not sure which ones because I didn't find any in meta-oe master but nvm) need to be fixed in the same way we fixed rust recipes: by listing all items in SRC_URI, and having tooling in place to update the list automatically from source trees. Patches welcome. icecc is probably okay, as that's the kind of thing generally accessible without having to go through a proxy. Alex
On Sun, 2023-09-10 at 14:46 -0500, rs@ti.com wrote: > What I feel would be the proper solution to the aforementioned issues. > > Examples of layers that currently use the network flag outside of the do_fetch > stage: > > oe-core (icecc.bbclass) > meta-openembedded (go recipes) > meta-flutter (all flutter recipes and classes) > > This is not an attempt to shame maintainers. I'm just curious why the network > task expects every maintainer to handle proxy variables (something pretty much > required if you are actually using the network in that task) independently. It > just results in needless debugging and a bunch of patches attempting to export > variables in their own way. Sorry if this comes off as antagonistic in any way. You are doing yourself no favours at all by posting this patch again. My instinctive reaction would be to filter all your mails to /dev/null from now on. You have to keep in mind that bitbake/OE allow you to do pretty much anything from metadata so it is hard to put boundaries and any that we do put in place can and will get bypassed. We've gone backwards and forwards on these issues for a long time. The "Yocto Project Compatible" status was born out of a desire to at least make it possible to highlight layers doing things which could be considered anti-social. We haven't quite caught up with network enable/disable there yet but we should and likely will (please file a bug for that). Sadly the work in maintaining that standard is more complex than people realise, e.g. a new revision of the criteria has been held up by a YP website refresh for over a year <snip long painful story> :(. We added the network flag since before that, code was doing pretty much what it wanted anywhere. We now at least can tell exactly where the accesses are being enabled, which is a start to fixing some of these issues. icecc in OE-Core enables it since by its very nature, distributed compiling needs the network. It is unlikely to need/want proxy info though as it will likely be local network only. go needs someone to work on the fetcher side of things for it. We've solutions with rust/crates which might help provide a model for how that could work in the future. For better or worse, someone does need to spend time on it. I've no experience with flutter to comment there. I will note that most of these "new" things start with basics and things like proxy support have to follow later as they're not top of people's minds when they're trying to get anything working. I am frustrated in general about a great many areas of the project where things don't get "completed" to the level everyone would like. Over time we do have a good track record of sorting them out though, I just wish people could work on them proactively rather than reactively. We do have some proactive work happening at least for the first time in a while with the RFQs. I do understand your frustration but I think you need to work out a more productive way to move forward. The network flag did at least let us mark up and know where the issues are so in that sense we did improve on what went before. Cheers, Richard
On 9/10/23 15:17, Richard Purdie wrote: > On Sun, 2023-09-10 at 14:46 -0500, rs@ti.com wrote: >> What I feel would be the proper solution to the aforementioned issues. >> >> Examples of layers that currently use the network flag outside of the do_fetch >> stage: >> >> oe-core (icecc.bbclass) >> meta-openembedded (go recipes) >> meta-flutter (all flutter recipes and classes) >> >> This is not an attempt to shame maintainers. I'm just curious why the network >> task expects every maintainer to handle proxy variables (something pretty much >> required if you are actually using the network in that task) independently. It >> just results in needless debugging and a bunch of patches attempting to export >> variables in their own way. Sorry if this comes off as antagonistic in any way. > > You are doing yourself no favours at all by posting this patch again. > My instinctive reaction would be to filter all your mails to /dev/null > from now on. > > You have to keep in mind that bitbake/OE allow you to do pretty much > anything from metadata so it is hard to put boundaries and any that we > do put in place can and will get bypassed. We've gone backwards and > forwards on these issues for a long time. > > The "Yocto Project Compatible" status was born out of a desire to at > least make it possible to highlight layers doing things which could be > considered anti-social. We haven't quite caught up with network > enable/disable there yet but we should and likely will (please file a > bug for that). Sadly the work in maintaining that standard is more > complex than people realise, e.g. a new revision of the criteria has > been held up by a YP website refresh for over a year <snip long painful > story> :(. > > We added the network flag since before that, code was doing pretty much > what it wanted anywhere. We now at least can tell exactly where the > accesses are being enabled, which is a start to fixing some of these > issues. > > icecc in OE-Core enables it since by its very nature, distributed > compiling needs the network. It is unlikely to need/want proxy info > though as it will likely be local network only. > > go needs someone to work on the fetcher side of things for it. We've > solutions with rust/crates which might help provide a model for how > that could work in the future. For better or worse, someone does need > to spend time on it. > > I've no experience with flutter to comment there. I will note that most > of these "new" things start with basics and things like proxy support > have to follow later as they're not top of people's minds when they're > trying to get anything working. > > I am frustrated in general about a great many areas of the project > where things don't get "completed" to the level everyone would like. > Over time we do have a good track record of sorting them out though, I > just wish people could work on them proactively rather than reactively. > We do have some proactive work happening at least for the first time in > a while with the RFQs. > > I do understand your frustration but I think you need to work out a > more productive way to move forward. The network flag did at least let > us mark up and know where the issues are so in that sense we did > improve on what went before. > > Cheers, > > Richard Hey Richard, Thanks for the background. I won't push this issue anymore. The current method does seem to be pretty good at passively catching offending tasks. Distributed compiling itself is an interesting cornercase to both this patch and my previous proxy related one that shows that the current approach is reasonable and doesn't necessarily warrant either of these changes. Sorry to bother you all on a Sunday. Regards, Randolph
On 10/09/2023 15:50:22-0500, Randolph Sapp via lists.openembedded.org wrote: > Thanks for the background. I won't push this issue anymore. The current > method does seem to be pretty good at passively catching offending tasks. > This is certainly way more important for your customers than you realize. The proxy issue is not actually an issue because it is very likely that anyway they would have to set up a local mirror to ensure they are able to rebuild without relying on a third party. You should rather teach them to do that instead of working around the networking issues they are creating for themselves. This is even more important as the companies having those network issue are probably the one that care the most about the correctness of spdx. Really, the proper fix is in the fetcher.
On 9/10/23 16:16, Alexandre Belloni wrote: > On 10/09/2023 15:50:22-0500, Randolph Sapp via lists.openembedded.org wrote: >> Thanks for the background. I won't push this issue anymore. The current >> method does seem to be pretty good at passively catching offending tasks. >> > > This is certainly way more important for your customers than you > realize. The proxy issue is not actually an issue because it is very > likely that anyway they would have to set up a local mirror to ensure > they are able to rebuild without relying on a third party. You should > rather teach them to do that instead of working around the networking > issues they are creating for themselves. > > This is even more important as the companies having those network issue > are probably the one that care the most about the correctness of spdx. > > Really, the proper fix is in the fetcher. > Right, I suppose I'm just pessimistic in assuming that users will tend to flock to the easier solution of enabling the network flag and not circling back to proper method of implementing the fetcher. (I know that's what I did for my first layer.) Education and community involvement ideally will solve this though. I just have my reservations about allowing multiple methods of solving an issue to exist in parallel. Makes debugging things interesting. Regardless, thanks Alexandre. :)
diff --git a/bin/bitbake-worker b/bin/bitbake-worker index 451e6926..f25880c1 100755 --- a/bin/bitbake-worker +++ b/bin/bitbake-worker @@ -270,7 +270,7 @@ def fork_off_task(cfg, data, databuilder, workerdata, extraconfigdata, runtask): bb.utils.set_process_name("%s:%s" % (the_data.getVar("PN"), taskname.replace("do_", ""))) - if not bb.utils.to_boolean(the_data.getVarFlag(taskname, 'network')): + if taskname != 'do_fetch': if bb.utils.is_local_uid(uid): logger.debug("Attempting to disable network for %s" % taskname) bb.utils.disable_network(uid, gid) diff --git a/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst b/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst index 58975f4c..b35c332c 100644 --- a/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst +++ b/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst @@ -1519,12 +1519,6 @@ functionality of the task: released. You can use this variable flag to accomplish mutual exclusion. -- ``[network]``: When set to "1", allows a task to access the network. By - default, only the ``do_fetch`` task is granted network access. Recipes - shouldn't access the network outside of ``do_fetch`` as it usually - undermines fetcher source mirroring, image and licence manifests, software - auditing and supply chain security. - - ``[noexec]``: When set to "1", marks the task as being empty, with no execution required. You can use the ``[noexec]`` flag to set up tasks as dependency placeholders, or to disable tasks defined