Autoupdate tweaks #442

MikeMcQuaid · 2016-07-03T20:37:01Z

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?

Details in the commit messages. Inspired by discussion in #429. CC @ilovezfs @UniqMartin.

UniqMartin · 2016-07-04T07:35:44Z

Library/Homebrew/cmd/update.sh

+      then
+        echo "Fetching $DIR..."
+      fi
+


I think we can skip this. Feels like the preceding “Checking for Homebrew updates...” is enough to make it clear to the user what is going on and this will just make the output unwieldy for a big number of taps (though only the taps with pending updates will trigger this) and may cause overlap in the output due to the parallel nature of this code section.

I'd rather leave this as-is; it was really useful when debugging as otherwise if a particular git fetch is happening/taking a long time it's really hard to figure out which one. Reminder this is for --verbose only. If you'd rather we can make it only for HOMEBREW_DEBUG but I personally want/need this output for debugging.

I guess I'm just finding the following output not very useful, particularly since all the echo calls happen before the actual Git fetches, so this doesn't help understand the (due to parallelism) intermingled output of git fetch. But since I rarely use --verbose in combination with brew update, I don't feel strongly about removing this output.

$ brew update --verbose Fetching /opt/brewery/tests... Fetching /opt/brewery/tests/Library/Taps/caskroom/homebrew-cask... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-aliases... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-apache... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-binary... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-boneyard... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-bundle... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-command-not-found... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-completions... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-core... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-dev-tools... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-devel-only... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-dupes... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-emacs... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-fuse... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-games... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-gui... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-head-only... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-nginx... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-php... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-python... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-science... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-services... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-tex... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-versions... Fetching /opt/brewery/tests/Library/Taps/homebrew/homebrew-x11... Fetching /opt/brewery/tests/Library/Taps/uniqmartin/homebrew-bento... Fetching /opt/brewery/tests/Library/Taps/uniqmartin/homebrew-tools... Fetching /opt/brewery/tests/Library/Taps/xu-cheng/homebrew-portable... remote: Counting objects: 5, done. remote: Total 5 (delta 4), reused 4 (delta 4), pack-reused 1 Unpacking objects: 100% (5/5), done. From https://github.com/Homebrew/homebrew-tex 4f96da55..75885801 master -> origin/master remote: Counting objects: 4, done. remote: Total 4 (delta 3), reused 3 (delta 3), pack-reused 1 Unpacking objects: 100% (4/4), done. From https://github.com/Homebrew/homebrew-x11 e858b375..ed64be3a master -> origin/master remote: Counting objects: 4, done. remote: Compressing objects: 100% (4/4), done. remote: Total 4 (delta 0), reused 0 (delta 0), pack-reused 0 Unpacking objects: 100% (4/4), done. From https://github.com/xu-cheng/homebrew-portable 2bdfb192..afa9fa33 master -> origin/master * [new tag] 20160630 -> 20160630 remote: Counting objects: 15, done. remote: Counting objects: 4, done. remote: Compressing objects: 100% (3/3), done. remote: Total 4 (delta 0), reused 0 (delta 0), pack-reused 0 Unpacking objects: 100% (4/4), done. remote: Compressing objects: 100% (13/13), done. remote: Total 15 (delta 10), reused 7 (delta 2), pack-reused 0 From https://github.com/Homebrew/brew 0926f920..310d7067 master -> origin/master From https://github.com/Homebrew/homebrew-bundle 5db1322c..07ef740d master -> origin/master remote: Counting objects: 9, done. remote: Compressing objects: 100% (8/8), done. remote: Total 9 (delta 4), reused 1 (delta 1), pack-reused 0 Unpacking objects: 100% (9/9), done. From https://github.com/Homebrew/homebrew-binary 36e0486f..d8129452 master -> origin/master Unpacking objects: 100% (15/15), done. From https://github.com/Homebrew/homebrew-command-not-found 8617f442..7f2573d4 master -> origin/master remote: Counting objects: 6, done. remote: Total 6 (delta 4), reused 4 (delta 4), pack-reused 2 Unpacking objects: 100% (6/6), done. From https://github.com/Homebrew/homebrew-python 90621036..ed8d5c5b master -> origin/master remote: Counting objects: 24, done. remote: Compressing objects: 100% (21/21), done. remote: Total 24 (delta 11), reused 3 (delta 3), pack-reused 0 remote: Counting objects: 36, done. remote: Compressing objects: 100% (20/20), done. remote: Total 36 (delta 23), reused 29 (delta 16), pack-reused 0 Unpacking objects: 100% (24/24), done. Unpacking objects: 100% (36/36), done. From https://github.com/Homebrew/homebrew-emacs b7f01ada..2287b7c8 master -> origin/master From https://github.com/Homebrew/homebrew-games e535cffc..fdcbcceb master -> origin/master remote: Counting objects: 22, done. remote: Total 22 (delta 19), reused 19 (delta 19), pack-reused 3 Unpacking objects: 100% (22/22), done. From https://github.com/Homebrew/homebrew-dupes b8bc2dc5..3ee82285 master -> origin/master remote: Counting objects: 53, done. remote: Compressing objects: 100% (19/19), done. remote: Total 53 (delta 26), reused 18 (delta 18), pack-reused 16 Unpacking objects: 100% (53/53), done. From https://github.com/Homebrew/homebrew-versions a8b2acfe..f8c59174 master -> origin/master remote: Counting objects: 60, done. remote: Total 60 (delta 46), reused 46 (delta 46), pack-reused 14 Unpacking objects: 100% (60/60), done. remote: Counting objects: 96, done. remote: Compressing objects: 100% (13/13), done. remote: Total 96 (delta 37), reused 32 (delta 32), pack-reused 51 Unpacking objects: 100% (96/96), done. From https://github.com/Homebrew/homebrew-php 8be4a9e6..d768beb9 master -> origin/master remote: Counting objects: 1231, done. remote: Compressing objects: 100% (34/34), done. From https://github.com/Homebrew/homebrew-science c22e05e9..6ce62c0d master -> origin/master remote: Total 1231 (delta 703), reused 684 (delta 684), pack-reused 513 Receiving objects: 100% (1231/1231), 1.12 MiB | 782.00 KiB/s, done. Resolving deltas: 100% (916/916), completed with 238 local objects. From https://github.com/Homebrew/homebrew-core 14be2111..0a598ccc master -> origin/master remote: Counting objects: 369, done. remote: Compressing objects: 100% (5/5), done. remote: Total 369 (delta 214), reused 214 (delta 214), pack-reused 150 Receiving objects: 100% (369/369), 138.00 KiB | 0 bytes/s, done. Resolving deltas: 100% (246/246), completed with 74 local objects. From https://github.com/Caskroom/homebrew-cask 98f70727..c7fbf76e master -> origin/master

UniqMartin · 2016-07-04T07:54:05Z

Haven't had a chance to test this just yet, but pointed out one major and a few minor issues. Overall, the changes are looking pretty solid and will be a huge step in the right direction. Thanks for transforming our discussion into code!

MikeMcQuaid · 2016-07-06T09:59:50Z

@UniqMartin Let me know if/when you have any further thoughts here or you're 👍

UniqMartin · 2016-07-06T14:52:32Z

Library/Homebrew/cmd/update.sh

@@ -9,6 +9,14 @@
 # shellcheck source=/dev/null
 source "$HOMEBREW_LIBRARY/Homebrew/utils/lock.sh"

+git() {


I think it would be worth mentioning (in a comment) that this replaces the implementation in Library/brew.sh and why it does that.

I'm wondering if this behaviour makes sense in Library/brew.sh too?

It does. It should speed up the git config calls in utils/analytics.sh a bit. There are typically three of them and they happen synchronously during analytics setup, thus slow down every brew run (except for special cases like --prefix). All of this happens before the analytics are sent in a curl process in the background.

UniqMartin · 2016-07-06T14:54:17Z

One minor comment, but otherwise I think this is a fairly straightforward and solid improvement, that speeds up brew update for all users (independently of whether it is used in the auto-update or not.)

This could happen when trying to `brew install git` inside `brew update --preinstall`.

We don’t need to look it up from superenv every time; this is slow.

This aids reading `brew update --verbose --debug` output.

We don’t need to update them as we’re not invoking them.

This is too slow.

MikeMcQuaid · 2016-07-07T09:24:50Z

DAMNIT I squashed again without meaning to 😭.

UniqMartin · 2016-07-07T16:09:26Z

DAMNIT I squashed again without meaning to 😭.

Feature request for GitHub: Warn when you're about to squash several commits from a PR or allow disabling squash merge for PRs that consist of multiple commits (though in the tap repositories that's almost always what we want in case contributors are not comfortable with squashing/rebasing). 😉

MikeMcQuaid · 2016-07-07T16:11:30Z

Perhaps we just disable squash commits in favour of merge commits for this repo?

UniqMartin · 2016-07-07T16:19:08Z

Or perhaps we just stop merging from the web UI and go back to brew pull entirely? (Sadly, it's not possible to completely disable the merge button in the web UI.) The reason is that I'm still very fond of our almost completely linear Git history and we don't merge this many pull requests per day in this repository that switching to the command line for this task is a noticeable burden (in my opinion). A lot more time is devoted to actually developing the features and reviewing them. Just a thought …

ilovezfs · 2016-07-07T16:24:02Z

Yeah not having any merge commits is really nice.

* Don't infinitely recurse `brew update --preinstall`. This could happen when trying to `brew install git` inside `brew update --preinstall`. * update.sh: cache Git PATH. We don’t need to look it up from superenv every time; this is slow. * update.sh: print message before preinstall updates. * update.sh: verbose output fetch directory. This aids reading `brew update --verbose --debug` output. * update.sh: skip taps without formulae on preinstall. We don’t need to update them as we’re not invoking them. * update.sh: don't force update-report on developer preinstall. This is too slow.

BrewTestBot added the in progress Maintainers are working on this label Jul 3, 2016

UniqMartin reviewed Jul 4, 2016
View reviewed changes

ilovezfs mentioned this pull request Jul 6, 2016

brew.sh: enable autoupdate for Homebrew developers #429

Merged

5 tasks

UniqMartin reviewed Jul 6, 2016
View reviewed changes

MikeMcQuaid added 6 commits July 7, 2016 10:11

Don't infinitely recurse brew update --preinstall.

107847b

This could happen when trying to `brew install git` inside `brew update --preinstall`.

update.sh: cache Git PATH.

054c96f

We don’t need to look it up from superenv every time; this is slow.

update.sh: print message before preinstall updates.

c00ccc2

update.sh: verbose output fetch directory.

fc4ba9a

This aids reading `brew update --verbose --debug` output.

update.sh: skip taps without formulae on preinstall.

6cc2dd5

We don’t need to update them as we’re not invoking them.

update.sh: don't force update-report on developer preinstall.

a90951b

This is too slow.

MikeMcQuaid merged commit 86b1df9 into Homebrew:master Jul 7, 2016

MikeMcQuaid deleted the autoupdate-tweaks branch July 7, 2016 09:24

BrewTestBot removed the in progress Maintainers are working on this label Jul 7, 2016

Homebrew locked and limited conversation to collaborators May 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Autoupdate tweaks #442

Autoupdate tweaks #442

MikeMcQuaid commented Jul 3, 2016

UniqMartin Jul 4, 2016

MikeMcQuaid Jul 4, 2016

UniqMartin Jul 4, 2016

UniqMartin commented Jul 4, 2016

MikeMcQuaid commented Jul 6, 2016

UniqMartin Jul 6, 2016

MikeMcQuaid Jul 7, 2016

UniqMartin Jul 7, 2016

UniqMartin commented Jul 6, 2016

MikeMcQuaid commented Jul 7, 2016

UniqMartin commented Jul 7, 2016

MikeMcQuaid commented Jul 7, 2016

UniqMartin commented Jul 7, 2016 •

edited

Loading

ilovezfs commented Jul 7, 2016

Autoupdate tweaks #442

Autoupdate tweaks #442

Conversation

MikeMcQuaid commented Jul 3, 2016

UniqMartin Jul 4, 2016

Choose a reason for hiding this comment

MikeMcQuaid Jul 4, 2016

Choose a reason for hiding this comment

UniqMartin Jul 4, 2016

Choose a reason for hiding this comment

UniqMartin commented Jul 4, 2016

MikeMcQuaid commented Jul 6, 2016

UniqMartin Jul 6, 2016

Choose a reason for hiding this comment

MikeMcQuaid Jul 7, 2016

Choose a reason for hiding this comment

UniqMartin Jul 7, 2016

Choose a reason for hiding this comment

UniqMartin commented Jul 6, 2016

MikeMcQuaid commented Jul 7, 2016

UniqMartin commented Jul 7, 2016

MikeMcQuaid commented Jul 7, 2016

UniqMartin commented Jul 7, 2016 • edited Loading

ilovezfs commented Jul 7, 2016

UniqMartin commented Jul 7, 2016 •

edited

Loading