-
-
Notifications
You must be signed in to change notification settings - Fork 10k
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
brew.sh: enable autoupdate for Homebrew developers #429
brew.sh: enable autoupdate for Homebrew developers #429
Conversation
Autoupdate has been working well for me/opt-in folks for a while so lets enable it for Homebrew developers to test before we enable it for everyone.
Yeah, let's dogfood it. |
I hit about 7 seconds as well, FWIW. I'm not completely enamoured with blending developer & auto-update, but agree it's probably the easiest way for it to be more widely tested. |
Not thrilled with this idea to put it mildly, since the first thing that comes to mind is planning to defeat it effectively. |
(this is why I wanted to roll this out more widely) brew/Library/Homebrew/cmd/update.sh Line 389 in e3b6c96
It's perhaps worth changing that and seeing if it speeds things up (it almost certainly will). We could consider avoiding that case on the autoupdate (i.e. unless anything has changed rather than doing it unconditionally for developers). |
|
How many taps do you have? Check out |
24 taps. |
All on GitHub or not? Mind playing around and see if you can figure out the slow bit? I assume it's the |
|
Here are my numbers for two different Homebrew installations on my system: /opt/brewery/tests $ /usr/bin/time brew update --preinstall
20.10 real 12.73 user 8.46 sys
/opt/brewery/tests $ /usr/bin/time brew update --preinstall
19.74 real 12.75 user 8.26 sys
/opt/brewery/tests $ brew tap | wc -l
28 /opt/brewery/dummy $ /usr/bin/time brew update --preinstall
2.17 real 1.21 user 0.91 sys
/opt/brewery/dummy $ /usr/bin/time brew update --preinstall
1.83 real 1.16 user 0.70 sys
/opt/brewery/dummy $ brew tap | wc -l
1 Notice that it's not even updating anything, probably because my remotes are a bit unconventional (none of them start with So whatever takes that long is probably happening locally … 20 seconds delay on every |
That's a worst-case. The typical Homebrew user is not using any non-core taps. Perhaps we could try to e.g. only updating the taps that are being requested for installation? |
It seems to be the git pull time plus about three seconds:
|
Sure, that's true. But if we want to test this on ourselves, it's going to be annoying at times. And I definitely use that 28-tap repository regularly to try out things where I'd like to have the full picture of all formulae from all Homebrew-hosted taps.
Maybe before we start optimizing, we try to find out what is slowing down the process this much? Sadly, I'm not aware of a good solution for profiling shell/Bash code, but I sure would like to find one, as it might come in handy on other occasions, too. |
I think it's just doing full git pulls. Those numbers don't look like a coincidence to me. |
@UniqMartin You could check your pull time with the same command to see if the finding holds up
|
@ilovezfs I re-ran with |
Well then the script must be busy contemplating the universe, I suppose. |
Hehe yes, you're right. I killed the WiFi and...
|
Looks like it's spending most of the time doing the git stash/git stash pop routine. Without that,
So testing with HOMEBREW_DEVELOPER set is especially unrealistic. |
So with
The output will be something like: $ gstdbuf -oL -eL brew update --debug --verbose --preinstall 2>&1 | ts '%.s'
1467389638.650671 ++ [[ /opt/brewery/tests = \/\u\s\r\/\l\o\c\a\l ]]
1467389638.651005 ++ [[ ! -w /opt/brewery/tests ]]
1467389638.651037 ++ git --version
1467389638.721324 ++ export GIT_TERMINAL_PROMPT=0
1467389638.721429 ++ GIT_TERMINAL_PROMPT=0
1467389638.721489 ++ export 'GIT_SSH_COMMAND=ssh -oBatchMode=yes'
1467389638.721531 ++ GIT_SSH_COMMAND='ssh -oBatchMode=yes'
1467389638.721638 ++ [[ -z 1 ]]
1467389638.721679 ++ QUIET_ARGS=()
1467389638.721716 ++ unset GIT_CONFIG
1467389638.721754 ++ lock update
1467389638.721789 ++ local name=update
1467389638.721824 ++ local lock_dir=/opt/brewery/tests/Library/Locks
1467389638.721863 ++ local lock_file=/opt/brewery/tests/Library/Locks/update
1467389638.721900 ++ [[ -d /opt/brewery/tests/Library/Locks ]]
1467389638.721936 ++ exec
1467389638.721970 ++ exec
1467389638.722004 ++ _create_lock 200
1467389638.722041 ++ local lock_fd=200
1467389638.722076 ++ local ruby=/usr/bin/ruby
1467389638.722109 ++ [[ -x /usr/bin/ruby ]]
1467389638.722142 ++ [[ -n /usr/bin/ruby ]]
1467389638.722264 ++ /usr/bin/ruby -e 'File.new(200).flock(File::LOCK_EX | File::LOCK_NB) || exit(1)'
1467389638.763823 ++ safe_cd /opt/brewery/tests
1467389638.763936 ++ cd /opt/brewery/tests
[…] |
Even the 4 second delay for the installs to start, which would have started instantly before, is not ... fun. |
And attempting to control + C an install leads to some interesting output:
|
Neither are the regular issues we get which are fixed by a |
I think update should be manually invoked or asynchronous, not a synchronous blocker for every install. |
I think if we can compromise between never running My suggestion (though I remember it being shot down in a previous discussion) would be to make |
I think we need to explore asynchronous options more thoroughly, since that's the gold standard these days. |
Installing probably really should block on the update check though, right? |
Not unless it's mission critical (which it most certainly is not) that brew be up-to-the-second up to date, as opposed to having been updated asynchronously within the last x time period. |
As I said, I'm not against enabling the auto-update and I'm not objecting to it. I just don't think it's what I personally want and I can sympathize with some of the points @ilovezfs has brought up.
I was referring to
I think we might have different workflows then. None of my local changes that haven't been merged into
This isn't a criticism of your work. I don't think it makes sense to deal with the full complexity that Git repositories and their working copies can have in |
@MikeMcQuaid there seems to be a bug where preinstall sometimes fails to update my local homebrew/core master and my local origin/master. Full log here: So right now it ends up with the latest commit locally being But the actual latest commit on GitHub is Running it more than once doesn't trigger the update either. |
@ilovezfs This looks correct (or more precisely, is by design). |
@UniqMartin Most of the time it does do the auto update and lists items from core. Are you saying that's an accident because it noticed one of the https's was outdated and then proceeded to do everything, but if it's only one of the ssh's that's outdated it does nothing? |
Also, what in the world is all of the stashing and popping behavior when HOMEBREW_DEVELOPER is set intended to accomplish if what you're saying is true and this is supposed to do nothing with ssh repos? |
Ah now I see what was going on. It was only ever working with my main tree, not the tree I use for pushes. I guess "ideally" it would substitute in the https url for the ssh one in order to do the API check and then use ssh for the actual transactions (I believe this is exactly what the |
We tap using HTTP by default and GitHub uses HTTP by default so we double down on that. If you wanted to implement that approach, though, feel free 👍 |
Gonna ship this now; this doesn't mean we're any more likely to ship the autoupdate feature as-is but just that we want to get more 👀 on this and hopefully more people tweaking bits that are annoying. |
Well here's another annoying bit.
It might be good if it decided it was going to error out due to already installed before spinning its wheels. |
Agreed but unsure how to do that in a preinstall check. In this case: it did update stuff, though. |
Right and then I get to sit through the update a second time after I resolve the it's-already-installed issue :) |
@MikeMcQuaid So I've been thinking about whether there would be any way for auto-update both to run asynchronously and to run every time. If I put aside the corner case where the brew update takes a longer time than the installation of the first item (whether explicit target or dependency) for a moment, it should be possible to have the install and the update run simultaneously in separate threads. If, following the completion of the update, brew determines that any of the items involved in the install have actually been updated, it could go back to the beginning, and start over with the newer version(s). Since most of the time there will be no overlap between what the user is installing and what formulae actually got updated, in most cases there would be no additional time added to the installation and brew install would not block waiting on a pre-install update to finish. The corner case where the update takes longer than the install of the first item could be addressed by not finalizing the install of the first item until the update completes, which would then be the only case where this added time to installs. (Of course, I still think it would be nicer to have this run entirely asynchronously (launchd) or only do it on some arbitrary minimum interval between updates.) |
@ilovezfs That seems like it would be a bit of a nightmare for the code to handle. I've got various profiling tools bookmarked I want to investigate; I suspect this would be not annoying if the no-op case was e.g. 0.1 seconds. |
Why a nightmare? As it is, Ctrl-C already triggers a canceling procedure, no? |
Also if the no-op case included no-op when none of the items being upgraded/installed have updates. It's annoying in the non-no-op case when it does its thing solely because it's updating cask or some obscure formulae upgrades that nutty ilovezfs just pushed, and the non-no-op update had nothing to do with what I'm doing. |
I suspect that might be somehow achievable locally, but would require some serious tuning. But even then this feels like a lot of work. I'm a bit doubtful this can be achieved at all if network access is involved. |
@UniqMartin Sure; I was just throwing a number out there. I think sub a second is probably both doable and a reasonable compromise e.g. by only updating the tap of the things being installed.
I'm more thinking of the "starting compling |
As an initial version, it could probably treat the cancelable/waits-on-update-to-finish event as the download of the first item rather than the install of the first item. |
Regardless we'll still need to have some sort of "kill parent Ruby process from the build process and restart the Bash process" dance that will be fairly involved and hard to debug if it goes wrong. |
Perhaps we'll need a ballet instructor. |
@MikeMcQuaid So I noticed that brew upgrade is running the autoupdate before doing its thing. That seems like it has the potential to create undesirable behavior. If you run |
@ilovezfs Stuff like that people will just end up adapting their flows for, I think. Another option I'm coming around to is making the "only update every X" super short so it's e.g. 10 seconds which would avoid this situation. |
I'm also thinking for |
Is there a command line switch to prevent the auto update? Or are you suggesting people would need to adapt by doing |
IMO, at least Possibly makes sense to calculate the recursive dependencies & update those taps only, but that's not exactly a breezy quick process. |
Autoupdate has been working well for me/opt-in folks for a while so lets enable it for Homebrew developers to test before we enable it for everyone.
brew tests
with your changes locally?Autoupdate has been working well for me/opt-in folks for a while so lets enable it for Homebrew developers to test before we enable it for everyone.
CC @Homebrew/maintainers