Skip to content

Commit

Permalink
Various brew update behaviour improvements
Browse files Browse the repository at this point in the history
- Output a message every time auto-update is run rather than a 3 second
  timer. This makes it more obvious that Homebrew isn't just sitting
  doing nothing for 2.9 seconds.
- Output a message when running `brew update` so Homebrew doesn't just
  sit there silently doing nothing.
- Update all taps when `brew update` is run, not just those hosted on
  GitHub. This makes it more obvious that people don't need to explictly
  run `brew update` "just in case".
- As a result of this, remove `brew tap --force-auto-update` as it's no
  longer necessary.
  • Loading branch information
MikeMcQuaid committed Mar 8, 2024
1 parent 815db06 commit 6cafde4
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 88 deletions.
28 changes: 0 additions & 28 deletions Library/Homebrew/brew.sh
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
# 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
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
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
25 changes: 6 additions & 19 deletions Library/Homebrew/tap.rb
Expand Up @@ -345,13 +345,11 @@ def core_cask_tap?
# 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 @@ -367,9 +365,8 @@ def install(quiet: false, clone_target: nil, force_auto_update: nil,

requested_remote = clone_target || default_remote

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?
if installed? && !custom_remote && (clone_target && requested_remote != remote)
raise TapRemoteMismatchError.new(name, @remote, requested_remote)
end

# ensure git is installed
Expand All @@ -380,14 +377,7 @@ def install(quiet: false, clone_target: nil, force_auto_update: nil,
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)

$stderr.ohai "Unshallowing #{name}" if shallow? && !quiet
args = %w[fetch]
Expand Down Expand Up @@ -432,8 +422,6 @@ def install(quiet: false, clone_target: nil, force_auto_update: nil,
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 +1079,7 @@ def remote
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 +1091,7 @@ def install(quiet: false, clone_target: nil, force_auto_update: nil,
$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:)
end

# @private
Expand Down
25 changes: 0 additions & 25 deletions Library/Homebrew/test/tap_spec.rb
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

0 comments on commit 6cafde4

Please sign in to comment.