From 85d48da3640fe7de496fd941375088c9baff3645 Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Sun, 15 Jun 2025 15:28:43 +0300 Subject: [PATCH 1/3] Refactor FormulaCreator args and call parse_url automatically Co-authored-by: Mike McQuaid --- Library/Homebrew/dev-cmd/create.rb | 11 ++- Library/Homebrew/formula_creator.rb | 64 +++++++++-------- Library/Homebrew/test/formula_creator_spec.rb | 68 +++++++++++++------ 3 files changed, 87 insertions(+), 56 deletions(-) diff --git a/Library/Homebrew/dev-cmd/create.rb b/Library/Homebrew/dev-cmd/create.rb index e3c4f660f8..b74be43d40 100644 --- a/Library/Homebrew/dev-cmd/create.rb +++ b/Library/Homebrew/dev-cmd/create.rb @@ -184,16 +184,15 @@ module Homebrew end fc = FormulaCreator.new( - args.set_name, - args.set_version, - tap: args.tap, url: args.named.fetch(0), + name: args.set_name, + version: args.set_version, + tap: args.tap, mode:, license: args.set_license, fetch: !args.no_fetch?, head: args.HEAD?, ) - fc.parse_url # ask for confirmation if name wasn't passed explicitly if args.set_name.blank? print "Formula name [#{fc.name}]: " @@ -205,7 +204,7 @@ module Homebrew # Check for disallowed formula, or names that shadow aliases, # unless --force is specified. unless args.force? - if (reason = MissingFormula.disallowed_reason(fc.name)) + if (reason = MissingFormula.disallowed_reason(T.must(fc.name))) odie <<~EOS The formula '#{fc.name}' is not allowed to be created. #{reason} @@ -229,7 +228,7 @@ module Homebrew formula = Homebrew.with_no_api_env do CoreTap.instance.clear_cache - Formula[fc.name] + Formula[T.must(fc.name)] end PyPI.update_python_resources! formula, ignore_non_pypi_packages: true if args.python? diff --git a/Library/Homebrew/formula_creator.rb b/Library/Homebrew/formula_creator.rb index d5c66228bd..399ebaaec5 100644 --- a/Library/Homebrew/formula_creator.rb +++ b/Library/Homebrew/formula_creator.rb @@ -7,21 +7,30 @@ require "erb" module Homebrew # Class for generating a formula from a template. class FormulaCreator + sig { returns(T.nilable(String)) } attr_accessor :name + sig { returns(T.nilable(Version)) } + attr_reader :version + + sig { returns(T::Boolean) } + attr_reader :head + sig { - params(name: T.nilable(String), version: T.nilable(String), tap: T.nilable(String), url: String, + params(url: String, name: T.nilable(String), version: T.nilable(String), tap: T.nilable(String), mode: T.nilable(Symbol), license: T.nilable(String), fetch: T::Boolean, head: T::Boolean).void } - def initialize(name, version, tap:, url:, mode:, license:, fetch:, head:) - @name = name - @version = Version.new(version) if version - @tap = Tap.fetch(tap || "homebrew/core") + def initialize(url:, name: nil, version: nil, tap: nil, mode: nil, license: nil, fetch: false, head: false) @url = url - @mode = mode - @license = license + @name = name.presence + @version = T.let(Version.new(version), T.nilable(Version)) if version.present? + @tap = T.let(Tap.fetch(tap.presence || "homebrew/core"), Tap) + @mode = T.let(mode.presence, T.nilable(Symbol)) + @license = T.let(license.presence, T.nilable(String)) @fetch = fetch @head = head + + parse_url end sig { void } @@ -29,28 +38,27 @@ module Homebrew raise TapUnavailableError, @tap.name unless @tap.installed? end - sig { params(url: String).returns(T.nilable(String)) } - def self.name_from_url(url) - stem = Pathname.new(url).stem - # special cases first - if stem.start_with? "index.cgi" - # gitweb URLs e.g. http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary - stem.rpartition("=").last - elsif url =~ %r{github\.com/\S+/(\S+)/(archive|releases)/} - # e.g. https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz - Regexp.last_match(1) - else - # e.g. http://digit-labs.org/files/tools/synscan/releases/synscan-5.02.tar.gz - pathver = Version.parse(stem).to_s - stem.sub(/[-_.]?#{Regexp.escape(pathver)}$/, "") - end - end - sig { void } def parse_url - @name = FormulaCreator.name_from_url(@url) if @name.blank? - odebug "name_from_url: #{@name}" - @version = Version.detect(@url) if @version.nil? + @name ||= begin + stem = Pathname.new(@url).stem + name = if stem.start_with? "index.cgi" + # special cases first + # gitweb URLs e.g. http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary + stem.rpartition("=").last + elsif @url =~ %r{github\.com/\S+/(\S+)/(archive|releases)/} + # e.g. https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz + Regexp.last_match(1) + else + # e.g. http://digit-labs.org/files/tools/synscan/releases/synscan-5.02.tar.gz + pathver = Version.parse(stem).to_s + stem.sub(/[-_.]?#{Regexp.escape(pathver)}$/, "") + end + odebug "name from url: #{name}" + name + end + + @version ||= Version.detect(@url) case @url when %r{github\.com/(\S+)/(\S+)\.git} @@ -91,7 +99,7 @@ module Homebrew raise "Downloaded URL is not archive" end - @sha256 = filepath.sha256 + @sha256 = T.let(filepath.sha256, T.nilable(String)) end if @github diff --git a/Library/Homebrew/test/formula_creator_spec.rb b/Library/Homebrew/test/formula_creator_spec.rb index a708686406..772a123d05 100644 --- a/Library/Homebrew/test/formula_creator_spec.rb +++ b/Library/Homebrew/test/formula_creator_spec.rb @@ -3,28 +3,52 @@ require "formula_creator" RSpec.describe Homebrew::FormulaCreator do - it "gets name from GitHub archive URL" do - t = described_class.name_from_url("https://github.com/abitrolly/lapce/archive/v0.3.0.tar.gz") - expect(t).to eq("lapce") - end + describe ".new" do + tests = { + "generic tarball URL": { + url: "http://digit-labs.org/files/tools/synscan/releases/synscan-5.02.tar.gz", + expected: { + name: "synscan", + version: "5.02", + }, + }, + "gitweb URL": { + url: "http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary", + expected: { + name: "libzipper", + }, + }, + "GitHub repo URL": { + url: "https://github.com/abitrolly/lapce.git", + expected: { + name: "lapce", + head: true, + }, + }, + "GitHub archive URL": { + url: "https://github.com/abitrolly/lapce/archive/v0.3.0.tar.gz", + expected: { + name: "lapce", + version: "0.3.0", + }, + }, + "GitHub download URL": { + url: "https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz", + expected: { + name: "stella", + version: "6.7", + }, + }, + } - it "gets name from gitweb URL" do - t = described_class.name_from_url("http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary") - expect(t).to eq("libzipper") - end - - it "gets name from GitHub repo URL" do - t = described_class.name_from_url("https://github.com/abitrolly/lapce.git") - expect(t).to eq("lapce") - end - - it "gets name from GitHub download URL" do - t = described_class.name_from_url("https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz") - expect(t).to eq("stella") - end - - it "gets name from generic tarball URL" do - t = described_class.name_from_url("http://digit-labs.org/files/tools/synscan/releases/synscan-5.02.tar.gz") - expect(t).to eq("synscan") + tests.each do |description, test| + it "parses #{description}" do + fc = described_class.new(url: test.fetch(:url)) + ex = test.fetch(:expected) + expect(fc.name).to eq(ex[:name]) if ex.key?(:name) + expect(fc.version).to eq(ex[:version]) if ex.key?(:version) + expect(fc.head).to eq(ex[:head]) if ex.key?(:head) + end + end end end From c5d091af219e6930aa6b5c85b3e3c19febb9e6eb Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 17 Jun 2025 11:44:15 +0100 Subject: [PATCH 2/3] Refactor/typecheck `create` and `formula_creator` --- Library/Homebrew/dev-cmd/create.rb | 30 +++---- Library/Homebrew/formula_creator.rb | 79 +++++++++++-------- Library/Homebrew/test/formula_creator_spec.rb | 51 ++++++------ 3 files changed, 84 insertions(+), 76 deletions(-) diff --git a/Library/Homebrew/dev-cmd/create.rb b/Library/Homebrew/dev-cmd/create.rb index b74be43d40..edf874afeb 100644 --- a/Library/Homebrew/dev-cmd/create.rb +++ b/Library/Homebrew/dev-cmd/create.rb @@ -183,7 +183,7 @@ module Homebrew :zig end - fc = FormulaCreator.new( + formula_creator = FormulaCreator.new( url: args.named.fetch(0), name: args.set_name, version: args.set_version, @@ -193,30 +193,32 @@ module Homebrew fetch: !args.no_fetch?, head: args.HEAD?, ) + # ask for confirmation if name wasn't passed explicitly if args.set_name.blank? - print "Formula name [#{fc.name}]: " - fc.name = __gets || fc.name + print "Formula name [#{formula_creator.name}]: " + confirmed_name = __gets.presence + formula_creator.name = confirmed_name if confirmed_name.present? end - fc.verify + formula_creator.verify_tap_available! # Check for disallowed formula, or names that shadow aliases, # unless --force is specified. unless args.force? - if (reason = MissingFormula.disallowed_reason(T.must(fc.name))) + if (reason = MissingFormula.disallowed_reason(formula_creator.name)) odie <<~EOS - The formula '#{fc.name}' is not allowed to be created. + The formula '#{formula_creator.name}' is not allowed to be created. #{reason} If you really want to create this formula use `--force`. EOS end Homebrew.with_no_api_env do - if Formula.aliases.include? fc.name - realname = Formulary.canonical_name(fc.name) + if Formula.aliases.include?(formula_creator.name) + realname = Formulary.canonical_name(formula_creator.name) odie <<~EOS - The formula '#{realname}' is already aliased to '#{fc.name}'. + The formula '#{realname}' is already aliased to '#{formula_creator.name}'. Please check that you are not creating a duplicate. To force creation use `--force`. EOS @@ -224,19 +226,19 @@ module Homebrew end end - path = fc.write_formula! + path = formula_creator.write_formula! formula = Homebrew.with_no_api_env do CoreTap.instance.clear_cache - Formula[T.must(fc.name)] + Formula[formula_creator.name] end PyPI.update_python_resources! formula, ignore_non_pypi_packages: true if args.python? puts <<~EOS Please audit and test formula before submitting: - HOMEBREW_NO_INSTALL_FROM_API=1 brew audit --new #{fc.name} - HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source --verbose --debug #{fc.name} - HOMEBREW_NO_INSTALL_FROM_API=1 brew test #{fc.name} + HOMEBREW_NO_INSTALL_FROM_API=1 brew audit --new #{formula_creator.name} + HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source --verbose --debug #{formula_creator.name} + HOMEBREW_NO_INSTALL_FROM_API=1 brew test #{formula_creator.name} EOS path end diff --git a/Library/Homebrew/formula_creator.rb b/Library/Homebrew/formula_creator.rb index 399ebaaec5..4fc3d7c430 100644 --- a/Library/Homebrew/formula_creator.rb +++ b/Library/Homebrew/formula_creator.rb @@ -1,4 +1,4 @@ -# typed: true # rubocop:todo Sorbet/StrictSigil +# typed: strict # frozen_string_literal: true require "digest" @@ -7,10 +7,10 @@ require "erb" module Homebrew # Class for generating a formula from a template. class FormulaCreator - sig { returns(T.nilable(String)) } + sig { returns(String) } attr_accessor :name - sig { returns(T.nilable(Version)) } + sig { returns(Version) } attr_reader :version sig { returns(T::Boolean) } @@ -22,55 +22,66 @@ module Homebrew } def initialize(url:, name: nil, version: nil, tap: nil, mode: nil, license: nil, fetch: false, head: false) @url = url - @name = name.presence - @version = T.let(Version.new(version), T.nilable(Version)) if version.present? - @tap = T.let(Tap.fetch(tap.presence || "homebrew/core"), Tap) - @mode = T.let(mode.presence, T.nilable(Symbol)) - @license = T.let(license.presence, T.nilable(String)) - @fetch = fetch - @head = head - parse_url - end - - sig { void } - def verify - raise TapUnavailableError, @tap.name unless @tap.installed? - end - - sig { void } - def parse_url - @name ||= begin - stem = Pathname.new(@url).stem - name = if stem.start_with? "index.cgi" + if name.blank? + stem = Pathname.new(url).stem + name = if stem.start_with?("index.cgi") && stem.include?("=") # special cases first # gitweb URLs e.g. http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary stem.rpartition("=").last - elsif @url =~ %r{github\.com/\S+/(\S+)/(archive|releases)/} + elsif url =~ %r{github\.com/\S+/(\S+)/(archive|releases)/} # e.g. https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz - Regexp.last_match(1) + T.must(Regexp.last_match(1)) else # e.g. http://digit-labs.org/files/tools/synscan/releases/synscan-5.02.tar.gz pathver = Version.parse(stem).to_s stem.sub(/[-_.]?#{Regexp.escape(pathver)}$/, "") end odebug "name from url: #{name}" - name end + @name = T.let(name, String) - @version ||= Version.detect(@url) + version = if version.present? + Version.new(version) + else + Version.detect(url) + end + @version = T.let(version, Version) - case @url + tap = if tap.blank? + CoreTap.instance + else + Tap.fetch(tap) + end + @tap = T.let(tap, Tap) + + @mode = T.let(mode.presence, T.nilable(Symbol)) + @license = T.let(license.presence, T.nilable(String)) + @fetch = fetch + + case url when %r{github\.com/(\S+)/(\S+)\.git} - @head = true + head = true user = Regexp.last_match(1) - repo = Regexp.last_match(2) - @github = GitHub.repository(user, repo) if @fetch + repository = Regexp.last_match(2) + github = GitHub.repository(user, repository) if fetch when %r{github\.com/(\S+)/(\S+)/(archive|releases)/} user = Regexp.last_match(1) - repo = Regexp.last_match(2) - @github = GitHub.repository(user, repo) if @fetch + repository = Regexp.last_match(2) + github = GitHub.repository(user, repository) if fetch end + @head = head + @github = T.let(github, T.untyped) + + @sha256 = T.let(nil, T.nilable(String)) + @desc = T.let(nil, T.nilable(String)) + @homepage = T.let(nil, T.nilable(String)) + @license = T.let(nil, T.nilable(String)) + end + + sig { void } + def verify_tap_available! + raise TapUnavailableError, @tap.name unless @tap.installed? end sig { returns(Pathname) } @@ -114,6 +125,8 @@ module Homebrew path end + private + sig { params(name: String).returns(String) } def latest_versioned_formula(name) name_prefix = "#{name}@" diff --git a/Library/Homebrew/test/formula_creator_spec.rb b/Library/Homebrew/test/formula_creator_spec.rb index 772a123d05..28d5c03f0d 100644 --- a/Library/Homebrew/test/formula_creator_spec.rb +++ b/Library/Homebrew/test/formula_creator_spec.rb @@ -6,48 +6,41 @@ RSpec.describe Homebrew::FormulaCreator do describe ".new" do tests = { "generic tarball URL": { - url: "http://digit-labs.org/files/tools/synscan/releases/synscan-5.02.tar.gz", - expected: { - name: "synscan", - version: "5.02", - }, + url: "http://digit-labs.org/files/tools/synscan/releases/synscan-5.02.tar.gz", + name: "synscan", + version: "5.02", }, "gitweb URL": { - url: "http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary", - expected: { - name: "libzipper", - }, + url: "http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary", + name: "libzipper", }, "GitHub repo URL": { - url: "https://github.com/abitrolly/lapce.git", - expected: { - name: "lapce", - head: true, - }, + url: "https://github.com/abitrolly/lapce.git", + name: "lapce", + head: true, }, "GitHub archive URL": { - url: "https://github.com/abitrolly/lapce/archive/v0.3.0.tar.gz", - expected: { - name: "lapce", - version: "0.3.0", - }, + url: "https://github.com/abitrolly/lapce/archive/v0.3.0.tar.gz", + name: "lapce", + version: "0.3.0", }, "GitHub download URL": { - url: "https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz", - expected: { - name: "stella", - version: "6.7", - }, + url: "https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz", + name: "stella", + version: "6.7", }, } tests.each do |description, test| it "parses #{description}" do - fc = described_class.new(url: test.fetch(:url)) - ex = test.fetch(:expected) - expect(fc.name).to eq(ex[:name]) if ex.key?(:name) - expect(fc.version).to eq(ex[:version]) if ex.key?(:version) - expect(fc.head).to eq(ex[:head]) if ex.key?(:head) + formula_creator = described_class.new(url: test.fetch(:url)) + expect(formula_creator.name).to eq(test.fetch(:name)) + if (version = test[:version]) + expect(formula_creator.version).to eq(version) + else + expect(formula_creator.version).to be_null + end + expect(formula_creator.head).to eq(test.fetch(:head, false)) end end end From 1f29f516544de8b6a1daa381a48a4c0b4e8b64a9 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 17 Jun 2025 14:44:59 +0100 Subject: [PATCH 3/3] dev-cmd/create: avoid duplicate presence check. Co-authored-by: Anatoli Babenia --- Library/Homebrew/dev-cmd/create.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/dev-cmd/create.rb b/Library/Homebrew/dev-cmd/create.rb index edf874afeb..1823ee59d9 100644 --- a/Library/Homebrew/dev-cmd/create.rb +++ b/Library/Homebrew/dev-cmd/create.rb @@ -197,7 +197,7 @@ module Homebrew # ask for confirmation if name wasn't passed explicitly if args.set_name.blank? print "Formula name [#{formula_creator.name}]: " - confirmed_name = __gets.presence + confirmed_name = __gets formula_creator.name = confirmed_name if confirmed_name.present? end