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.

Sign up for GitHub

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

Jump to bottom

update.sh: refuse to update shallow homebrew-core/cask clones. #9383

Merged
merged 1 commit into from
Dec 3, 2020
Merged

update.sh: refuse to update shallow homebrew-core/cask clones. #9383

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions Library/Homebrew/cmd/update.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,26 @@ EOS
fi
fi

# Homebrew/homebrew-core is extremely expensive to perform shallow clones on
# so, on GitHub's request, don't allow it.
if [[ -f "$HOMEBREW_LIBRARY/Taps/homebrew/homebrew-core/.git/shallow" ]]
then
odie <<EOS
homebrew-core is a shallow clone. To \`brew update\` first run:
git -C "$HOMEBREW_LIBRARY/Taps/homebrew/homebrew-core" fetch --unshallow
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just run this command instead of failing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxim-belkin Sorry, missed this before merging.

Can we add an extra comment about brew update potentially 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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add an extra comment about brew update potentially 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 update this 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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

I think this being documented in the PR is fine for now.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened #9447 to clarify the reasoning in the output message.

EOS
fi

# Homebrew/homebrew-cask is also extremely expensive to perform shallow clones
# on so, on GitHub's request, don't allow it.
if [[ -f "$HOMEBREW_LIBRARY/Taps/homebrew/homebrew-cask/.git/shallow" ]]
then
odie <<EOS
homebrew-cask is a shallow clone. To \`brew update\` first run:
git -C "$HOMEBREW_LIBRARY/Taps/homebrew/homebrew-cask" fetch --unshallow
EOS
fi

MikeMcQuaid marked this conversation as resolved.
Show resolved Hide resolved
export GIT_TERMINAL_PROMPT="0"
export GIT_SSH_COMMAND="ssh -oBatchMode=yes"

Expand Down