Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should check for unlinked dependencies of all dependents, not just named formulae #544

Closed
scpeters opened this issue Dec 25, 2020 · 4 comments · Fixed by #659
Closed

Should check for unlinked dependencies of all dependents, not just named formulae #544

scpeters opened this issue Dec 25, 2020 · 4 comments · Fixed by #659

Comments

@scpeters
Copy link
Member

While investigating CI failures in Homebrew/homebrew-core#67615 (applying an upstream patch to boost), I noticed that brew test pcl failed for all architectures, even though it had passed very recently in Homebrew/homebrew-core#66792. The tests in Homebrew/homebrew-core#67615 failed to find eigen when running cmake to configure some sample code that links against pcl. Searching earlier in the console log, I noticed brew unlink eigen just before freeling was tested. Looking at the console logs from Homebrew/homebrew-core#66792, I noticed the same call to brew unlink eigen just before freeling, but there was also a call to brew link eigen just before pcl was handled. The difference between those two pull requests is that pcl received a revision bump in the earlier one, and there is logic in setup_formulae_deps_instances() in lib/tests/formulae.rb that calls brew link on unlinked dependencies for any formulae to be built. I believe this logic should be added to link dependencies for any dependents that are installed, not just the explicitly named formulae to be tested.

@MikeMcQuaid
Copy link
Member

The difference between those two pull requests is that pcl received a revision bump in the earlier one, and there is logic in setup_formulae_deps_instances() in lib/tests/formulae.rb that calls brew link on unlinked dependencies for any formulae to be built. I believe this logic should be added to link dependencies for any dependents that are installed, not just the explicitly named formulae to be tested.

Makes sense to me @scpeters! Could you open a PR?

@scpeters
Copy link
Member Author

The difference between those two pull requests is that pcl received a revision bump in the earlier one, and there is logic in setup_formulae_deps_instances() in lib/tests/formulae.rb that calls brew link on unlinked dependencies for any formulae to be built. I believe this logic should be added to link dependencies for any dependents that are installed, not just the explicitly named formulae to be tested.

Makes sense to me @scpeters! Could you open a PR?

I'll work up the courage and give it a shot 😅

@BrewTestBot
Copy link
Member

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@Homebrew Homebrew locked as resolved and limited conversation to collaborators Mar 1, 2021
@scpeters scpeters reopened this Sep 2, 2021
@scpeters
Copy link
Member Author

scpeters commented Sep 2, 2021

potential fix in #659

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants