Skip to content

Conversation

@Bo98
Copy link
Member

@Bo98 Bo98 commented Oct 3, 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?

Currently when curl is used in formulae, the value set in HOMEBREW_CURL is completely discarded. That means brewed curl is currently never used in formula builds/tests where it may be necessary (HOMEBREW_FORCE_BREWED_CURL), and may be used when not wanted (e.g. HOMEBREW_CURL_PATH).

This changes curl to use the same shim strategy as git does.

This fixes test failures on Linux involving curl, as seen in https://github.com/Homebrew/homebrew-core/pull/86291/checks?check_run_id=3773393332 when paired with Homebrew/homebrew-test-bot#671, unblocking a step towards fixing curl on older macOS.

@Bo98 Bo98 added the critical Critical change which should be shipped as soon as possible. label Oct 3, 2021
@Bo98 Bo98 requested a review from carlocab October 3, 2021 20:55
@BrewTestBot
Copy link
Member

Review period skipped due to critical label.

@Bo98
Copy link
Member Author

Bo98 commented Oct 3, 2021

@carlocab This should also cover Homebrew/homebrew-core#86257 (comment).

@Bo98
Copy link
Member Author

Bo98 commented Oct 4, 2021

Thanks both. I'd normally give longer review time for this, but it is absence of it is making CI fail just now for the nghttp2 PR - which is a part of whole fixing older macOS versions stuff.

I'll merge this now but will respond to feedback while the nghttp2 PR is running.

@Bo98 Bo98 merged commit 6c397a5 into Homebrew:master Oct 4, 2021
@Bo98 Bo98 deleted the curl-shim branch October 4, 2021 02:04
Copy link
Member

@Rylan12 Rylan12 left a comment

Choose a reason for hiding this comment

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

I know it's already merged but ✅ from me as well

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.

Makes sense to me!

@thewilsonator
Copy link

I think this PR is the cause of some very strange issues we are experiencing in CI:
https://cirrus-ci.com/task/5219352065605632?logs=install_prerequisites#L95

in particular recent runs give

$ brew update-reset
==> Fetching /usr/local/Homebrew...
From https://github.com/Homebrew/brew
   OLD-SHA.. 28e4c9c  master     -> origin/master
 * [new tag]             2.7.2      -> 2.7.2
 * ... [lots off tags]...
 
==> Resetting /usr/local/Homebrew...
Switched to and reset branch 'master'
Branch 'master' set up to track remote branch 'master' from 'origin'.
Your branch is up to date with 'origin/master'.

## N.B. git HEAD _is_ 28e4c9c here

==> Fetching /usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask...
/usr/local/Homebrew/Library/Homebrew/brew.sh: line 102: /usr/local/Homebrew/Library/Homebrew/shims/scm/git: No such file or directory
/usr/local/Homebrew/Library/Homebrew/brew.sh: line 102: /usr/local/Homebrew/Library/Homebrew/shims/scm/git: No such file or directory
==> Resetting /usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask...
/usr/local/Homebrew/Library/Homebrew/brew.sh: line 102: /usr/local/Homebrew/Library/Homebrew/shims/scm/git: No such file or directory
/usr/local/Homebrew/Library/Homebrew/brew.sh: line 102: /usr/local/Homebrew/Library/Homebrew/shims/scm/git: No such file or directory

## same output for Taps "homebrew-cask-versions", "homebrew-core" and  "robotsandpencils/homebrew-made"

$ brew install gnupg
Error: gnupg: Calling `sha256 "digest" => :tag` in a bottle block is disabled! Use `brew style --fix` on the formula to update the style or use `sha256 tag: "digest"` instead.
Please report this issue to the homebrew/core tap (not Homebrew/brew or Homebrew/core), or even better, submit a PR to fix it:
  /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/gnupg.rb:14

I can confirm line 102 of /usr/local/Homebrew/Library/Homebrew/brew.sh is a comment as it is in 28e4c9c and the git() function's path is Homebrew/shims/shared/git not Homebrew/shims/scm/git, I have no idea were the latter is coming from.

Any ideas?

@carlocab
Copy link
Member

carlocab commented Oct 5, 2021

You have an old version of the gnupg formula getting picked up somewhere for some reason. Doesn’t seem related to this PR.

@thewilsonator
Copy link

It seems to me like the Fetching/Resetting of homebrew-core in brew update-reset is failing to execute properly due to missing git-shim. Our pipelines started failing yesterday, which is about when this PR was pulled.

@thewilsonator
Copy link

...and it seems to have been fixed by running brew update-reset twice?!??!??!! Oh well, this can serve as a note to any other unfortunate people who run into this.

@Bo98
Copy link
Member Author

Bo98 commented Oct 5, 2021

That seems like a bug with update-reset specifically where it doesn't reload itself after updating brew. I'm working on a fix.

@thewilsonator
Copy link

Thanks! If you could ping me on that PR when you open it that would be great.

@carlocab
Copy link
Member

carlocab commented Oct 5, 2021

Fixed in #12186. Thanks for the report, @thewilsonator.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

critical Critical change which should be shipped as soon as possible. outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants