brew
brew copied to clipboard
brew.sh: create `current` symlink in `pkgconfig` directory
- [x] Have you followed the guidelines in our Contributing document?
- [x] Have you checked to ensure there aren't other open Pull Requests for the same change?
- [x] Have you added an explanation of what your changes do and why you'd like us to include them?
- [ ] Have you written new tests for your changes? Here's an example.
- [x] Have you successfully run
brew style
with your changes locally? - [ ] Have you successfully run
brew typecheck
with your changes locally? - [ ] Have you successfully run
brew tests
with your changes locally?
pkg-config
currently hardcodes the OS version it was built on in its
pc path in order to find the .pc
files shipped with Homebrew. This
requires reinstallation of pkg-config
with OS upgrades, which is
something we currently do not handle.
We can avoid this by automatically creating a current
symlink that
points to the right directory whenever brew.sh
runs. This will require
a rebuild of pkg-config
that points it to the current
directory.
In order to simplify identifying the right directory to symlink to, I've
dropped the patch version from HOMEBREW_OS_VERSION
on Catalina and
earlier. I don't think this should break anything, but I can try a
different approach if this will cause problems.
Review period will end on 2022-08-09 at 00:00:00 UTC.
pkg-config
rebuild: https://github.com/Homebrew/homebrew-core/pull/107528
This requires reinstallation of pkg-config with OS upgrades, which is something we currently do not handle.
@Bo98 had done some work in this direction. What's the latest here @Bo98?
No idea if it's the right place to put this code or not, but functionally this looks good to me.
Yeh, I'd rather this was something that was done inside Ruby-land rather than Bash here (given no Bash commands rely on this). Feels like perhaps in e.g. formula_installer.rb
?
@Bo98 had done some work in this direction. What's the latest here @Bo98?
The data is now all there (compare the built_on
OS in the tab to the running OS). The two main things left was:
- A DSL so that we could mark which formulae should have this behaviour.
- Make
brew upgrade
actually work with formula that are marked as outdated like this, since it currently only supports pkg_version changing updates. My initial thought is this could be worked around by appending the macOS version to the formula version number. But this by chance happens to be the same problem as #13661 so maybe something more generic can be done instead.
Even if we end up not needing this for pkg-config, there are other formulae which would find this functionality useful.
- But this by chance happens to be the same problem as formula: ensure that kegs with
version_scheme == -1
are outdated #13661 so maybe something more generic can be done instead.Even if we end up not needing this for pkg-config, there are other formulae which would find this functionality useful.
Agreed, feels like a general solution might be nice here.
Review period ended.
Agreed, feels like a general solution might be nice here.
I agree with aiming for a general solution where we reinstall formulae with OS upgrades when needed.
However, the need to reinstall pkg-config
is because we hardcode the OS version in its search path. A solution where we don't do that probably won't be general, but is better than reinstalling to fix the hard-coded OS version.
Yeh, I'd rather this was something that was done inside Ruby-land rather than Bash here (given no Bash commands rely on this). Feels like perhaps in e.g.
formula_installer.rb
?
We can move it into Ruby-land, but I don't think formula_installer
is the right place.
The symlink can become stale due to things happening outside of brew
, so waiting for another brew install
before updating it means that users could be stuck with a stale symlink for longer than necessary.
My idea was that we want to make sure this symlink is updated with every brew
invocation, and the closest I could come to that was putting it here.
We can move it into Ruby-land, but I don't think
formula_installer
is the right place.
I'm fine with it being in a different place.
My idea was that we want to make sure this symlink is updated with every
brew
invocation, and the closest I could come to that was putting it here.
Do we have other things that e.g. make brew list
make changes on disk? That feels a bit weird to me.
To me only brew install/upgrade/reinstall/uninstall/autoremove/cleanup are what I'd expect to write changes on disk like this. brew doctor could warn if it's missing.
I think of the symlink as part of the internal state of brew
, it just so happens to live on-disk. We also write to the disk when modifying .git/config
(e.g. when running a dev command, or on any invocation of brew
where homebrew.analyticsmessage
is true
but homebrew.analyticsuuid
is unset), and I view this as similar to those.
But I don't mind moving this into brew update
. I'm going to do that.
@carlocab would be good to get this in for 3.6.0 if you get the chance 🙇🏻
How soon did you want to get 3.6.0 out? Will try to get this ready with sufficient time for review before then.
@carlocab Not that soon! A week or two?
Ok, will work on this tomorrow. Feel free to nudge if you don't hear anything by Thursday.
Apologies for the delay here; I was mulling over how specific this solution really is to pkg-config
, and it seems we have a bunch of symlink handling elsewhere too (e.g. Linux-preferred GCC, but we want to get rid of those).
I wonder if
- there's a more general approach that handles the necessary symlinking
- creating symlinks might be how we might want to avoid the reinstall-at-OS-upgrade problem in other formulae too (e.g. a symlink to the correct SDK inside
HOMEBREW_PREFIX
?)
2. creating symlinks might be how we might want to avoid the reinstall-at-OS-upgrade problem in other formulae too (e.g. a symlink to the correct SDK inside
HOMEBREW_PREFIX
?)
TBH I think we should just handle the reinstall-at-OS-upgrade next time brew install
/brew upgrade
or maybe even brew update
is run after an OS upgrade?
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.