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.

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

tap: allow to change tap remote with brew tap --custom-remote #12221

Merged
merged 6 commits into from Oct 12, 2021

Conversation

XuehaiPan
Copy link
Contributor

@XuehaiPan XuehaiPan commented Oct 11, 2021

  • 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 style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Add option --force to brew tap for reset the tap remote.

$ brew tap homebrew/command-not-found
==> Tapping homebrew/command-not-found
Cloning into '/home/PanXuehai/Projects/brew/Library/Taps/homebrew/homebrew-command-not-found'...
remote: Enumerating objects: 4580, done.
remote: Counting objects: 100% (833/833), done.
remote: Compressing objects: 100% (372/372), done.
remote: Total 4580 (delta 518), reused 731 (delta 436), pack-reused 3747
Receiving objects: 100% (4580/4580), 5.24 MiB | 1.29 MiB/s, done.
Resolving deltas: 100% (2988/2988), done.
Tapped 3 commands (28 files, 5.8MB).

$ git -C "$(brew --repo homebrew/command-not-found)" remote get-url origin
https://github.com/Homebrew/homebrew-command-not-found

$ brew tap homebrew/command-not-found https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-command-not-found.git        
Error: Tap homebrew/command-not-found remote mismatch.
https://github.com/Homebrew/homebrew-command-not-found != https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-command-not-found.git

$ brew tap --force homebrew/command-not-found https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-command-not-found.git
==> homebrew/command-not-found: changed remote from https://github.com/Homebrew/homebrew-command-not-found to https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-command-not-found.git
From https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-command-not-found
 + 97fc8b5...bb517f7 master     -> origin/master  (forced update)

$ git -C "$(brew --repo homebrew/command-not-found)" remote get-url origin
https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-command-not-found.git

New tests have not been added yet, since we do not test fix_remote_configuration.

@XuehaiPan XuehaiPan changed the title tap: allow change tap remote with brew tap --force tap: allow to change tap remote with brew tap --force Oct 11, 2021
@MikeMcQuaid
Copy link
Member

@XuehaiPan I like the idea of the functionality but I'm not sure about:

  • the name, maybe --fix-remote or something would be more appropriate?
  • how widely used this would be given that brew update does (or at least should) already do this for Homebrew/brew and Homebrew/homebrew-core

@XuehaiPan
Copy link
Contributor Author

XuehaiPan commented Oct 11, 2021

the name, maybe --fix-remote or something would be more appropriate?

The --force option will take no effect when the tap hasn't been installed yet. Name as --fix-remote means we should install the tap first and then change the remote. If we are sure that the tap has already been installed, we can use the follows rather than brew tap --fix-remote:

git -C "$(brew --repo homebrew/repo)" remote set-url origin <REMOTE-URL>

how widely used this would be given that brew update does (or at least should) already do this for Homebrew/brew and Homebrew/homebrew-core

brew update will change the remote of Homebrew/brew and/or Homebrew/homebrew-core if HOMEBREW_{BREW,CORE}_GIT_REMOTE set. However, there are still many other taps do not have the corresponding variables for the Git remotes (e.g. HOMEBREW_CASK_GIT_REMOTE for Homebrew/homebrew-cask). This brew tap --force option will allow user to use brew command rather than an explicit git -C "$(brew --repo ...)" remote set-url ... command to achieve this.

In the docs for Homebrew Git Mirroring at TUNA:

# Set manually
git -C "$(brew --repo homebrew/core)" remote set-url origin https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-core.git
git -C "$(brew --repo homebrew/cask)" remote set-url origin https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-cask.git
git -C "$(brew --repo homebrew/cask-fonts)" remote set-url origin https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-cask-fonts.git
git -C "$(brew --repo homebrew/cask-drivers)" remote set-url origin https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-cask-drivers.git
git -C "$(brew --repo homebrew/cask-versions)" remote set-url origin https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-cask-versions.git
git -C "$(brew --repo homebrew/command-not-found)" remote set-url origin https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-command-not-found.git

# Or use the following scripts
BREW_TAPS="$(BREW_TAPS="$(brew tap 2>/dev/null)"; echo -n "${BREW_TAPS//$'\n'/:}")"
for tap in core cask{,-fonts,-drivers,-versions} command-not-found; do
    if [[ ":${BREW_TAPS}:" == *":homebrew/${tap}:"* ]]; then  # if installed
        # Set the remote of tap to TUNA and mark as auto update
        git -C "$(brew --repo homebrew/${tap})" remote set-url origin "https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-${tap}.git"
        git -C "$(brew --repo homebrew/${tap})" config homebrew.forceautoupdate true
    else  # install when not installed
        brew tap --force-auto-update "homebrew/${tap}" "https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-${tap}.git"
    fi
done

With the --force option, we can use brew's built-in command to change the remote mirror:

for tap in core cask{,-fonts,-drivers,-versions} command-not-found; do
    brew tap --force --force-auto-update "homebrew/${tap}" "https://mirrors.tuna.tsinghua.edu.cn/git/homebrew/homebrew-${tap}.git"
done

No error will raise with --force option if the tap has already been installed (no if ... then ... else condition anymore).

@MikeMcQuaid
Copy link
Member

Name as --fix-remote means we should install the tap first and then change the remote.

Not sure I agree, it could just error out.

The --force option will take no effect when the tap hasn't been installed yet.

This doesn't make sense to me, sounds like it should always e.g. retap even if already tapped.

However, there are still many other taps do not have the corresponding variables for the Git remotes (e.g. HOMEBREW_CASK_GIT_REMOTE for Homebrew/homebrew-cask).

I think we should consider adding this variable 👍🏻 Not sure about other taps because they aren't as slow.

With the --force option, we can use brew's built-in command to change the remote mirror:

I see. I think it's probably worth using a different name or adding more variables for these taps (or a generic way that doesn't require a separate variable for each tap).

@XuehaiPan
Copy link
Contributor Author

XuehaiPan commented Oct 11, 2021

The --force option will take no effect when the tap hasn't been installed yet.

This doesn't make sense to me, sounds like it should always e.g. retap even if already tapped.

I mean brew tap --force ... is equivalent to brew tap ... when the tap is not installed. Otherwise, we will change the remote if it is already installed.

With the --force option, we can use brew's built-in command to change the remote mirror:

I see. I think it's probably worth using a different name or adding more variables for these taps (or a generic way that doesn't require a separate variable for each tap).

There are many official taps in Homebrew (cask, cask-fonts, command-not-found, bundle, etc.). If we add a variable for each tap repo, maybe it will make brew update slow? As for the name, what if we add a new command as brew retap? For me, I prefer to reuse the existing command brew tap as the new option --force does not break the old behaviors.

@MikeMcQuaid
Copy link
Member

I mean brew tap --force ... is equivalent to brew tap ... when the tap is not installed. Otherwise, we will change the remote if it is already installed.

Yes, it sounds like it should change the remote when already installed. Maybe something like --custom-remote?

There are many official taps in Homebrew (cask, cask-fonts, command-not-found, bundle, etc.). If we add a variable for each tap repo, maybe it will make brew update slow?

Only homebrew-core and homebrew-cask are really slow from China though, right? I don't think they all need mirrored. It won't make brew update slow but it will bloat the file so I agree not adding them for every tap.

As for the name, what if we add a new command as brew retap? For me, I prefer to reuse the existing command brew tap as the new option --force does not break the old behaviors.

I think keeping it on brew tap makes sense, too.

@XuehaiPan
Copy link
Contributor Author

Maybe something like --custom-remote?

I'm fine with the name. I can change the name of the new option in this PR if requested. I choose --force for "never prompt errors".

@MikeMcQuaid
Copy link
Member

I'm fine with the name. I can change the name of the new option in this PR if requested. I choose --force for "never prompt errors".

Yeh, a new name would be good.

@XuehaiPan XuehaiPan force-pushed the brew-retap branch 2 times, most recently from ec69bab to b25d846 Compare October 11, 2021 14:19
@XuehaiPan XuehaiPan changed the title tap: allow to change tap remote with brew tap --force tap: allow to change tap remote with brew tap --custom-remote Oct 11, 2021
@XuehaiPan
Copy link
Contributor Author

Yeh, a new name would be good.

I changed the name to --custom-remote.

Library/Homebrew/cmd/tap.rb Outdated Show resolved Hide resolved
Library/Homebrew/tap.rb Outdated Show resolved Hide resolved
@XuehaiPan XuehaiPan force-pushed the brew-retap branch 4 times, most recently from 266e1e2 to 7982686 Compare October 11, 2021 16:46
@XuehaiPan

This comment has been minimized.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Looking good! A few more tweaks.

Library/Homebrew/tap.rb Outdated Show resolved Hide resolved
Library/Homebrew/tap.rb Outdated Show resolved Hide resolved
Library/Homebrew/tap.rb Outdated Show resolved Hide resolved
Library/Homebrew/tap.rb Outdated Show resolved Hide resolved
Library/Homebrew/tap.rb Outdated Show resolved Hide resolved
@MikeMcQuaid
Copy link
Member

Thanks again @XuehaiPan!

auto-merge was automatically disabled October 12, 2021 12:31

Head branch was pushed to by a user without write access

@MikeMcQuaid MikeMcQuaid merged commit cc2c19b into Homebrew:master Oct 12, 2021
@XuehaiPan XuehaiPan deleted the brew-retap branch October 12, 2021 13:52
@github-actions github-actions bot added the outdated PR was locked due to age label Nov 12, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants