From 08c927b6a17fa36368e389d552fa8bb3f9240611 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Sun, 8 Dec 2024 12:22:57 -0500 Subject: [PATCH 1/7] Json: Allow nil regex block argument This updates the block-handling logic in `Json::versions_from_content` to naively pass the regex value when the block has two parameters. Up to now, we have been ensuring that `regex` is not `nil` and this makes sense with existing usage (the `Crate` strategy's default block, formulae/cask `strategy` blocks). However, we need to allow a `nil` `regex` value to make it possible to add an optional `regex` parameter in the `Pypi::DEFAULT_BLOCK` Proc. This is necessary to allow the `Pypi` strategy to work with an optional regex from a `livecheck` block again [without creating an additional `DEFAULT_BLOCK` variant with a regex parameter]. --- Library/Homebrew/livecheck/strategy/json.rb | 10 +++++----- Library/Homebrew/test/livecheck/strategy/json_spec.rb | 5 ----- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/Library/Homebrew/livecheck/strategy/json.rb b/Library/Homebrew/livecheck/strategy/json.rb index a4631c58e2327..b8f05e29ae725 100644 --- a/Library/Homebrew/livecheck/strategy/json.rb +++ b/Library/Homebrew/livecheck/strategy/json.rb @@ -58,8 +58,10 @@ def self.parse_json(content) end # Parses JSON text and identifies versions using a `strategy` block. - # If a regex is provided, it will be passed as the second argument to - # the `strategy` block (after the parsed JSON data). + # If the block has two parameters, the parsed JSON data will be used as + # the first argument and the regex (if any) will be the second. + # Otherwise, only the parsed JSON data will be passed to the block. + # # @param content [String] the JSON text to parse and check # @param regex [Regexp, nil] a regex used for matching versions in the # content @@ -77,10 +79,8 @@ def self.versions_from_content(content, regex = nil, &block) json = parse_json(content) return [] if json.blank? - block_return_value = if regex.present? + block_return_value = if block.arity == 2 yield(json, regex) - elsif block.arity == 2 - raise "Two arguments found in `strategy` block but no regex provided." else yield(json) end diff --git a/Library/Homebrew/test/livecheck/strategy/json_spec.rb b/Library/Homebrew/test/livecheck/strategy/json_spec.rb index dc18a8f074512..5949d83e19bf0 100644 --- a/Library/Homebrew/test/livecheck/strategy/json_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/json_spec.rb @@ -107,11 +107,6 @@ expect(json.versions_from_content(content_simple, regex) { next }).to eq([]) end - it "errors if a block uses two arguments but a regex is not given" do - expect { json.versions_from_content(content_simple) { |json, regex| json["version"][regex, 1] } } - .to raise_error("Two arguments found in `strategy` block but no regex provided.") - end - it "errors on an invalid return type from a block" do expect { json.versions_from_content(content_simple, regex) { 123 } } .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG) From 270313f6490bdeee73b7db748580d11f52ca1247 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Sun, 8 Dec 2024 12:47:50 -0500 Subject: [PATCH 2/7] Pypi: Restore regex support We recently updated the `Pypi` strategy to use the PyPI JSON API and the default strategy behavior no longer relies on a regex, so the initial implementation didn't include regex handling. This restores support for a `livecheck` block regex by updating the `DEFAULT_BLOCK` logic to handle an optional regex. This allows us to use a regex to omit parts of the `info.version` value without having to duplicate the default block logic in a `strategy` block only to use a regex. This isn't currently necessary for any existing formulae using the `Pypi` strategy but we have a few that needed a custom regex with the previous strategy approach, so they may need this functionality in the future. Besides that, restoring regex support to `Pypi` ensures that `livecheck`/`strategy` blocks work in a fairly consistent manner across strategies. --- Library/Homebrew/livecheck/strategy/pypi.rb | 10 ++++--- .../test/livecheck/strategy/pypi_spec.rb | 26 ++++++++++++++----- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/Library/Homebrew/livecheck/strategy/pypi.rb b/Library/Homebrew/livecheck/strategy/pypi.rb index 66351f884c221..8711a2264a539 100644 --- a/Library/Homebrew/livecheck/strategy/pypi.rb +++ b/Library/Homebrew/livecheck/strategy/pypi.rb @@ -20,10 +20,14 @@ class Pypi # The default `strategy` block used to extract version information when # a `strategy` block isn't provided. - DEFAULT_BLOCK = T.let(proc do |json| - json.dig("info", "version").presence + DEFAULT_BLOCK = T.let(proc do |json, regex| + version = json.dig("info", "version") + next if version.blank? + + regex ? version[regex, 1] : version end.freeze, T.proc.params( - arg0: T::Hash[String, T.untyped], + json: T::Hash[String, T.untyped], + regex: T.nilable(Regexp), ).returns(T.nilable(String))) # The `Regexp` used to extract the package name and suffix (e.g. file diff --git a/Library/Homebrew/test/livecheck/strategy/pypi_spec.rb b/Library/Homebrew/test/livecheck/strategy/pypi_spec.rb index be93644477108..d1f44376bb549 100644 --- a/Library/Homebrew/test/livecheck/strategy/pypi_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/pypi_spec.rb @@ -8,7 +8,7 @@ let(:pypi_url) { "https://files.pythonhosted.org/packages/ab/cd/efg/example-package-1.2.3.tar.gz" } let(:non_pypi_url) { "https://brew.sh/test" } - let(:regex) { /^v?(\d+(?:\.\d+)+)$/i } + let(:regex) { /^v?(\d+(?:\.\d+)+)/i } let(:generated) do { @@ -17,25 +17,26 @@ end # This is a limited subset of a PyPI JSON API response object, for the sake - # of testing. + # of testing. Typical versions use a `1.2.3` format but this adds a suffix, + # so we can test regex matching. let(:content) do <<~JSON { "info": { - "version": "1.2.3" + "version": "1.2.3-456" } } JSON end - let(:matches) { ["1.2.3"] } + let(:matches) { ["1.2.3-456"] } let(:find_versions_return_hash) do { matches: { - "1.2.3" => Version.new("1.2.3"), + "1.2.3-456" => Version.new("1.2.3-456"), }, - regex: nil, + regex:, url: generated[:url], } end @@ -76,10 +77,17 @@ { cached:, cached_default: cached.merge({ matches: {} }), + cached_regex: cached.merge({ + matches: { "1.2.3" => Version.new("1.2.3") }, + regex:, + }), } end it "finds versions in provided content" do + expect(pypi.find_versions(url: pypi_url, regex:, provided_content: content)) + .to eq(match_data[:cached_regex]) + expect(pypi.find_versions(url: pypi_url, provided_content: content)) .to eq(match_data[:cached]) end @@ -92,7 +100,7 @@ next if match.blank? match[1] - end).to eq(match_data[:cached].merge({ regex: })) + end).to eq(match_data[:cached_regex]) expect(pypi.find_versions(url: pypi_url, provided_content: content) do |json| json.dig("info", "version").presence @@ -100,10 +108,14 @@ end it "returns default match_data when block doesn't return version information" do + no_match_regex = /will_not_match/i + expect(pypi.find_versions(url: pypi_url, provided_content: '{"info":{"version":""}}')) .to eq(match_data[:cached_default]) expect(pypi.find_versions(url: pypi_url, provided_content: '{"other":true}')) .to eq(match_data[:cached_default]) + expect(pypi.find_versions(url: pypi_url, regex: no_match_regex, provided_content: content)) + .to eq(match_data[:cached_default].merge({ regex: no_match_regex })) end it "returns default match_data when url is blank" do From ebede5631462767687d54f9c1ad6d93ecc941b4e Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Sun, 8 Dec 2024 15:48:43 -0800 Subject: [PATCH 3/7] cask/artifact/abstract_uninstall: handle sudo trashed paths correctly This showed a confusing message when the trash path was able to be deleted using sudo since the untrashable array was updated but the check was higher up for returning early. ``` ==> Trashing files: /Users/Shared/Maxon /Users/Shared/Red Giant ~/Library/Application Support/Maxon ~/Library/Application Support/Red Giant ~/Library/Caches/net.maxon.app-manager ~/Library/Preferences/Maxon ~/Library/Preferences/net.maxon.app-manager.plist ~/Library/Saved Application State/net.maxon.app-manager.savedState ==> Using sudo to gain ownership of path '/Users/Shared/Maxon' ==> Using sudo to gain ownership of path '/Users/Shared/Red Giant' Warning: The following files could not be trashed, please do so manually: ==> Removing all staged versions of Cask 'maxon' ``` The warning about files not getting trashed should only be shown if some files didn't get trashed. Fixes https://github.com/Homebrew/brew/issues/18901 --- Library/Homebrew/cask/artifact/abstract_uninstall.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index 0cfdcad7bad21..2242b7961a72c 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -468,9 +468,7 @@ def trash_paths(*paths, command: nil, **_) trashed = trashed.split(":") untrashable = untrashable.split(":") - return trashed, untrashable if untrashable.empty? - - untrashable.delete_if do |path| + trashed_with_permissions, untrashable = untrashable.partition do |path| Utils.gain_permissions(path, ["-R"], SystemCommand) do system_command! HOMEBREW_LIBRARY_PATH/"cask/utils/trash.swift", args: [path], @@ -482,6 +480,10 @@ def trash_paths(*paths, command: nil, **_) false end + trashed += trashed_with_permissions + + return trashed, untrashable if untrashable.empty? + opoo "The following files could not be trashed, please do so manually:" $stderr.puts untrashable From ccdf39ff4ea0e5ece9f2722e2576bfdcf32c521b Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Tue, 10 Dec 2024 05:24:10 +0000 Subject: [PATCH 4/7] dev-cmd/tap-new: improve handling of multi-user setups --- Library/Homebrew/dev-cmd/tap-new.rb | 25 +++++++++++++++++++++---- Library/Homebrew/utils/github/api.rb | 13 ++----------- Library/Homebrew/utils/uid.rb | 9 +++++++++ 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/Library/Homebrew/dev-cmd/tap-new.rb b/Library/Homebrew/dev-cmd/tap-new.rb index 6e59498428590..5f1423fb64cf0 100644 --- a/Library/Homebrew/dev-cmd/tap-new.rb +++ b/Library/Homebrew/dev-cmd/tap-new.rb @@ -4,6 +4,7 @@ require "abstract_command" require "fileutils" require "tap" +require "utils/uid" module Homebrew module DevCmd @@ -172,16 +173,32 @@ def run write_path(tap, ".github/workflows/publish.yml", actions_publish) unless args.no_git? - cd tap.path do + cd tap.path do |path| Utils::Git.set_name_email! Utils::Git.setup_gpg! # Would be nice to use --initial-branch here but it's not available in # older versions of Git that we support. safe_system "git", "-c", "init.defaultBranch=#{branch}", "init" - safe_system "git", "add", "--all" - safe_system "git", "commit", "-m", "Create #{tap} tap" - safe_system "git", "branch", "-m", branch + + args = [] + git_owner = File.stat(File.join(path, ".git")).uid + if git_owner != Process.uid && git_owner == Process.euid + # Under Homebrew user model, EUID is permitted to execute commands under the UID. + # Root users are never allowed (see brew.sh). + args << "-c" << "safe.directory=#{path}" + end + + # Use the configuration of the original user, which will have author information and signing keys. + Utils::UID.drop_euid do + env = { HOME: Utils::UID.uid_home }.compact + env[:TMPDIR] = nil if (tmpdir = ENV.fetch("TMPDIR", nil)) && !File.writable?(tmpdir) + with_env(env) do + safe_system "git", *args, "add", "--all" + safe_system "git", *args, "commit", "-m", "Create #{tap} tap" + safe_system "git", *args, "branch", "-m", branch + end + end end end diff --git a/Library/Homebrew/utils/github/api.rb b/Library/Homebrew/utils/github/api.rb index bcc2bcbe3b057..26b4e3d0aa68e 100644 --- a/Library/Homebrew/utils/github/api.rb +++ b/Library/Homebrew/utils/github/api.rb @@ -135,15 +135,6 @@ def initialize(github_message, errors) JSON::ParserError, ].freeze - sig { returns(T.nilable(String)) } - private_class_method def self.uid_home - require "etc" - Etc.getpwuid(Process.uid)&.dir - rescue ArgumentError - # Cover for misconfigured NSS setups - nil - end - # Gets the token from the GitHub CLI for github.com. sig { returns(T.nilable(String)) } def self.github_cli_token @@ -152,7 +143,7 @@ def self.github_cli_token # Avoid `Formula["gh"].opt_bin` so this method works even with `HOMEBREW_DISABLE_LOAD_FORMULA`. env = { "PATH" => PATH.new(HOMEBREW_PREFIX/"opt/gh/bin", ENV.fetch("PATH")), - "HOME" => uid_home, + "HOME" => Utils::UID.uid_home, }.compact gh_out, _, result = system_command "gh", args: ["auth", "token", "--hostname", "github.com"], @@ -173,7 +164,7 @@ def self.keychain_username_password git_credential_out, _, result = system_command "git", args: ["credential-osxkeychain", "get"], input: ["protocol=https\n", "host=github.com\n"], - env: { "HOME" => uid_home }.compact, + env: { "HOME" => Utils::UID.uid_home }.compact, print_stderr: false return unless result.success? diff --git a/Library/Homebrew/utils/uid.rb b/Library/Homebrew/utils/uid.rb index 4c102a2b8a763..129c569bb8715 100644 --- a/Library/Homebrew/utils/uid.rb +++ b/Library/Homebrew/utils/uid.rb @@ -15,5 +15,14 @@ def self.drop_euid(&_block) Process::Sys.seteuid(original_euid) end end + + sig { returns(T.nilable(String)) } + def self.uid_home + require "etc" + Etc.getpwuid(Process.uid)&.dir + rescue ArgumentError + # Cover for misconfigured NSS setups + nil + end end end From 52569372bbd8a0712d152db6b9bcb016ca69b425 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Tue, 10 Dec 2024 09:24:27 -0500 Subject: [PATCH 5/7] chore(formula): raise error if no universal binaries are found to deuniversalize Signed-off-by: Rui Chen --- Library/Homebrew/formula.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 3ef26943da25b..bede10e37813d 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -2005,6 +2005,8 @@ def time # If called with no parameters, does this with all compatible # universal binaries in a {Formula}'s {Keg}. # + # Raises an error if no universal binaries are found to deuniversalize. + # # @api public sig { params(targets: T.nilable(T.any(Pathname, String))).void } def deuniversalize_machos(*targets) @@ -2014,6 +2016,8 @@ def deuniversalize_machos(*targets) end end + raise "No universal binaries found to deuniversalize" if targets.blank? + targets&.each do |target| extract_macho_slice_from(Pathname(target), Hardware::CPU.arch) end From 84f06173099629bb85d49f678f18629b692fe542 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 10 Dec 2024 16:39:21 +0000 Subject: [PATCH 6/7] build(deps): bump actions/attest-build-provenance from 2.0.1 to 2.1.0 Bumps [actions/attest-build-provenance](https://github.com/actions/attest-build-provenance) from 2.0.1 to 2.1.0. - [Release notes](https://github.com/actions/attest-build-provenance/releases) - [Changelog](https://github.com/actions/attest-build-provenance/blob/main/RELEASE.md) - [Commits](https://github.com/actions/attest-build-provenance/compare/c4fbc648846ca6f503a13a2281a5e7b98aa57202...7668571508540a607bdfd90a87a560489fe372eb) --- updated-dependencies: - dependency-name: actions/attest-build-provenance dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- .github/workflows/pkg-installer.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pkg-installer.yml b/.github/workflows/pkg-installer.yml index a6a92eb4d0f1e..a80f8236524e6 100644 --- a/.github/workflows/pkg-installer.yml +++ b/.github/workflows/pkg-installer.yml @@ -133,7 +133,7 @@ jobs: fi - name: Generate build provenance - uses: actions/attest-build-provenance@c4fbc648846ca6f503a13a2281a5e7b98aa57202 # v2.0.1 + uses: actions/attest-build-provenance@7668571508540a607bdfd90a87a560489fe372eb # v2.1.0 with: subject-path: Homebrew-${{ steps.homebrew-version.outputs.version }}.pkg From abb330d496abe0897d295f1369d845a5b5e1c8c5 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 10 Dec 2024 17:19:20 +0000 Subject: [PATCH 7/7] github/workflows: use merge_group instead of push events. We're currently doing both which doubles the number of jobs we end up running for commits on `master`. --- .github/workflows/actionlint.yml | 3 --- .github/workflows/docker.yml | 5 +---- .github/workflows/docs.yml | 3 --- .github/workflows/tests.yml | 3 --- 4 files changed, 1 insertion(+), 13 deletions(-) diff --git a/.github/workflows/actionlint.yml b/.github/workflows/actionlint.yml index d3b3da0c6ee88..8d68cac31260e 100644 --- a/.github/workflows/actionlint.yml +++ b/.github/workflows/actionlint.yml @@ -2,8 +2,6 @@ name: actionlint on: push: - branches: - - master paths: - '.github/workflows/*.ya?ml' - '.github/actionlint.yaml' @@ -11,7 +9,6 @@ on: paths: - '.github/workflows/*.ya?ml' - '.github/actionlint.yaml' - merge_group: env: HOMEBREW_DEVELOPER: 1 diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index d1c41aef1d56e..ac3c56d8a1594 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -2,9 +2,6 @@ name: Docker on: pull_request: - push: - branches: - - master merge_group: release: types: @@ -75,7 +72,7 @@ jobs: "homebrew/brew:latest" ) fi - elif [[ "${GITHUB_EVENT_NAME}" == "push" && + elif [[ "${GITHUB_EVENT_NAME}" == "merge_group" && "${GITHUB_REF}" == "refs/heads/master" && "${{ matrix.version }}" == "22.04" ]]; then tags+=( diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index d4464c97db4a0..84a849a63ab53 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -1,9 +1,6 @@ name: Documentation CI on: - push: - branches: - - master pull_request: merge_group: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 27985bd4d7e2a..cfeb09c78ad7a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,9 +1,6 @@ name: CI on: - push: - branches: - - master pull_request: merge_group: