ninja: make python a build-only dependency #86257
Conversation
carlocab
added
do not merge
CI-skip-recursive-dependents
carlocab
changed the title
|
Tested locally by building |
|
This is weird: I think this is related to the Slack discussion about build dependencies of build dependencies being installed unnecessarily. CC @Bo98 |
|
Looks like a regression in |
|
Ah wait I think that's intentional behaviour. I'll look into tweaking it however. |
|
Basically |
Ninja doesn't actually need Python to build anything. It uses Python only for generating graphs, so there's no point requiring everyone to install Python as well. This is useful for users with non-default prefixes, since this is relocatable but Python is not. It's also useful for CI, because formulae that use Ninja to build will no longer need to install Python as well unless they depend on Python in some other way. Also, remove the Curl dependency, as the test probably still works with system Curl. While we're here, let's install some more useful files from the tarball.
| doc.install "doc/manual.asciidoc" | ||
| elisp.install "misc/ninja-mode.el" | ||
| (share/"vim/vimfiles/syntax").install "misc/ninja.vim" |
There was a problem hiding this comment.
These things seemed useful to install, but probably not worth a revision bump.
|
|
||
| depends_on "python@3.9" | ||
|
|
||
| uses_from_macos "curl" => :test |
There was a problem hiding this comment.
@iMichka where did you see the error that made you add this again? The test seems to pass on Linux even without this now.
There was a problem hiding this comment.
I did see #84724, which is why I tagged you here -- that doesn't quite answer my question. That PR answers what the error was, but it doesn't say anything about where that error came from.
There was a problem hiding this comment.
I don’t know. I saw an error, I fixed it. I thought it was the right thing to do. I did not investigate the root cause. It just looked like a dependency was missing, so I added it, and the error was gone. Maybe the fix was the wrong one, and if we have a better solution, please go ahead 😉
There was a problem hiding this comment.
I imagine the issue might happen if curl is installed in the CI but a dependency of curl is uninstalled (homebrew-test-bot is not designed really to rely on formulae like that - hence our issues with using HOMEBREW_FORCE_BREWED_CURL).
A fixed /usr/bin/curl reference might work.
|
🤖 A scheduled task has triggered a merge. |
4 participants
brew install --build-from-source <formula>, where<formula>is the name of the formula you're submitting?brew test <formula>, where<formula>is the name of the formula you're submitting?brew audit --strict <formula>(after doingbrew install --build-from-source <formula>)? If this is a new formula, does it passbrew audit --new <formula>?See discussion at #86250.