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

Various brew update behaviour improvements #16855

Merged
merged 1 commit into from Mar 11, 2024
Merged

Various brew update behaviour improvements #16855

Show file tree
Hide file tree
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 0 additions & 28 deletions Library/Homebrew/brew.sh
View file
Open in desktop
Expand Up @@ -262,22 +262,6 @@ check-array-membership() {
fi
}

# Let user know we're still updating Homebrew if brew update --auto-update
# exceeds 3 seconds.
auto-update-timer() {
sleep 3
Comment on lines -266 to -268
Copy link
Member

Choose a reason for hiding this comment

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

Not a strong opinion, but feels nicer to try tweaking the 3 second timer before jumping to showing it all the time.

Copy link
Member Author

Choose a reason for hiding this comment

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

@carlocab I think having it (almost) immediately print output makes the output more responsive (and, in turn, Homebrew "feel" faster).

Copy link
Contributor

Choose a reason for hiding this comment

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

Trying this out I agree that having the immediate feedback feels nicer. Otherwise I wonder whether brew has started updating, or whether it's just blocked on loading files or something.

# Outputting a command but don't want to run it, hence single quotes.
# shellcheck disable=SC2016
echo 'Running `brew update --auto-update`...' >&2
if [[ -z "${HOMEBREW_NO_ENV_HINTS}" && -z "${HOMEBREW_AUTO_UPDATE_SECS}" ]]
then
# shellcheck disable=SC2016
echo 'Adjust how often this is run with HOMEBREW_AUTO_UPDATE_SECS or disable with' >&2
# shellcheck disable=SC2016
echo 'HOMEBREW_NO_AUTO_UPDATE. Hide these hints with HOMEBREW_NO_ENV_HINTS (see `man brew`).' >&2
fi
}

# These variables are set from various Homebrew scripts.
# shellcheck disable=SC2154
auto-update() {
Expand Down Expand Up @@ -339,20 +323,8 @@ auto-update() {
return
fi

if [[ -z "${HOMEBREW_VERBOSE}" ]]
then
auto-update-timer &
timer_pid=$!
fi

brew update --auto-update

if [[ -n "${timer_pid}" ]]
then
kill "${timer_pid}" 2>/dev/null
wait "${timer_pid}" 2>/dev/null
fi

unset HOMEBREW_AUTO_UPDATING

# Restore user path as it'll be refiltered by HOMEBREW_BREW_FILE (bin/brew)
Expand Down
14 changes: 6 additions & 8 deletions Library/Homebrew/cmd/tap.rb
View file
Open in desktop
Expand Up @@ -34,8 +34,7 @@ def tap_args
replacement: false,
disable: true
switch "--[no-]force-auto-update",
description: "Auto-update tap even if it is not hosted on GitHub. By default, only taps " \
"hosted on GitHub are auto-updated (for performance reasons)."
hidden: true
switch "--custom-remote",
description: "Install or change a tap with a custom remote. Useful for mirrors."
switch "--repair",
Expand Down Expand Up @@ -64,12 +63,11 @@ def tap
else
tap = Tap.fetch(args.named.first)
begin
tap.install clone_target: args.named.second,
force_auto_update: args.force_auto_update?,
custom_remote: args.custom_remote?,
quiet: args.quiet?,
verify: args.eval_all? || Homebrew::EnvConfig.eval_all?,
force: args.force?
tap.install clone_target: args.named.second,
custom_remote: args.custom_remote?,
quiet: args.quiet?,
verify: args.eval_all? || Homebrew::EnvConfig.eval_all?,
force: args.force?
rescue TapRemoteMismatchError, TapNoCustomRemoteError => e
odie e
rescue TapAlreadyTappedError
Expand Down
28 changes: 20 additions & 8 deletions Library/Homebrew/cmd/update.sh
View file
Open in desktop
Expand Up @@ -622,6 +622,26 @@ EOS
[[ -n "${SKIP_FETCH_CORE_REPOSITORY}" && "${DIR}" == "${HOMEBREW_CORE_REPOSITORY}" ]] && continue
fi

if [[ -z "${UPDATING_MESSAGE_SHOWN}" ]]
then
if [[ -n "${HOMEBREW_UPDATE_AUTO}" ]]
then
# Outputting a command but don't want to run it, hence single quotes.
# shellcheck disable=SC2016
ohai 'Auto-updating Homebrew...' >&2
if [[ -z "${HOMEBREW_NO_ENV_HINTS}" && -z "${HOMEBREW_AUTO_UPDATE_SECS}" ]]
then
# shellcheck disable=SC2016
echo 'Adjust how often this is run with HOMEBREW_AUTO_UPDATE_SECS or disable with' >&2
# shellcheck disable=SC2016
echo 'HOMEBREW_NO_AUTO_UPDATE. Hide these hints with HOMEBREW_NO_ENV_HINTS (see `man brew`).' >&2
fi
else
ohai 'Updating Homebrew...' >&2
fi
UPDATING_MESSAGE_SHOWN=1
fi

# The upstream repository's default branch may not be master;
# check refs/remotes/origin/HEAD to see what the default
# origin branch name is, and use that. If not set, fall back to "master".
Expand Down Expand Up @@ -693,14 +713,6 @@ EOS
# Touch FETCH_HEAD to confirm we've checked for an update.
[[ -f "${DIR}/.git/FETCH_HEAD" ]] && touch "${DIR}/.git/FETCH_HEAD"
[[ -z "${HOMEBREW_UPDATE_FORCE}" ]] && [[ "${UPSTREAM_SHA_HTTP_CODE}" == "304" ]] && exit
elif [[ -n "${HOMEBREW_UPDATE_AUTO}" ]]
then
FORCE_AUTO_UPDATE="$(git config homebrew.forceautoupdate 2>/dev/null || echo "false")"
if [[ "${FORCE_AUTO_UPDATE}" != "true" ]]
then
# Don't try to do a `git fetch` that may take longer than expected.
exit
fi
fi

# HOMEBREW_VERBOSE isn't modified here so ignore subshell warning.
Expand Down
22 changes: 5 additions & 17 deletions Library/Homebrew/tap.rb
View file
Open in desktop
Expand Up @@ -345,13 +345,11 @@
# Install this {Tap}.
#
# @param clone_target [String] If passed, it will be used as the clone remote.
# @param force_auto_update [Boolean, nil] If present, whether to override the
# logic that skips non-GitHub repositories during auto-updates.
# @param quiet [Boolean] If set, suppress all output.
# @param custom_remote [Boolean] If set, change the tap's remote if already installed.
# @param verify [Boolean] If set, verify all the formula, casks and aliases in the tap are valid.
# @param force [Boolean] If set, force core and cask taps to install even under API mode.
def install(quiet: false, clone_target: nil, force_auto_update: nil,
def install(quiet: false, clone_target: nil,
custom_remote: false, verify: false, force: false)
require "descriptions"
require "readall"
Expand All @@ -369,7 +367,7 @@

if installed? && !custom_remote
raise TapRemoteMismatchError.new(name, @remote, requested_remote) if clone_target && requested_remote != remote
raise TapAlreadyTappedError, name if force_auto_update.nil? && !shallow?
raise TapAlreadyTappedError, name unless shallow?
end

# ensure git is installed
Expand All @@ -380,14 +378,7 @@
fix_remote_configuration(requested_remote:, quiet:)
end

case force_auto_update
when true
config[:forceautoupdate] = true
return
when false
config.delete(:forceautoupdate)
return
end
config.delete(:forceautoupdate)

Check warning on line 381 in Library/Homebrew/tap.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/tap.rb#L381 

Added line #L381 was not covered by tests

$stderr.ohai "Unshallowing #{name}" if shallow? && !quiet
args = %w[fetch]
Expand Down Expand Up @@ -432,8 +423,6 @@
raise
end

config[:forceautoupdate] = force_auto_update unless force_auto_update.nil?

Commands.rebuild_commands_completion_list
link_completions_and_manpages

Expand Down Expand Up @@ -1091,7 +1080,7 @@
end

# CoreTap never allows shallow clones (on request from GitHub).
def install(quiet: false, clone_target: nil, force_auto_update: nil,
def install(quiet: false, clone_target: nil,
custom_remote: false, verify: false, force: false)
remote = Homebrew::EnvConfig.core_git_remote # set by HOMEBREW_CORE_GIT_REMOTE
requested_remote = clone_target || remote
Expand All @@ -1103,8 +1092,7 @@
$stderr.puts "HOMEBREW_CORE_GIT_REMOTE set: using #{remote} as the Homebrew/homebrew-core Git remote."
end

super(quiet:, clone_target: remote, force_auto_update:,
custom_remote:, force:)
super(quiet:, clone_target: remote, custom_remote:, force:)

Check warning on line 1095 in Library/Homebrew/tap.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/tap.rb#L1095 

Added line #L1095 was not covered by tests
end

# @private
Expand Down
25 changes: 0 additions & 25 deletions Library/Homebrew/test/tap_spec.rb
View file
Open in desktop
Expand Up @@ -354,31 +354,6 @@ def setup_completion(link:)
end.to raise_error(TapNoCustomRemoteError)
end

describe "force_auto_update" do
before do
setup_git_repo
end

let(:already_tapped_tap) { described_class.fetch("Homebrew", "foo") }

it "defaults to nil" do
expect(already_tapped_tap).to be_installed
expect(already_tapped_tap.config[:forceautoupdate]).to be_nil
end

it "enables forced auto-updates when true" do
expect(already_tapped_tap).to be_installed
already_tapped_tap.install force_auto_update: true
expect(already_tapped_tap.config[:forceautoupdate]).to be true
end

it "disables forced auto-updates when false" do
expect(already_tapped_tap).to be_installed
already_tapped_tap.install force_auto_update: false
expect(already_tapped_tap.config[:forceautoupdate]).to be_nil
end
end

specify "Git error" do
tap = described_class.fetch("user", "repo")

Expand Down