Display unbottled dependencies when skipping #717
Conversation
We currently have a few places elsewhere that check for `:all` bottle dependencies separately. We can simplify those by building the `:all` bottle logic into the `#bottled?` method.
We also no longer need to handle the `:all` bottle case separately after e41d68e. Closes Homebrew#705.
The logic for `:all` bottles is now in the `#bottled?` method. We also remove `no_older_versions: true` since this is handled in the `#install_dependent` method.
If the `--build-dependents-from-source` flag is passed, then all dependents are built from source. This is the current behaviour. We change behaviour in two ways: 1. Build unbottled dependents from source if all its dependencies are either bottled, or were built earlier in the `test-bot` run. 2. Build Linux-only formulae from source too (either with the appropriate flag or when the above condition is true) instead of always skipping source builds on Linux. Building unbottled dependents will be useful for detecting dependents which should be bottled after Homebrew#714. This change should also allow us to clean up a bit of code in the `#dependent_formulae!` method. I've omitted that for now since I have a pending PR (Homebrew#717) which introduces conflicting changes.
| if formula.bottle_specification.tag?(Utils::Bottles.tag(:all)) | ||
| formula.deps.all? { |dep| bottled?(dep.to_formula, no_older_versions: no_older_versions) } |
There was a problem hiding this comment.
Should this be recursive deps? Feels like a comment explaining why this is done would be useful.
There was a problem hiding this comment.
No need. This procedure guarantees that the recursive dependencies are bottled too because a formula won't be bottled unless its dependencies are. (The handful that were accidentally bottled during mass bottling have been bumped since.)
We only need to recurse further up the tree whenever we find an :all bottle, but that's what this method does already.
There was a problem hiding this comment.
This procedure guarantees that the recursive dependencies are bottled too because a formula won't be bottled unless its dependencies are. (The handful that were accidentally bottled during mass bottling have been bumped since.)
This may be the case in core, but it is not the case in osrf/simulation right now. There are some formulae with big_sur bottles that were built before Nov 3 against catalina or earlier bottles of at least one dependency. Now, when rebuilding these formulae, they lose their big_sur bottle since their dependency chain isn't completely bottled on big_sur (for example osrf/homebrew-simulation@140ece0).
I'll try to test and find a reproducible test case from the osrf/simulation tap. Thanks for you help with all this!
There was a problem hiding this comment.
Changing deps to recursive_dependencies here only makes your problem worse, not better, though. This discussion is also about formulae with :all bottles, which I guess you don't have yet?
There was a problem hiding this comment.
fair enough; I don't fully understand all this code. thanks for your patience
we don't yet have :all bottles; we need to resolve osrf/homebrew-simulation#1682 before we will get :all bottles automatically from test-bot
| if formula.bottle_specification.tag?(Utils::Bottles.tag(:all)) | ||
| formula.deps.all? { |dep| bottled?(dep.to_formula, no_older_versions: no_older_versions) } |
If the `--build-dependents-from-source` flag is passed, then all dependents are built from source. This is the current behaviour. We change behaviour in two ways: 1. Build unbottled dependents from source if all its dependencies are either bottled, or were built earlier in the `test-bot` run. 2. Build Linux-only formulae from source too (either with the appropriate flag or when the above condition is true) instead of always skipping source builds on Linux. Building unbottled dependents will be useful for detecting dependents which should be bottled after Homebrew#714. This change should also allow us to clean up a bit of code in the `#dependent_formulae!` method. I've omitted that for now since I have a pending PR (Homebrew#717) which introduces conflicting changes.
Also, simplify handling of
:allbottles.Closes #705.