Allow specific --only-binary and --no-binary packages to override :all:
#4067
Conversation
|
In the future, we may want to combine edit: See #4284 |
| } | ||
|
|
||
| /// Overlapping usage of `--no-binary` and `--only-binary` | ||
| // TODO(zanieb): We should have a better error message here |
There was a problem hiding this comment.
Going to open an issue for this one
| // Allow `all` to be overridden by specific build exclusions | ||
| NoBuild::Packages(packages) => !packages.contains(&filename.name), | ||
| _ => true, | ||
| }, |
There was a problem hiding this comment.
Could we instead create a struct that couples NoBinary and NoBuild, and lets us query for the no_binary or no_build status, abstracting away this repeated logic? It seems really easy to miss a site as-is.
There was a problem hiding this comment.
Yeah we should totally do that as noted at #4067 (comment) but I'd rather do it in a follow-up.
I didn't want to invest in a refactor until we were on the same page about the behavior. I could do it here too if you think this makes sense.
There was a problem hiding this comment.
I think the behavior seems ok. Do you want to do it in this PR or na?
There was a problem hiding this comment.
I don't really mind doing it here or at least writing the follow-up before merge.
There was a problem hiding this comment.
See #4284 — opted for a separate pull request for review purposes.
zanieb
force-pushed
the
zb/only-no-binary
branch
from
c008d00 to
dc0e04a
Compare
Updates
--no-binary <package>to take precedence over--only-binary :all:and--only-binary <package>to take precedence over--no-binary :all:.I'm not entirely sure about this behavior, e.g. maybe I provided--only-binary :all:later on the command line and really want it to override those earlier arguments of--no-binary <package>for safety. Right now we just fail to solve though since we can't satisfy the overlapping requests.Closes #4063