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

brew.sh: enable autoupdate for Homebrew developers #429

Merged

Conversation

MikeMcQuaid
Copy link
Member

@MikeMcQuaid MikeMcQuaid commented Jun 30, 2016

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • 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.
  • Have you successfully run 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

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.
@BrewTestBot BrewTestBot added the in progress Maintainers are working on this label Jun 30, 2016
@tdsmith
Copy link
Contributor

tdsmith commented Jun 30, 2016

Yeah, let's dogfood it. brew update --preinstall takes several (~7) seconds to execute for me; should I expect that?

@DomT4
Copy link
Member

DomT4 commented Jul 1, 2016

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.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 1, 2016

Josephs-MacBook-Pro:~ joe$ time brew update --preinstall
==> Auto-updated Homebrew!
Updated 1 tap (caskroom/cask).
No changes to formulae.

real    0m15.983s
user    0m11.024s
sys 0m4.427s
Josephs-MacBook-Pro:~ joe$ time brew update --preinstall

real    0m13.027s
user    0m10.393s
sys 0m3.571s

Not thrilled with this idea to put it mildly, since the first thing that comes to mind is planning to defeat it effectively.

@MikeMcQuaid
Copy link
Member Author

(this is why I wanted to roll this out more widely)

-n "$HOMEBREW_DEVELOPER" ]]

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).

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 1, 2016

Josephs-MacBook-Pro:~ joe$ time brew update --preinstall

real    0m13.248s
user    0m9.632s
sys 0m3.553s
Josephs-MacBook-Pro:~ joe$ time brew update --preinstall

real    0m12.139s
user    0m9.632s
sys 0m3.228s
Josephs-MacBook-Pro:~ joe$ git -C /usr/local diff -- Library/Homebrew/cmd/update.sh
diff --git a/Library/Homebrew/cmd/update.sh b/Library/Homebrew/cmd/update.sh
index aafae72..b35d01b 100644
--- a/Library/Homebrew/cmd/update.sh
+++ b/Library/Homebrew/cmd/update.sh
@@ -385,8 +385,7 @@ EOS
   safe_cd "$HOMEBREW_REPOSITORY"

   if [[ -n "$HOMEBREW_UPDATED" ||
-        -n "$HOMEBREW_UPDATE_FAILED" ||
-        -n "$HOMEBREW_DEVELOPER" ]]
+        -n "$HOMEBREW_UPDATE_FAILED" ]]
   then
     brew update-report "$@"
     return $?

@MikeMcQuaid
Copy link
Member Author

How many taps do you have? Check out /usr/bin/time, too.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 1, 2016

24 taps.

@MikeMcQuaid
Copy link
Member Author

All on GitHub or not? Mind playing around and see if you can figure out the slow bit? I assume it's the curl calls. I still think this feature would help us a lot for typical users (and it has an opt-out).

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 1, 2016

https://github.com/Homebrew/homebrew-apache
https://github.com/Homebrew/homebrew-binary
https://github.com/Homebrew/homebrew-bundle
https://github.com/Homebrew/homebrew-command-not-found
https://github.com/Homebrew/homebrew-completions
https://github.com/Homebrew/homebrew-core
https://github.com/Homebrew/homebrew-dev-tools
https://github.com/Homebrew/homebrew-devel-only
https://github.com/Homebrew/homebrew-dupes
https://github.com/Homebrew/homebrew-emacs
https://github.com/Homebrew/homebrew-fuse
https://github.com/Homebrew/homebrew-games
https://github.com/Homebrew/homebrew-gui
https://github.com/Homebrew/homebrew-nginx
https://github.com/Homebrew/homebrew-php
https://github.com/Homebrew/homebrew-python
https://github.com/Homebrew/homebrew-science
https://github.com/Homebrew/homebrew-services
https://github.com/Homebrew/homebrew-tex
https://github.com/Homebrew/homebrew-versions
https://github.com/Homebrew/homebrew-x11
https://github.com/caskroom/homebrew-cask
https://github.com/scribusproject/homebrew-scribus
https://github.com/youtux/homebrew-livecheck

@UniqMartin
Copy link
Contributor

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 https:// though all of them eventually resolve to https://github.com/Homebrew/ after remapping via url."https://github.com/".insteadOf in my .gitconfig).

So whatever takes that long is probably happening locally … 20 seconds delay on every brew install for a no-op doesn't sound exciting just yet. 😉

@MikeMcQuaid
Copy link
Member Author

MikeMcQuaid commented Jul 1, 2016

So whatever takes that long is probably happening locally … 20 seconds delay on every brew install for a no-op doesn't sound exciting just yet. 😉

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?

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 1, 2016

It seems to be the git pull time plus about three seconds:

Josephs-MacBook-Pro:Taps joe$ time brew update --preinstall
==> Auto-updated Homebrew!
Updated 1 tap (homebrew/core).
No changes to formulae.

real    0m18.299s
user    0m16.224s
sys 0m5.768s
Josephs-MacBook-Pro:Taps joe$ (find -E . -type d -regex '^.*\/.git$'|while read d; do (cd $d/.. ; pwd) ; done|while read t; do (cd $t ; pwd ; time git pull) ; done) 2>&1|grep real|sed -e 's/real.*0m//'|tr -d 's' |perl -lne '$x+=$_; END{print $x;}'
15.009
Josephs-MacBook-Pro:Taps joe$ time brew update --preinstall

real    0m18.291s
user    0m16.414s
sys 0m5.792s
Josephs-MacBook-Pro:Taps joe$ (find -E . -type d -regex '^.*\/.git$'|while read d; do (cd $d/.. ; pwd) ; done|while read t; do (cd $t ; pwd ; time git pull) ; done) 2>&1|grep real|sed -e 's/real.*0m//'|tr -d 's' |perl -lne '$x+=$_; END{print $x;}'
16.02
Josephs-MacBook-Pro:Taps joe$ time brew update --preinstall

real    0m18.166s
user    0m16.308s
sys 0m5.739s
Josephs-MacBook-Pro:Taps joe$ (find -E . -type d -regex '^.*\/.git$'|while read d; do (cd $d/.. ; pwd) ; done|while read t; do (cd $t ; pwd ; time git pull) ; done) 2>&1|grep real|sed -e 's/real.*0m//'|tr -d 's' |perl -lne '$x+=$_; END{print $x;}'
21.297
Josephs-MacBook-Pro:Taps joe$ time brew update --preinstall

real    0m18.135s
user    0m16.251s
sys 0m5.719s
Josephs-MacBook-Pro:Taps joe$ (find -E . -type d -regex '^.*\/.git$'|while read d; do (cd $d/.. ; pwd) ; done|while read t; do (cd $t ; pwd ; time git pull) ; done) 2>&1|grep real|sed -e 's/real.*0m//'|tr -d 's' |perl -lne '$x+=$_; END{print $x;}'
15.421

@UniqMartin
Copy link
Contributor

That's a worst-case. The typical Homebrew user is not using any non-core taps.

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.

Perhaps we could try to e.g. only updating the taps that are being requested for installation?

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.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 1, 2016

I think it's just doing full git pulls. Those numbers don't look like a coincidence to me.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 1, 2016

@UniqMartin You could check your pull time with the same command to see if the finding holds up

(find -E . -type d -regex '^.*\/.git$'|while read d; do (cd $d/.. ; pwd) ; done|while read t; do (cd $t ; pwd ; time git pull) ; done) 2>&1|grep real|sed -e 's/real.*0m//'|tr -d 's' |perl -lne '$x+=$_; END{print $x;}'

@UniqMartin
Copy link
Contributor

@ilovezfs I re-ran with /usr/bin/time brew update --debug --verbose --preinstall and I don't see any git pulls being executed and there's no indication of any network traffic either, as far as I can tell. (All my repositories are still not current due to my non-standard origin remote.)

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 1, 2016

Well then the script must be busy contemplating the universe, I suppose.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 1, 2016

Hehe yes, you're right. I killed the WiFi and...

Josephs-MacBook-Pro:Taps joe$ time brew update --preinstall

real    0m17.797s
user    0m17.629s
sys 0m6.264s
Josephs-MacBook-Pro:Taps joe$ time brew update --preinstall

real    0m18.070s
user    0m17.717s
sys 0m6.335s

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 1, 2016

Looks like it's spending most of the time doing the git stash/git stash pop routine. Without that,

Josephs-MacBook-Pro:local joe$ time brew update --preinstall

real    0m4.430s
user    0m3.530s
sys 0m2.351s
Josephs-MacBook-Pro:local joe$ time brew update --preinstall

real    0m4.441s
user    0m3.570s
sys 0m2.365s

So testing with HOMEBREW_DEVELOPER set is especially unrealistic.

@UniqMartin
Copy link
Contributor

So with coreutils and moreutils installed you can create a pretty accurate trace of the output and that might help to identify some bottlenecks (haven't looked into it more closely yet):

gstdbuf -oL -eL brew update --debug --verbose --preinstall 2>&1 | ts '%.s'

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
[…]

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 1, 2016

Even the 4 second delay for the installs to start, which would have started instantly before, is not ... fun.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 1, 2016

And attempting to control + C an install leads to some interesting output:

Josephs-MacBook-Pro:~ joe$ brew install ffmpeg
^C/usr/local/Library/Homebrew/cmd/update.sh: line 245:  2104 Terminated: 15          ( UPSTREAM_REPOSITORY_URL="$(git config remote.origin.url)"; if [[ "$UPSTREAM_REPOSITORY_URL" = "https://github.com/"* ]]; then
    UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY_URL#https://github.com/}"; UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY%.git}"; UPSTREAM_BRANCH_LOCAL_SHA="$(git rev-parse "refs/remotes/origin/$UPSTREAM_BRANCH")"; UPSTREAM_SHA_HTTP_CODE="$("$HOMEBREW_CURL" --silent '--max-time' 3            --output /dev/null --write-out "%{http_code}"            --user-agent "$HOMEBREW_USER_AGENT_CURL"            --header "Accept: application/vnd.github.v3.sha"            --header "If-None-Match: \"$UPSTREAM_BRANCH_LOCAL_SHA\""            "https://api.github.com/repos/$UPSTREAM_REPOSITORY/commits/$UPSTREAM_BRANCH")"; [[ "$UPSTREAM_SHA_HTTP_CODE" = "304" ]] && exit;
else
    if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
        exit;
    fi;
fi; if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
    git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH" 2> /dev/null;
else
    if ! git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH"; then
        echo "Fetching $DIR failed!" >> "$update_failed_file";
    fi;
fi )  (wd: /usr/local/Library/Taps/homebrew/homebrew-python)
/usr/local/Library/Homebrew/cmd/update.sh: line 245:  2115 Terminated: 15          ( UPSTREAM_REPOSITORY_URL="$(git config remote.origin.url)"; if [[ "$UPSTREAM_REPOSITORY_URL" = "https://github.com/"* ]]; then
    UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY_URL#https://github.com/}"; UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY%.git}"; UPSTREAM_BRANCH_LOCAL_SHA="$(git rev-parse "refs/remotes/origin/$UPSTREAM_BRANCH")"; UPSTREAM_SHA_HTTP_CODE="$("$HOMEBREW_CURL" --silent '--max-time' 3            --output /dev/null --write-out "%{http_code}"            --user-agent "$HOMEBREW_USER_AGENT_CURL"            --header "Accept: application/vnd.github.v3.sha"            --header "If-None-Match: \"$UPSTREAM_BRANCH_LOCAL_SHA\""            "https://api.github.com/repos/$UPSTREAM_REPOSITORY/commits/$UPSTREAM_BRANCH")"; [[ "$UPSTREAM_SHA_HTTP_CODE" = "304" ]] && exit;
else
    if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
        exit;
    fi;
fi; if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
    git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH" 2> /dev/null;
else
    if ! git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH"; then
        echo "Fetching $DIR failed!" >> "$update_failed_file";
    fi;
fi )  (wd: /usr/local/Library/Taps/homebrew/homebrew-science)
/usr/local/Library/Homebrew/cmd/update.sh: line 245:  2122 Terminated: 15          ( UPSTREAM_REPOSITORY_URL="$(git config remote.origin.url)"; if [[ "$UPSTREAM_REPOSITORY_URL" = "https://github.com/"* ]]; then
    UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY_URL#https://github.com/}"; UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY%.git}"; UPSTREAM_BRANCH_LOCAL_SHA="$(git rev-parse "refs/remotes/origin/$UPSTREAM_BRANCH")"; UPSTREAM_SHA_HTTP_CODE="$("$HOMEBREW_CURL" --silent '--max-time' 3            --output /dev/null --write-out "%{http_code}"            --user-agent "$HOMEBREW_USER_AGENT_CURL"            --header "Accept: application/vnd.github.v3.sha"            --header "If-None-Match: \"$UPSTREAM_BRANCH_LOCAL_SHA\""            "https://api.github.com/repos/$UPSTREAM_REPOSITORY/commits/$UPSTREAM_BRANCH")"; [[ "$UPSTREAM_SHA_HTTP_CODE" = "304" ]] && exit;
else
    if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
        exit;
    fi;
fi; if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
    git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH" 2> /dev/null;
else
    if ! git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH"; then
        echo "Fetching $DIR failed!" >> "$update_failed_file";
    fi;
fi )  (wd: /usr/local/Library/Taps/homebrew/homebrew-services)
/usr/local/Library/Homebrew/cmd/update.sh: line 245:  2133 Terminated: 15          ( UPSTREAM_REPOSITORY_URL="$(git config remote.origin.url)"; if [[ "$UPSTREAM_REPOSITORY_URL" = "https://github.com/"* ]]; then
    UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY_URL#https://github.com/}"; UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY%.git}"; UPSTREAM_BRANCH_LOCAL_SHA="$(git rev-parse "refs/remotes/origin/$UPSTREAM_BRANCH")"; UPSTREAM_SHA_HTTP_CODE="$("$HOMEBREW_CURL" --silent '--max-time' 3            --output /dev/null --write-out "%{http_code}"            --user-agent "$HOMEBREW_USER_AGENT_CURL"            --header "Accept: application/vnd.github.v3.sha"            --header "If-None-Match: \"$UPSTREAM_BRANCH_LOCAL_SHA\""            "https://api.github.com/repos/$UPSTREAM_REPOSITORY/commits/$UPSTREAM_BRANCH")"; [[ "$UPSTREAM_SHA_HTTP_CODE" = "304" ]] && exit;
else
    if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
        exit;
    fi;
fi; if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
    git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH" 2> /dev/null;
else
    if ! git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH"; then
        echo "Fetching $DIR failed!" >> "$update_failed_file";
    fi;
fi )  (wd: /usr/local/Library/Taps/homebrew/homebrew-tex)
/usr/local/Library/Homebrew/cmd/update.sh: line 245:  2141 Terminated: 15          ( UPSTREAM_REPOSITORY_URL="$(git config remote.origin.url)"; if [[ "$UPSTREAM_REPOSITORY_URL" = "https://github.com/"* ]]; then
    UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY_URL#https://github.com/}"; UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY%.git}"; UPSTREAM_BRANCH_LOCAL_SHA="$(git rev-parse "refs/remotes/origin/$UPSTREAM_BRANCH")"; UPSTREAM_SHA_HTTP_CODE="$("$HOMEBREW_CURL" --silent '--max-time' 3            --output /dev/null --write-out "%{http_code}"            --user-agent "$HOMEBREW_USER_AGENT_CURL"            --header "Accept: application/vnd.github.v3.sha"            --header "If-None-Match: \"$UPSTREAM_BRANCH_LOCAL_SHA\""            "https://api.github.com/repos/$UPSTREAM_REPOSITORY/commits/$UPSTREAM_BRANCH")"; [[ "$UPSTREAM_SHA_HTTP_CODE" = "304" ]] && exit;
else
    if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
        exit;
    fi;
fi; if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
    git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH" 2> /dev/null;
else
    if ! git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH"; then
        echo "Fetching $DIR failed!" >> "$update_failed_file";
    fi;
fi )  (wd: /usr/local/Library/Taps/homebrew/homebrew-versions)
/usr/local/Library/Homebrew/cmd/update.sh: line 245:  2155 Terminated: 15          ( UPSTREAM_REPOSITORY_URL="$(git config remote.origin.url)"; if [[ "$UPSTREAM_REPOSITORY_URL" = "https://github.com/"* ]]; then
    UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY_URL#https://github.com/}"; UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY%.git}"; UPSTREAM_BRANCH_LOCAL_SHA="$(git rev-parse "refs/remotes/origin/$UPSTREAM_BRANCH")"; UPSTREAM_SHA_HTTP_CODE="$("$HOMEBREW_CURL" --silent '--max-time' 3            --output /dev/null --write-out "%{http_code}"            --user-agent "$HOMEBREW_USER_AGENT_CURL"            --header "Accept: application/vnd.github.v3.sha"            --header "If-None-Match: \"$UPSTREAM_BRANCH_LOCAL_SHA\""            "https://api.github.com/repos/$UPSTREAM_REPOSITORY/commits/$UPSTREAM_BRANCH")"; [[ "$UPSTREAM_SHA_HTTP_CODE" = "304" ]] && exit;
else
    if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
        exit;
    fi;
fi; if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
    git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH" 2> /dev/null;
else
    if ! git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH"; then
        echo "Fetching $DIR failed!" >> "$update_failed_file";
    fi;
fi )  (wd: /usr/local/Library/Taps/homebrew/homebrew-x11)
/usr/local/Library/Homebrew/cmd/update.sh: line 245:  2163 Terminated: 15          ( UPSTREAM_REPOSITORY_URL="$(git config remote.origin.url)"; if [[ "$UPSTREAM_REPOSITORY_URL" = "https://github.com/"* ]]; then
    UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY_URL#https://github.com/}"; UPSTREAM_REPOSITORY="${UPSTREAM_REPOSITORY%.git}"; UPSTREAM_BRANCH_LOCAL_SHA="$(git rev-parse "refs/remotes/origin/$UPSTREAM_BRANCH")"; UPSTREAM_SHA_HTTP_CODE="$("$HOMEBREW_CURL" --silent '--max-time' 3            --output /dev/null --write-out "%{http_code}"            --user-agent "$HOMEBREW_USER_AGENT_CURL"            --header "Accept: application/vnd.github.v3.sha"            --header "If-None-Match: \"$UPSTREAM_BRANCH_LOCAL_SHA\""            "https://api.github.com/repos/$UPSTREAM_REPOSITORY/commits/$UPSTREAM_BRANCH")"; [[ "$UPSTREAM_SHA_HTTP_CODE" = "304" ]] && exit;
else
    if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
        exit;
    fi;
fi; if [[ -n "$HOMEBREW_UPDATE_PREINSTALL" ]]; then
    git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH" 2> /dev/null;
else
    if ! git fetch --force "${QUIET_ARGS[@]}" origin "refs/heads/$UPSTREAM_BRANCH:refs/remotes/origin/$UPSTREAM_BRANCH"; then
        echo "Fetching $DIR failed!" >> "$update_failed_file";
    fi;
fi )
Warning: You are using OS X 10.8.
We (and Apple) do not provide support for this old version.
You may encounter build failures or other breakages.
Please create pull-requests instead of filing issues.
==> Installing dependencies for ffmpeg: texi2html, yasm, x264, lame, xvid
==> Installing ffmpeg dependency: texi2html
==> Downloading http://download.savannah.gnu.org/releases/texi2html/texi2html-5.0.tar.gz
==> Downloading from http://nongnu.askapache.com//texi2html/texi2html-5.0.tar.gz
^C
Josephs-MacBook-Pro:~ joe$ 

@MikeMcQuaid
Copy link
Member Author

Even the 4 second delay for the installs to start, which would have started instantly before, is not ... fun.

Neither are the regular issues we get which are fixed by a brew update 😉. It would have "started" instantly before but for most software it's not a large % increase of the overall installation time. We can add a message saying that it's running a self-update, though?

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 2, 2016

I think update should be manually invoked or asynchronous, not a synchronous blocker for every install.

@UniqMartin
Copy link
Contributor

I think if we can compromise between never running brew update automatically and running it on every single brew install invocation, we can achieve basically the same goal (avoid that users forget to run brew update and thus operate with an unnecessarily old package manager and formula database).

My suggestion (though I remember it being shot down in a previous discussion) would be to make brew update remember when it was last executed (e.g. by saving a timestamp to .git/config where we already save other Homebrew-related information). This would allow brew update --preinstall to check this timestamp and to skip the autoupdate if less than 24 hours have passed. If the autoupdate happens at most once a day during a brew install it won't be annoying, even if it takes 30 seconds to complete due to a non-trivial number of taps (as long as there's some output indicating what is going on).

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 2, 2016

I think we need to explore asynchronous options more thoroughly, since that's the gold standard these days.

@tdsmith
Copy link
Contributor

tdsmith commented Jul 2, 2016

Installing probably really should block on the update check though, right?

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 2, 2016

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.

@UniqMartin
Copy link
Contributor

UniqMartin commented Jul 6, 2016

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.

It does already operate on your local repository state; it restores your changes after update.

I was referring to brew install as it is now. And that should definitely not be messing with my brew or my various tap repositories. (That brew update [--preinstall] is doing this is clear.)

It does, actually, because it rebases changes on master and checks out your non-master branches again for you.

I think we might have different workflows then. None of my local changes that haven't been merged into origin/master are in my local master. Essentially, my local master is always in a pristine state. Thus unless I'm overlooking something, that's what I believe is happening when I run brew update: I'm usually on branch dev, then the update switches to master, does a rebase there (which is always a fast-forward merge in my case), then switches back to my dev branch, so the working copy is exactly where it was before the update. (It's just that master is pointing at a different commit now.)

If you can point to a reproducible bug: please let me know so I can fix it. I don't think it's really fair to publicly comment on the work of another maintainer implying that you don't trust it to do what it does without putting in the effort to either fix or reproduce it.

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 brew update. I think the logic in place works well (and is robust) for the typical contributor and user. And if it doesn't and there's a reproducible bug, I'm happy to help solve the issue. But I simply don't want this automatic handling (for the various stated reasons).

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2016

@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:
https://gist.github.com/ilovezfs/6dbe53cd5ae2cc25098efd01df01c96a

So right now it ends up with the latest commit locally being
19e6e3e
passpie: update 1.5.2 bottle.

But the actual latest commit on GitHub is
489c62e
cmake: update 3.6.0 bottle.

Running it more than once doesn't trigger the update either.

@UniqMartin
Copy link
Contributor

@ilovezfs This looks correct (or more precisely, is by design). brew update --preinstall is only checking and auto-updating repositories that are fetched via HTTPS (where the origin remote URLs start with https://github.com/ to be more precise). So in your case it's only ever attempting to update 12 out of all the repositories you have installed. Here's the relevant code in cmd/update.sh.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2016

@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?

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2016

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?

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2016

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 hub command does, btw)?

@MikeMcQuaid
Copy link
Member Author

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 hub command does, btw)?

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 👍

@MikeMcQuaid
Copy link
Member Author

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.

@MikeMcQuaid MikeMcQuaid merged commit 51025a9 into Homebrew:master Jul 8, 2016
@MikeMcQuaid MikeMcQuaid deleted the homebrew-developer-autoupdate branch July 8, 2016 19:30
@BrewTestBot BrewTestBot removed the in progress Maintainers are working on this label Jul 8, 2016
@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 10, 2016

Well here's another annoying bit.

Josephs-MacBook-Pro:~ joe$ brew install -dvs wget
Checking for Homebrew updates...
==> Auto-updated Homebrew!
Updated 3 taps (caskroom/cask, homebrew/core, homebrew/science).
==> Updated Formulae
homebrew/science/adol-c           homebrew/science/colpack          percona-server                  
/usr/local/Library/brew.rb (Formulary::FormulaLoader): loading /usr/local/Library/Taps/homebrew/homebrew-core/Formula/wget.rb
Warning: wget-1.18 already installed

It might be good if it decided it was going to error out due to already installed before spinning its wheels.

@MikeMcQuaid
Copy link
Member Author

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.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 11, 2016

Right and then I get to sit through the update a second time after I resolve the it's-already-installed issue :)

@ilovezfs
Copy link
Contributor

@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.)

@MikeMcQuaid
Copy link
Member Author

@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.

@ilovezfs
Copy link
Contributor

Why a nightmare? As it is, Ctrl-C already triggers a canceling procedure, no?

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 11, 2016

I suspect this would be not annoying if the no-op case was e.g. 0.1 seconds

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.

@UniqMartin
Copy link
Contributor

I suspect this would be not annoying if the no-op case was e.g. 0.1 seconds.

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.

@MikeMcQuaid
Copy link
Member Author

@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.

Why a nightmare? As it is, Ctrl-C already triggers a canceling procedure, no?

I'm more thinking of the "starting compling wget, oh there's an update, cancel compiling wget, relaunch the new Homebrew" etc.

@ilovezfs
Copy link
Contributor

I'm more thinking of the "starting compling wget, oh there's an update, cancel compiling wget, relaunch the new Homebrew" etc.

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.

@MikeMcQuaid
Copy link
Member Author

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.

@ilovezfs
Copy link
Contributor

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.

@ilovezfs
Copy link
Contributor

@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 brew update; brew outdated; brew upgrade you can end up getting more than you expected from inspecting the outdated list unless you're paying careful attention after the upgrade has already started and you're quick on the control-c.

@MikeMcQuaid
Copy link
Member Author

@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.

@MikeMcQuaid
Copy link
Member Author

I'm also thinking for brew install X Y Z we check which taps these map to and only update them (and also perhaps homebrew-core but not e.g. Homebrew/brew)

@ilovezfs
Copy link
Contributor

@ilovezfs Stuff like that people will just end up adapting their flows for,

Is there a command line switch to prevent the auto update? Or are you suggesting people would need to adapt by doing brew update; brew outdated; HOMEBREW_NO_AUTO_UPDATE=1 brew upgrade?

@DomT4
Copy link
Member

DomT4 commented Jul 21, 2016

I'm also thinking for brew install X Y Z we check which taps these map to and only update them (and also perhaps homebrew-core but not e.g. Homebrew/brew)

IMO, at least homebrew/core would have to be done every time, otherwise we're opening the door to seeing tap builds with core dependencies fall apart in all sorts of entirely preventable ways.

Possibly makes sense to calculate the recursive dependencies & update those taps only, but that's not exactly a breezy quick process.

souvik1997 pushed a commit to souvik1997/brew that referenced this pull request Jul 25, 2016
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.
@Homebrew Homebrew locked and limited conversation to collaborators May 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants