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

formula_installer: always pre-install formulae required for fetching sources #12296

Merged
merged 4 commits into from Nov 9, 2021

Conversation

EricFromCanada
Copy link
Member

@EricFromCanada EricFromCanada commented Oct 22, 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?

This fixes two similar issues:

  • ever since the Let's Encrypt CA expiry, the built-in svn on macOS 10.14 and earlier is unable to access repositories hosted on SourceForge (i.e. svn.code.sf.net) and possibly others, so the subversion formula needs to always be installed before attempting to download and build from source any formula that gets its source from SVN. (Also, if a formula URL is marked with trust_cert: true, any unknown or outdated certificate errors will still be ignored.)
  • macOS's built-in curl doesn't (yet) properly handle TLS 1.3 connections, so for installing from source any formulae using such sites (e.g. xiph.org) that are marked with using: :homebrew_curl, brewed curl needs to be installed first.
`svn` before:
% brew install -s acme
==> Downloading https://ghcr.io/v2/homebrew/core/subversion/manifests/1.14.1_4
######################################################################## 100.0%
==> Downloading https://ghcr.io/v2/homebrew/core/subversion/blobs/sha256:34f8d1862f1480c068ff3798c8e1cd90f833b43c33d1731aca15f1d875b16834
==> Downloading from https://pkg-containers.githubusercontent.com/ghcr1/blobs/sha256:34f8d1862f1480c068ff3798c8e1cd90f833b43c33d1731aca15f1d875b16834?se=2021-11-06T18%3A55%3A00Z&sig=6LIHuXbN%2B3UxpRLpjq4hFJQzouvAYhGAPR4j9iBtCN
######################################################################## 100.0%
==> Cloning https://svn.code.sf.net/p/acme-crossass/code-0/trunk
==> Checking out 266
You must: brew install svn
Error: acme: Failed to download resource "acme"
Failure while executing; `svn checkout https://svn.code.sf.net/p/acme-crossass/code-0/trunk /Users/serveradmin/Library/Caches/Homebrew/acme--svn --quiet -r 266` exited with 1. Here's the output:
You must: brew install svn
`svn` after:
% brew install -s acme         
==> Downloading https://ghcr.io/v2/homebrew/core/subversion/manifests/1.14.1_4
Already downloaded: /Users/serveradmin/Library/Caches/Homebrew/downloads/a5926227bbead69887dfea12408cbd3c29e7280770ad0bd62320ef20525fd028--subversion-1.14.1_4.bottle_manifest.json
==> Downloading https://ghcr.io/v2/homebrew/core/subversion/blobs/sha256:34f8d1862f1480c068ff3798c8e1cd90f833b43c33d1731aca15f1d875b16834
Already downloaded: /Users/serveradmin/Library/Caches/Homebrew/downloads/9ba0f1eb092b0a83dfa1bd0d8e31c9569b61f96a189d97a321820bf7c1b9e126--subversion--1.14.1_4.arm64_big_sur.bottle.tar.gz
==> Installing acme dependency: subversion
==> Pouring subversion--1.14.1_4.arm64_big_sur.bottle.tar.gz
🍺  /opt/homebrew/Cellar/subversion/1.14.1_4: 234 files, 33.8MB
==> Cloning https://svn.code.sf.net/p/acme-crossass/code-0/trunk
==> Checking out 266
==> make -C src install BINDIR=/opt/homebrew/Cellar/acme/0.97/bin
🍺  /opt/homebrew/Cellar/acme/0.97: 26 files, 321.9KB, built in 2 seconds
`curl` before:
% brew deps --tree --include-build libvorbis
libvorbis
β”œβ”€β”€ pkg-config
└── libogg

% brew install -s libvorbis
==> Downloading https://upload.wikimedia.org/wikipedia/commons/c/c8/Example.ogg
Already downloaded: /Users/serveradmin/Library/Caches/Homebrew/downloads/e163886f3d57674f022eb584f07ef6d71dbbf58fb39c481a2c516ce6bfd4af18--Example.ogg
==> Downloading https://downloads.xiph.org/releases/vorbis/libvorbis-1.3.7.tar.xz
Trying a mirror...
==> Downloading https://ftp.osuosl.org/pub/xiph/releases/vorbis/libvorbis-1.3.7.tar.xz
Error: libvorbis: Failed to download resource "libvorbis"
Download failed: Homebrew-installed `curl` is not installed for: https://downloads.xiph.org/releases/vorbis/libvorbis-1.3.7.tar.xz
`curl` after:
% brew deps --tree --include-build libvorbis
libvorbis
β”œβ”€β”€ curl
β”‚   β”œβ”€β”€ pkg-config
β”‚   β”œβ”€β”€ brotli
β”‚   β”‚   └── cmake
β”‚   β”œβ”€β”€ libidn2
β”‚   β”‚   β”œβ”€β”€ pkg-config
β”‚   β”‚   β”œβ”€β”€ gettext
β”‚   β”‚   └── libunistring
β”‚   β”œβ”€β”€ libnghttp2
β”‚   β”‚   └── pkg-config
β”‚   β”œβ”€β”€ libssh2
β”‚   β”‚   └── openssl@1.1
β”‚   β”‚       └── ca-certificates
β”‚   β”œβ”€β”€ openldap
β”‚   β”‚   └── openssl@1.1
β”‚   β”‚       └── ca-certificates
β”‚   β”œβ”€β”€ openssl@1.1
β”‚   β”‚   └── ca-certificates
β”‚   β”œβ”€β”€ rtmpdump
β”‚   β”‚   └── openssl@1.1
β”‚   β”‚       └── ca-certificates
β”‚   └── zstd
β”‚       └── cmake
β”œβ”€β”€ pkg-config
└── libogg

% brew install -s libvorbis
==> Downloading https://ghcr.io/v2/homebrew/core/rtmpdump/manifests/2.4.20151223_1
Already downloaded: /Users/serveradmin/Library/Caches/Homebrew/downloads/cc1598f2946e431654b278ceac5c9e7f21238bb5ed66d9e5c8640f9030a528cc--rtmpdump-2.4+20151223_1.bottle_manifest.json
==> Downloading https://ghcr.io/v2/homebrew/core/rtmpdump/blobs/sha256:67c47ecf95d2f4367685fb0ab04c913d55743e5bafccce721f665c6579f3b599
Already downloaded: /Users/serveradmin/Library/Caches/Homebrew/downloads/f27b0c32ba3ce61628afbe09b4850f065480f938e8d441591fddff48310d4780--rtmpdump--2.4+20151223_1.arm64_big_sur.bottle.tar.gz
==> Downloading https://ghcr.io/v2/homebrew/core/zstd/manifests/1.5.0
######################################################################## 100.0%
==> Downloading https://ghcr.io/v2/homebrew/core/zstd/blobs/sha256:e8962c7923904213f312c86372b670b6b5a7ac7103ee63254ab3d1c349913246
==> Downloading from https://pkg-containers.githubusercontent.com/ghcr1/blobs/sha256:e8962c7923904213f312c86372b670b6b5a7ac7103ee63254ab3d1c349913246?se=2021-11-06T20%3A10%3A00Z&sig=4vyBfrai281HLjH%2B4H9r9KdDBO5Iur3L%2BzBfaPtc
######################################################################## 100.0%
==> Downloading https://ghcr.io/v2/homebrew/core/curl/manifests/7.79.1_1
Already downloaded: /Users/serveradmin/Library/Caches/Homebrew/downloads/1778e02b2a4545c6f17f9db910fb589048c2fac70144a261fc05c670d2ccf598--curl-7.79.1_1.bottle_manifest.json
==> Downloading https://ghcr.io/v2/homebrew/core/curl/blobs/sha256:fb8bc11a92e6b9c69415ebcc0ac30a69d5fa7399ed48eda8f39acd08fb030f91
Already downloaded: /Users/serveradmin/Library/Caches/Homebrew/downloads/a72698a6aacff55c1e13fdb3d8b765dd82948b0f404f8de4be3c393f59285ecb--curl--7.79.1_1.arm64_big_sur.bottle.tar.gz
==> Installing libvorbis dependency: curl
==> Installing dependencies for curl: rtmpdump and zstd
==> Installing curl dependency: rtmpdump
==> Pouring rtmpdump--2.4+20151223_1.arm64_big_sur.bottle.tar.gz
🍺  /opt/homebrew/Cellar/rtmpdump/2.4+20151223_1: 20 files, 711KB
==> Installing curl dependency: zstd
==> Pouring zstd--1.5.0.arm64_big_sur.bottle.tar.gz
🍺  /opt/homebrew/Cellar/zstd/1.5.0: 31 files, 3.3MB
==> Installing curl
==> Pouring curl--7.79.1_1.arm64_big_sur.bottle.tar.gz
🍺  /opt/homebrew/Cellar/curl/7.79.1_1: 486 files, 4.0MB
==> Downloading https://upload.wikimedia.org/wikipedia/commons/c/c8/Example.ogg
Already downloaded: /Users/serveradmin/Library/Caches/Homebrew/downloads/e163886f3d57674f022eb584f07ef6d71dbbf58fb39c481a2c516ce6bfd4af18--Example.ogg
==> Downloading https://downloads.xiph.org/releases/vorbis/libvorbis-1.3.7.tar.xz
######################################################################## 100.0%
==> ./configure --prefix=/opt/homebrew/Cellar/libvorbis/1.3.7
==> make install
🍺  /opt/homebrew/Cellar/libvorbis/1.3.7: 157 files, 2.4MB, built in 7 seconds

xref. Homebrew/homebrew-core#87527, #11724 (cask SVN dependencies not yet handled)

@BrewTestBot
Copy link
Member

Review period will end on 2021-10-25 at 00:53:23 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Oct 22, 2021
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.

I think it'd be good to use Homebrew's Subversion instead of silencing reducing security on the use of the system Subversion in these cases. Thoughts?

@BrewTestBot BrewTestBot added waiting for feedback Merging is blocked until sufficient time has passed for review and removed waiting for feedback Merging is blocked until sufficient time has passed for review labels Oct 22, 2021
@EricFromCanada
Copy link
Member Author

I suspected that might come up, which is why I was looking into how to get SVN to auto-install for #11724.

@BrewTestBot BrewTestBot added waiting for feedback Merging is blocked until sufficient time has passed for review and removed waiting for feedback Merging is blocked until sufficient time has passed for review labels Oct 22, 2021
@EricFromCanada EricFromCanada added the help wanted We want help addressing this label Nov 1, 2021
@BrewTestBot
Copy link
Member

Review period ended.

@Bo98
Copy link
Member

Bo98 commented Nov 1, 2021

The formula side of it should be trivial since we already have a system in place for >= Catalina which we can remove the restriction from.

@EricFromCanada
Copy link
Member Author

The issue, though, is that >= Catalina doesn't auto-install SVN. It'd be nice if it did.

@MikeMcQuaid
Copy link
Member

It'd be nice if it did.

Agreed.

@Bo98
Copy link
Member

Bo98 commented Nov 1, 2021

Ah yes, all that sort of stuff is currently test-bot specific for now: https://github.com/Homebrew/homebrew-test-bot/blob/6b596cf15dd3aceb7e83a4c1b1dc31a618ae7c40/lib/tests/formulae.rb#L285-L287

The reason being that our FormulaInstaller fetches everything before installing, but we need subversion installed before fetching.

@luckydonald

This comment has been minimized.

@EricFromCanada EricFromCanada changed the title svn: work around certificate errors on macOS pre-10.15 formula_installer: always pre-install formulae required for fetching sources Nov 3, 2021
@EricFromCanada EricFromCanada marked this pull request as draft November 3, 2021 22:26
@EricFromCanada

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.

Great work so far @EricFromCanada!

Library/Homebrew/dependency_collector.rb Outdated Show resolved Hide resolved
Library/Homebrew/download_strategy.rb Outdated Show resolved Hide resolved
Library/Homebrew/formula_installer.rb Outdated Show resolved Hide resolved
Library/Homebrew/utils/svn.rb Outdated Show resolved Hide resolved
@EricFromCanada EricFromCanada force-pushed the svn-certificates branch 5 times, most recently from c0a4f19 to 1af42a2 Compare November 6, 2021 22:23
@EricFromCanada EricFromCanada marked this pull request as ready for review November 6, 2021 22:29
@EricFromCanada EricFromCanada removed the help wanted We want help addressing this label Nov 7, 2021
Library/Homebrew/utils/svn.rb Outdated Show resolved Hide resolved
Library/Homebrew/utils/svn.rb Outdated Show resolved Hide resolved
@EricFromCanada
Copy link
Member Author

EricFromCanada commented Nov 8, 2021

Would this allow skipping the preinstallation of subversion and curl in the tests.yml workflow?

@MikeMcQuaid
Copy link
Member

Would this allow skipping the preinstallation of subversion and curl in the tests.yml workflow?

@EricFromCanada I think so? Can try it either in this or another PR.

Are you πŸ‘πŸ» to merge this?

@luckydonald
Copy link

luckydonald commented Nov 9, 2021

That πŸ‘ button probably don't yield a notification/email.

@EricFromCanada
Copy link
Member Author

Just removing svn for now; curl should stay at least until #12341 is solved.

@EricFromCanada EricFromCanada merged commit 95d2ca2 into Homebrew:master Nov 9, 2021
@EricFromCanada EricFromCanada deleted the svn-certificates branch November 9, 2021 19:28
@github-actions github-actions bot added the outdated PR was locked due to age label Dec 10, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 10, 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

5 participants