Remove unresolvable names from buildreq_cache #563
Conversation
There are a few places where set.discard() is called instead of set.remove(), but the intention of set.remove() is more clear. Add helper functions for set.remove() that can be called to avoid the try/except at call sites. Call sites will be updated in the next commit. Signed-off-by: Patrick McCarty <[email protected]>
Signed-off-by: Patrick McCarty <[email protected]>
The buildreq cache should only contain names/symbols/etc that are resolvable at build time. The names may indicate missing packages, but they may also be invalid, so it is best to avoid adding the names to the cache in the first place. Fixes clearlinux#480 Signed-off-by: Patrick McCarty <[email protected]>
phmccarty
force-pushed
the
cache-root-log
branch
from
0fef8b5 to
d69bf90
Compare
| name = match.group(1) | ||
| util.print_fatal("Cannot resolve dependency name: {}".format(name)) | ||
| # Avoid adding the dependency name to the buildreq cache | ||
| buildreq.remove_buildreq(name) |
There was a problem hiding this comment.
Hrm so this removes it from the buildreq and rebuildreq_cache. Ideally we want to see if this is from the buildreq_cache or the buildreq_add and if it is in the cache just remove it and retry the build. If it is from the buildreq_add then it is fatal and the build should halt. Unfortunately we don't really keep the original file contents themselves around and just the consolidated sets.
There was a problem hiding this comment.
Thanks, I had not considered the interaction between buildreq_add and buildreq_cache here..
So, the cases to handle are:
- Unresolvable name in buildreq_add only -> halt build
- Unresolvable name in buildreq_cache only -> eject from cache and start new build round
- Unresolvable name in both files -> eject from cache and halt build
Does that look right?
There was a problem hiding this comment.
Unfortunately we don't really keep the original file contents themselves around and just the consolidated sets.
I didn't understand what you meant here at first, but after testing, it's very clear now :-P
I would like to solve that problem, too, but not sure how to go about it yet.
|
I will split the last commit into a new PR, since that's a bug fix for #562 |
phmccarty
force-pushed
the
cache-root-log
branch
from
d69bf90 to
182dc05
Compare
|
I'm not entirely sure how I want to account for the gaps William mentioned. I will open a new PR at a later time once I determine a better solution. |
Fixes #480
buildreq_ban,pkgconfig_ban, andrequires_ban