-
-
Notifications
You must be signed in to change notification settings - Fork 10.7k
utils/path: allow loading API source cache #20988
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for loading packages from the Homebrew API source cache directory when API installation is enabled. This allows the loadable_package_path? method to accept paths from the API source cache as valid package locations.
- Adds API source cache path to allowed paths when API installation is enabled
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rylan12
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
Although I wonder, should we be reading the post-install block from the cache or from the newly installed keg?
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thanks!
ZhongRuoyu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
@ZhongRuoyu Yes, probably. Can you or @Bo98 suggest an alternate solution here? |
|
Actually: let's merge this as-is for now and we can address afterwards. I don't really see a way of handling this without that. If we want to ensure that specific security cases that rely on multiple overlapping environment variables cannot be handled: that should be done with a regression test. |
I think we can load the |
brew stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?To fix #20984
Before
After