Fix check_coretap_git_origin #5607

sjackman · 2019-01-25T06:55:49Z

check_coretap_git_origin was not working as intended.
Permit Linuxbrew/homebrew-core as a valid origin.
Factor out check_coretap_git_branch.

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 tests with your changes locally?

sjackman · 2019-01-25T06:57:58Z

Library/Homebrew/diagnostic.rb


        if origin.nil?
          <<~EOS
            Missing #{CoreTap.instance} git origin remote.

            Without a correctly configured origin, Homebrew won't update
            properly. You can solve this by adding the Homebrew remote:
-              git -C "#{coretap_path}" remote add origin #{Formatter.url("https://github.com/Homebrew/homebrew-core.git")}
+              git -C "#{coretap_path}" remote add origin #{Formatter.url(default_remote)}
          EOS


This string went nowhere.

sjackman · 2019-01-25T06:58:06Z

Library/Homebrew/diagnostic.rb

@@ -595,12 +596,17 @@ def check_coretap_git_origin

            Unless you have compelling reasons, consider setting the
            origin remote to point at the main repository by running:
-              git -C "#{coretap_path}" remote set-url origin #{Formatter.url("https://github.com/Homebrew/homebrew-core.git")}
+              git -C "#{coretap_path}" remote set-url origin #{Formatter.url(default_remote)}
          EOS


This string went nowhere.

MikeMcQuaid

One change request then good to go 👍

MikeMcQuaid · 2019-01-25T15:16:49Z

Library/Homebrew/diagnostic.rb

          EOS
-        elsif origin !~ %r{Homebrew/homebrew-core(\.git|/)?$}
+        elsif origin !~ %r{#{URI(default_remote).path}(\.git|/)?$}


Use CoreTap.instance.full_name instead.

On Linux, CoreTap.instance.full_name returns Homebrew/homebrew-core rather than the desired Linuxbrew/homebrew-core.

I'd like to use…

elsif origin != default_remote

It's stricter in three ways:

Doesn't allow trailing .git

Doesn't allow trailing /

Doesn't allow ssh

I'm personally okay with that. Homogeneity is good.

Now that I think about it, it's simply a bug that CoreTap.instance.full_name returns Homebrew/homebrew-core on Linux. I'll fix it.

Now that I think about it, it's simply a bug that CoreTap.instance.full_name returns Homebrew/homebrew-core on Linux. I'll fix it.

👍

MikeMcQuaid · 2019-01-25T15:17:34Z

Library/Homebrew/diagnostic.rb

        return if ENV["CI"]

+        coretap_path = CoreTap.instance.path
+        return if !Utils.git_available? || !(coretap_path/".git").exist?
+
        branch = coretap_path.git_branch
        return if branch.nil? || branch =~ /master/



Want to update the string two lines below, too?

You mean change Homebrew/homebrew-core is not on the master branch? Yep, can do.

@sjackman 👍

sjackman · 2019-01-25T19:51:01Z

Weird CI error. I'm looking into it.
https://dev.azure.com/Homebrew/Homebrew/_build/results?buildId=3209&view=logs&jobId=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&taskId=8ef82b3b-1feb-5bbd-06f6-b1f7b5467f03&lineStart=42&lineEnd=52

sjackman · 2019-01-25T19:56:14Z

I'm not able to reproduce this error locally.

2019-01-25T18:41:19.7027843Z ‌Warning:‌ Suspicious homebrew/core git origin remote found.‌
2019-01-25T18:41:19.7028040Z 
2019-01-25T18:41:19.7028435Z With a non-standard origin, Homebrew won't pull updates from
2019-01-25T18:41:19.7028709Z the main repository. The current git origin is:
2019-01-25T18:41:19.7029140Z   https://github.com/Homebrew/homebrew-core
2019-01-25T18:41:19.7029321Z 
2019-01-25T18:41:19.7029497Z Unless you have compelling reasons, consider setting the
2019-01-25T18:41:19.7029723Z origin remote to point at the main repository by running:
2019-01-25T18:41:19.7030639Z   git -C "/home/linuxbrew/.linuxbrew/Library/Taps/homebrew/homebrew-core" remote set-url origin ‌https://github.com/Homebrew/homebrew-core‌
2019-01-25T18:41:19.7947556Z ==> ‌FAILED‌

The curent origin and suggested default_remote appear identical.

@MikeMcQuaid Any idea why the regex is not matching?

        elsif origin !~ %r{#{CoreTap.instance.full_name}(\.git|/)?$}

sjackman · 2019-01-25T19:58:33Z

CI passed on macOS, and failed on Linux. HOMEBREW_FORCE_HOMEBREW_ON_LINUX=1 must be set because it's using and suggesting Homebrew/homebrew-core, though I don't see in azure-pipelines.yml where that's set. Perhaps it's set in brew test-bot.

MikeMcQuaid · 2019-01-25T20:05:40Z

@sjackman Only for uploading bottles. It's doing brew readall on Linuxbrew.

MikeMcQuaid · 2019-01-25T20:07:05Z

With a non-standard origin, Homebrew won't pull updates from the main repository. The current git origin is: https://github.com/Homebrew/homebrew-core

This failure is correct; that origin is wrong on Linux.

MikeMcQuaid · 2019-01-25T20:07:37Z

Library/Homebrew/diagnostic.rb

          EOS
-        elsif origin !~ %r{Homebrew/homebrew-core(\.git|/)?$}
+        elsif origin !~ %r{#{CoreTap.instance.full_name}(\.git|/)?$}
          return if ENV["CI"] && origin.include?("Homebrew/homebrew-test-bot")


This can be removed now by the way.

MikeMcQuaid · 2019-01-25T20:08:03Z

Library/Homebrew/extend/os/linux/tap.rb

+  # @private
+  def initialize
+    super "Homebrew", "core"
+    @full_name = "Linuxbrew/homebrew-core" unless ENV["HOMEBREW_FORCE_HOMEBREW_ON_LINUX"]


I'm not sure this is properly overriding e.g.the path etc.

The aim here is to override only full_name and default_remote on Linux. The other variables should be identical on Linux as on macOS. In particular, CoreTap.instance.path is $HOMEBREW_PREFIX/Homebrew/Library/Taps/homebrew/homebrew-core.

$ brew irb <<<'CoreTap.instance.path' … <Pathname:/home/linuxbrew/.linuxbrew/Homebrew/Library/Taps/homebrew/homebrew-core>

check_coretap_git_origin was not working as intended. Permit Linuxbrew/homebrew-core as a valid origin. Factor out check_coretap_git_branch.

sjackman · 2019-01-25T22:04:17Z

I found the problem. The generic CoreTap.default_remote was still defined, and shouldn't be.

sjackman · 2019-01-25T23:53:22Z

💚 Ready for re-review.

MikeMcQuaid · 2019-01-26T08:38:08Z

Thanks @sjackman!

sjackman · 2019-01-28T07:09:13Z

Thanks for merging, Mike!

sjackman requested review from reitermarkus and MikeMcQuaid January 25, 2019 06:56

sjackman self-assigned this Jan 25, 2019

sjackman added the bug Reproducible Homebrew/brew bug label Jan 25, 2019

sjackman commented Jan 25, 2019

View reviewed changes

MikeMcQuaid requested changes Jan 25, 2019

View reviewed changes

MikeMcQuaid reviewed Jan 25, 2019

View reviewed changes

sjackman force-pushed the check_coretap_git_origin branch from 3d3d0dc to d523ea6 Compare January 25, 2019 18:39

MikeMcQuaid reviewed Jan 25, 2019

View reviewed changes

Fix check_coretap_git_origin

d13a287

check_coretap_git_origin was not working as intended. Permit Linuxbrew/homebrew-core as a valid origin. Factor out check_coretap_git_branch.

sjackman force-pushed the check_coretap_git_origin branch from d523ea6 to d13a287 Compare January 25, 2019 22:01

MikeMcQuaid approved these changes Jan 26, 2019

View reviewed changes

MikeMcQuaid merged commit 7eb7ecf into Homebrew:master Jan 26, 2019

sjackman deleted the check_coretap_git_origin branch January 28, 2019 07:09

lock bot added the outdated PR was locked due to age label Feb 27, 2019

lock bot locked as resolved and limited conversation to collaborators Feb 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix check_coretap_git_origin #5607

Fix check_coretap_git_origin #5607

sjackman commented Jan 25, 2019

sjackman Jan 25, 2019

sjackman Jan 25, 2019

MikeMcQuaid left a comment

MikeMcQuaid Jan 25, 2019

sjackman Jan 25, 2019

sjackman Jan 25, 2019

sjackman Jan 25, 2019

MikeMcQuaid Jan 25, 2019

MikeMcQuaid Jan 25, 2019

sjackman Jan 25, 2019

MikeMcQuaid Jan 25, 2019

sjackman commented Jan 25, 2019

sjackman commented Jan 25, 2019

sjackman commented Jan 25, 2019

MikeMcQuaid commented Jan 25, 2019

MikeMcQuaid commented Jan 25, 2019

MikeMcQuaid Jan 25, 2019

MikeMcQuaid Jan 25, 2019

sjackman Jan 25, 2019

sjackman commented Jan 25, 2019

sjackman commented Jan 25, 2019

MikeMcQuaid commented Jan 26, 2019

sjackman commented Jan 28, 2019

Fix check_coretap_git_origin #5607

Fix check_coretap_git_origin #5607

Conversation

sjackman commented Jan 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjackman commented Jan 25, 2019

sjackman commented Jan 25, 2019

sjackman commented Jan 25, 2019

MikeMcQuaid commented Jan 25, 2019

MikeMcQuaid commented Jan 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjackman commented Jan 25, 2019

sjackman commented Jan 25, 2019

MikeMcQuaid commented Jan 26, 2019

sjackman commented Jan 28, 2019