update.sh: refuse to update shallow homebrew-core/cask clones. #9383
Conversation
|
Review period will end on 2020-12-03 at 13:23:00 UTC. |
BrewTestBot
added
the
waiting for feedback
| then | ||
| odie <<EOS | ||
| homebrew-core is a shallow clone. To \`brew update\` first run: | ||
| git -C "$HOMEBREW_LIBRARY/Taps/homebrew/homebrew-core" fetch --unshallow |
There was a problem hiding this comment.
Why not just run this command instead of failing?
There was a problem hiding this comment.
If you have e.g. a CI configuration that's manually installing Homebrew/homebrew-core with a shallow clone and then deliberately or unintentionally running brew update this will add even more load onto GitHub than the version before this pull request. Running this command an expensive, one-time operation but if it's run once, manually by humans spread out over a time period it will be a small increase in load which will then drop. If those CI configurations all run this e.g. as soon as this gets onto master or a stable tag: there will be a massive spike in a short time window.
Similarly, this motivates configurations that repeatedly setup a shallow clone of homebrew-core to be fixed rather than us automatically "fix" them in a way that's even more expensive.
There was a problem hiding this comment.
Can we add an extra comment about brew update potentially running in someone else's CI? I think the comment for the cask block could be shortened to something along the lines of "Same applies to Homebrew/homebrew-cask".
There was a problem hiding this comment.
@maxim-belkin Sorry, missed this before merging.
Can we add an extra comment about
brew updatepotentially running in someone else's CI?
I don't understand this, sorry?
I think the comment for the cask block could be shortened to something along the lines of "Same applies to Homebrew/homebrew-cask".
I'd rather the more explicit comment for when partial diffs are shown.
There was a problem hiding this comment.
Can we add an extra comment about
brew updatepotentially running in someone else's CI?I don't understand this, sorry?
I was referring to the first sentence in your response:
If you have e.g. a CI configuration that's manually installing Homebrew/homebrew-core with a shallow clone and then deliberately or unintentionally running
brew updatethis will add even more load onto GitHub than the version before this pull request.
This part clarifies that if we run git fetch --unshallow automatically and someone sets up a shallow clone in their CI, it would lead to a greatly increased load on GitHub's servers, which we don't want.
There was a problem hiding this comment.
This part clarifies that if we run
git fetch --unshallowautomatically and someone sets up a shallow clone in their CI, it would lead to a greatly increased load on GitHub's servers, which we don't want.
I think this being documented in the PR is fine for now.
There was a problem hiding this comment.
Why not just run this command instead of failing?
If that can't be done, why not split the difference by wrapping the git ... in a flag and saying:
To continue using `brew update` first run: `brew update --unshallow` once.
As it stands the handling of this is bad on two counts: it makes it sound like the user has done something wring ("you made a nasty shallow clone", whatever that is); and it asks them to drop unto the implementation of Homebrew to fix it.
There was a problem hiding this comment.
I've opened #9447 to clarify the reasoning in the output message.
GitHub has requested this as these are extremely expensive operations.
|
Review period ended. |
BrewTestBot
removed
the
waiting for feedback
This comment has been minimized.
This comment has been minimized.
GitHub has requested this as these are extremely expensive operations.