classes/externalsrc: Don't delete tasks
PATCH 1/1 classes/externalsrc: Don't delete tasks
Set the noexec flag to prevent tasks from executing instead of deleting them. This allows inter-tasks dependencies on these tasks to still function. For example, perf has the line:
do_populate_lic[depends] += "virtual/kernel:do_patch"
which will break if the kernel uses EXTERNALSRC and the do_patch task is deleted.
--- meta/classes/externalsrc.bbclass | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
meta/classes/externalsrc.bbclass +1 -1
@@ -102,7 +102,7 @@ python () { for task in d.getVar("SRCTREECOVEREDTASKS").split(): if local_srcuri and task in fetch_tasks: continue - bb.build.deltask(task, d) + d.setVarFlag(task, 'noexec', '1') d.prependVarFlag('do_compile', 'prefuncs', "externalsrc_compile_prefunc ") d.prependVarFlag('do_configure', 'prefuncs', "externalsrc_configure_prefunc ")
Signature · 1 line
2.14.3
Quoted 23 lines · Joshua Watt
Set the noexec flag to prevent tasks from executing instead of deleting
them. This allows inter-tasks dependencies on these tasks to still function. For example, perf has the line:do_populate_lic[depends] += "virtual/kernel:do_patch"
which will break if the kernel uses EXTERNALSRC and the
do_patchtask is deleted.Signed-off-by Joshua Watt <[email protected]>---
meta/classes/externalsrc.bbclass | 2 +-1 file changed, 1 insertion(+), 1 deletion(-)meta/classes/externalsrc.bbclass +0
-0@@ -102,7 +102,7 @@ python () {for task in d.getVar("SRCTREECOVEREDTASKS").split():
iflocal_srcuriand task infetch_tasks:
Quoted 8 lines
continue
- bb.build.deltask(task, d)
+ d.setVarFlag(task, 'noexec', '1')
d.prependVarFlag('do_compile', 'prefuncs', "externalsrc_compile_prefunc")d.prependVarFlag('do_configure', 'prefuncs', "externalsrc_configure_prefunc ")
There are a few things I worry about with this:
a) Its confusing for users as it will now show a fetch/unpack/patch task as part of the dependency tree but they won't actually run.
b) Dependencies for those tasks will be followed
c) There has always been a bit of ambiguity about what should happen with task properties like cleandirs, dirs and others. Whilst these don't run for noexec, I have worries that somehow we'll end up wiping out the external source dir accidentally (e.g. with do_clean). I think the noexec move makes this more likely, sadly.
externalsrc started out as a 30 line class back in 2012 and its now 235 lines and getting ridiculously complicated.
I'm starting to wonder if we should change the approach and have it modify SRC_URI and take a copy of the source. Yes, it'd be fractionally slower but its getting far too complex trying to handle all the corner cases. It was meant so developers could quickly hack/test things and its turning into a full development tool/workflow.
I think we do need to take a step back and try and come up with a way to simplify it...
We should also think about whether there are changes to the core which would make things like this easier. I do have a fairly strong belief that we should make the system more robust and able to handle tasks being missing, its now a lot better at handling that than it used to be. Chris as talked about a "srcready" type task and reworking the fetch/unpack/patch process and I can see the reasoning for that, it would help here too.
Not sure whether Paul has any thoughts on this as externalsrc will significantly impact devtool too...
Cheers,
Richard
Quoted 52 lines · Richard Purdie
Quoted 30 lines · Joshua Watt
Set the noexec flag to prevent tasks from executing instead of deleting
them. This allows inter-tasks dependencies on these tasks to still function. For example, perf has the line:do_populate_lic[depends] += "virtual/kernel:do_patch"
which will break if the kernel uses EXTERNALSRC and the
do_patchtask
is deleted.Signed-off-by Joshua Watt <[email protected]>--- meta/classes/externalsrc.bbclass | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)meta/classes/externalsrc.bbclass +1
-1@@ -102,7 +102,7 @@ python () { for task in d.getVar("SRCTREECOVEREDTASKS").split(): if local_srcuri and task in fetch_tasks: continue - bb.build.deltask(task, d) + d.setVarFlag(task, 'noexec', '1') d.prependVarFlag('do_compile', 'prefuncs', "externalsrc_compile_prefunc ")Quoted 2 lines
d.prependVarFlag('do_configure', 'prefuncs',
"externalsrc_configure_prefunc")There are a few things I worry about with this:
a) Its confusing for users as it will now show a fetch/unpack/patch task as part of the dependency tree but they won't actually run.
b) Dependencies for those tasks will be followed
c) There has always been a bit of ambiguity about what should happen with task properties like cleandirs, dirs and others. Whilst these don't run for noexec, I have worries that somehow we'll end up wiping out the external source dir accidentally (e.g. with
do_clean). I think
the noexec move makes this more likely, sadly.externalsrc started out as a 30 line class back in 2012 and its now 235
lines and getting ridiculously complicated.
Quoted 7 lines
I'm starting to wonder if we should change the approach and have it modify
SRC_URIand take a copy of the source. Yes, it'd be fractionally
slower but its getting far too complex trying to handle all the corner
cases. It was meant so developers could quickly hack/test things and its turning into a full development tool/workflow.
Ya, that seems reasonable. I think the tricky part is getting the data copied over in such a way that incremental builds still work (which is 99% of the time the reason I use externalsrc), since some build systems actually care about the timestamps. Hmmm.... maybe something with rsync would work.... although, it would be a bit of a strange host tool dependency.
Quoted 4 lines
I think we do need to take a step back and try and come up with a way to simplify it...
Quoted 8 lines
We should also think about whether there are changes to the core which
would make things like this easier. I do have a fairly strong belief that we should make the system more robust and able to handle tasks being missing, its now a lot better at handling that than it used to be. Chris as talked about a "srcready" type task and reworking the fetch/unpack/patch process and I can see the reasoning for that, it would help here too.
Another idea I had (dunno if it is actually *good*): perhaps using EXTERNALSRC should add some thing to OVERRIDES when it is enabled? That way at least the corner cases can be codified close to the code they modify instead of having to live generically in externalsrc.bbclass
Quoted 3 lines
Not sure whether Paul has any thoughts on this as externalsrc will significantly impact devtool too...
I don't use devtool much, but I do use externalsrc all the time. If I get a bit of down time, I might try to take a look at it.
Quoted 8 lines
Cheers,
Richard
Quoted 15 lines · NZDT Richard Purdie
Quoted 8 lines · Joshua Watt
Set the noexec flag to prevent tasks from executing instead of deleting them. This allows inter-tasks dependencies on these tasks to still function. For example, perf has the line:
do_populate_lic[depends] += "virtual/kernel:do_patch"
which will break if the kernel uses EXTERNALSRC and the
do_patchtask is deleted.There are a few things I worry about with this:
a) Its confusing for users as it will now show a fetch/unpack/patch task as part of the dependency tree but they won't actually run.
Quoted 7 lines
b) Dependencies for those tasks will be followed
c) There has always been a bit of ambiguity about what should happen with task properties like cleandirs, dirs and others. Whilst these don't run for noexec, I have worries that somehow we'll end up wiping out the external source dir accidentally (e.g. with
do_clean). I think the noexec move makes this more likely, sadly.
I definitely agree we don't want this noexec. FYI we have hit issues of this nature before with externalsrc - in fact we add a dependency between do_configure and do_unpack here to cover do_patch being deleted:
http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=a5047717bd11d332fc9f4320beaa26719de1793c
Quoted 20 lines
externalsrc started out as a 30 line class back in 2012 and its now 235 lines and getting ridiculously complicated.
I'm starting to wonder if we should change the approach and have it modify
SRC_URIand take a copy of the source. Yes, it'd be fractionally slower but its getting far too complex trying to handle all the corner cases. It was meant so developers could quickly hack/test things and its turning into a full development tool/workflow.I think we do need to take a step back and try and come up with a way to simplify it...
We should also think about whether there are changes to the core which would make things like this easier. I do have a fairly strong belief that we should make the system more robust and able to handle tasks being missing, its now a lot better at handling that than it used to be. Chris as talked about a "srcready" type task and reworking the fetch/unpack/patch process and I can see the reasoning for that, it would help here too.
Quoted 2 lines
Not sure whether Paul has any thoughts on this as externalsrc will significantly impact devtool too...
I know you're unhappy with the state of the externalsrc class, and it's certainly true that it has grown organically since we first implemented it. A bunch of the things we have added in there have been for the benefit of devtool users (where there is an intended workflow) and I can see that it's possible be getting in the way of folks who are using externalsrc on its own.
I'm not against simplifying externalsrc per se, but we need to be sure we are making the right tradeoffs. As far as devtool usage is concerned I'd really prefer not to do something that involves source tree copying, for the following reasons:
1) With large source trees it can only perform worse than currently. I'm already a bit concerned about the turnaround time for repeat builds with externalsrc on the kernel for example, and if you take it to the extreme with something like webkit or Qt it'll be even more noticeable.
2) We would be making things more complicated for the user. For example, error messages will be mentioning the copy path not the source path the user knows about, potentially leading the user to edit the wrong files and then wonder what is going on when those changes get overwritten. If S==B then it seems to me it'll be worse - I think you'd have to delete any partial build results each time or you couldn't be sure you'd synchronised the source trees.
3) If we were to add in mitigations for the two issues above, we're going to start to build a monster similar to the one we're trying to slay.
4) We actually already have a solution where you get a copy of the source:
file://. You might need to build on top of that a little to get something close to what externalsrc does, of course, but it's there.
I know fragility is only one aspect of your concerns, but the solution to that aspect at least is more focused tests. We exercise externalsrc currently via devtool's oe-selftest tests a little, but it's not a complete proof of externalsrc's functionality as it exists today.
If we need to we could also split out the things we added for devtool to a separate class, however the difficulty would be deciding where the line is between things externalsrc-only users need and things they don't.
Cheers,
Paul
Signature · 2 lines
Paul Eggleton Intel Open Source Technology Centre