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
Bottle path substitution when HOMEBREW_REPOSITORY == HOMEBREW_PREFIX #9453
Comments
It seems like a bug to me: in @Homebrew/brew do you think this is a bug indeed? |
Any chance that the native ARM machine had Homebrew installed using |
@sjackman I had my local machine set up the latter way, but @fxcoudert said in #7857 (comment) that we should have HOMEBREW_PREFIX== HOMEBREW_REPOSITORY on ARM. Personally that felt weird, so I put my stuff in |
FWIW I tracked down where the issue appears to be coming from... brew/Library/Homebrew/keg_relocate.rb Line 76 in e416668
It's running replacements.keys.sort_by(&:length).reverse which, since the HOMEBREW_PREFIX and HOMEBREW_REPOSITORY paths are the same length, may not be deterministic in this scenario.
|
@MikeMcQuaid this seems to be a good argument to |
I think Every single unnecessary subdirectory adds up over the years, even if it’s just half a second of extra typing. On top of that, that double (but differently-cased) |
But then, how do we resolve in bottles between the two paths, when they are equal? Right now, that means we would completely remove support for people who have |
@fxcoudert I disagree, this is a bug and we should fix this. This is how we currently direct people to install Homebrew on ARM, too: https://docs.brew.sh/Installation#untar-anywhere
I agree. The |
I think this means I will have to remove all ARM bottles already committed and build them again. I currently don't have a good way to know which are affected by this issue, but probably all of them. That's 165 files and I don't want to have to remove them from bintray manually… does anyone know the API well enough to do that? |
@fxcoudert I'm sorry about that. I would suggest this is a learning opportunity that we need to take our time more when shipping bottles for a new architecture that's never had them before; there's going to be more issues still that need to be fixed here.
We can't really just remove them as anyone who has them downloading already will then have a broken checksum. We'll need to |
A new |
@MikeMcQuaid I only committed a “small” number of bottles because I wanted to test them out “in the wild”… the problem was not visible in my original testing, and was only made visible because of another user with a system that has HOMEBREW_REPOSITORY != HOMEBREW_PREFIX
We never advertised them as supported, anyway, so I'd prefer that option to revision-bumping those formulas for all other users (when there is no content changed).
That's another option, of course |
@fxcoudert Ok, can you remove the bottle values ASAP then. As for Bintray: you should be able to hack something together using |
Bottles gone from homebrew-core and bintray. For the bintray part, in case we ever have to do that again:
|
I'm just a user here voicing my two cents, but it just feels weird to me to have both a repository root and a standard layout of |
I agree with William here. I would prefer to have
Homebrew on macOS for Intel and Homebrew on Linux both use I would like to bring this issue to the attention of the @Homebrew/tsc for review. I recommend not building any bottles for Homebrew on macOS for ARM until this issue has been decided. |
This is already the case, we have stopped bottling until this is resolved.
To me this is the key point. @MikeMcQuaid stated it is possible to discriminate, if possible I don't have an objection. (It is probably possible because each subdirectory is unambiguously either part of the prefix or the repo.) |
I agree with what @sjackman said above: #9453 (comment) We should have If we are able to fix the bug, all should be good. |
If we absolutely must go that route, please let’s at least pick something more concise like |
By default the installer installs to this location on macOS and Intel, correct. However:
Given 3. we need to make these changes anyway.
I am volunteering to do the work to fix this.
Yes. This is not a problem provided we add the necessary checks.
This is not an objective discussion being brought to the TSC. This is the TSC being brought in to simply overrule me and the work I've been doing. I was going to ensure that I fixed the relevant issues today but it seems now that I should instead wait for the TSC to make a decision before I do any more work on this, it seems. This is not a productive way for an open source project powered by volunteer time and motivation to function. |
I agree this is not a good way to involve the TSC. There was a discussion about prefix choice on ARM, it concluded with a decision that is currently documented and that leads to HOMEBREW_PREFIX == HOMEBREW_REPOSITORY. Those points were not raised and we are now implementing that decision, with significant work already being done in that direction. Three of the main actors involved in this work (@MikeMcQuaid @claui and I) favour the situation we are now implementing (even though I was not originally entirely convinced by that choice of prefix, I now think it was the right call). |
@MikeMcQuaid Regardless of anyone’s position, including mine: |
Thanks @fxcoudert.
I don't think anyone has bad intentions here, I agree. I do think that opinions should be as strong as the willingness of the person proposing them to implement them in a timely fashion. This has not been happening consistently and I find this frustrating. I think individuals can act in good faith and intend to serve the project's goals and end up achieving the opposite.
There has been 6 referrals to the TSC since October. In every case: it’s been to overrule a decision I stated or code I’ve already written. In 5/6 cases: the referrals have been by PLC members. In the times when there’s been agreed follow-up work that’s needed done: I have done so every time, even when I disagreed with the outcome. This has not consistently been the case otherwise.
I do feel this way, yes. Regardless, I appreciate the apology and want to make clear I don't see you as part of the problem here @claui. |
I'm not going to do this, after all. If the TSC wants to overrule and make changes otherwise: fine with me. Until then I'm going to keep on doing the actual work rather than talking about it. |
Can somebody summarise the differents paths (current and future) on the 3 platforms? I am now completely lost. From what I understand, we are getting rid of the "Homebrew" subfolder and will have Readme.md and other src files in /usr/local, /home/linuxbrew/.linuxbrew and /opt/homebrew. Will there be a migration script to move old installs to this new layout? But maybe I misunderstood the different options. |
There is no proposal to change Using https://docs.brew.sh/Installation#untar-anywhere to install into |
I'll summarise myself then. Here is the proposal (with some example paths, so we do not talk past each-other): Option 1:
Option 2:
And @MikeMcQuaid is in favour of option 1. One question, the 2 bin folders in option 1 are merged together, will this not cause an issue? |
@iMichka Yes, those two options are correct. It can be summarised down to just being ARM because that's all we're talking about. I also think Linux shouldn't have the
No, it actually removes complexity. Can someone split this into a new issue? I don't think this (closed, user-submitted) issue is the best place for discussing this. |
Ok so I changed my mind, given the above. Actually it's just "/usr/local/" on Intel that is special, and ARM and Linux should be the same. I'm fine with option 2 then, if we keep track somewhere that we want to change this for Linux too one day, for consistency. Let me call this Option 3 and this is the ideal situation we would end with:
|
I would highlight that “option 2” is the currently agreed-upon, already documented, choice. It's what I've used to install, set up, and test CI nodes on ARM Big Sur. Going back now will cause significant extra work and delay of the ARM availability. The end user consequences (“it will make /opt/homebrew more cluttered for users") are, in my opinion, weak and do not seem to justify disturbing on-going work. |
Just a thought, hope it's not unwelcome: Making sure Admittedly, I know I'm not doing any of the actual work here; just thought some input might be helpful. |
This decision of whether |
Just from looking at the options, I think option 2 is the cleanest approach, since regardless of platform, |
I was wondering the same thing just now. I can't think of any reason. |
Three that come to mind are…
References to the shim compiler and scm scripts: References to the |
I've confirmed with Homebrew 2.6.1 that when the bottle is built with |
The bottle for |
One compromise option is available. CI can build bottles using I would expect an installation with |
I don't have a comment on which option to use, but I do have a proposal for how to handle the Right now we have three placeholders that we use:
As a hypothetical long-term goal, we may want to remove If we replace the references to Not totally sure exactly what changing to using this would take. I think we could do this while avoiding the need to rebuild bottles for all formulae. If we start to build new bottles that use Hypothetical plan for implementation;
It may be that totally removing I'd be willing to help out with the planning/implementation of this. If I'm totally off-base with this disregard my suggestion. If this is something we're interested in, we'll need a separate issue for discussion because this one has been resolved and closed. Edit: looks like Shaun also posted a (much simpler) suggestion for this at the same time 😄 |
In my opinion it would be best to use separate variables for |
Mike, first of all, thank you very much for the fix in #9481. I appreciate your putting the work into resolving this. At this point it sounds like option 2 has developer support and is what we've already documented. |
The first one really should not happen (we have an audit for that in place, don't we?). For pkg-config files, I also think it should probably be avoided, shouldn't it? |
I thought option 2 would be to have |
@reitermarkus I've removed my post, I may have gotten the option numbers confused. |
There is actually an example of why I think this got in the wrong way to involve the TSC, and leads to confusion rather than clarity. There was a choice of prefix discussed already, and we are implementing it, as documented. If it is felt that there is need to change things, then:
I'd like to highlight that even now, after we stopped bottling, there are many hours (many dozens, in my case) of work from several maintainers that have been invested into implementing what the consensus was. It is already late in the process, and I would hope that we don't reverse course (and make all that work useless) unless there are very strong incentives. And to be clear: right now, the arguments that have been discussed are mostly “there are existing bugs for this configuration” (sure, and they can be fixed) or subjective arguments about what is “cleaner” or “less confusing”. |
Thanks @mistydemeo, I appreciate it. Repeating myself from yesterday:
It's unclear to me based one the above that anyone besides @fxcoudert and @mistydemeo has looked at (or even noticed) #9481. That fixes the bug referenced in this issue. If you have other bugs with the current behaviour: please open a reproducible issue. If you want to propose an alternative case: please open a PR with the code change you propose. |
I've opened a PR at #9503 |
For the record, the TSC decision is to proceed with the current choice (leading to |
First report here: #7857 (comment)
The text was updated successfully, but these errors were encountered: