diff --git a/Library/Homebrew/.rubocop.yml b/Library/Homebrew/.rubocop.yml index 795790afd4..98815f0ae4 100644 --- a/Library/Homebrew/.rubocop.yml +++ b/Library/Homebrew/.rubocop.yml @@ -18,6 +18,8 @@ Metrics/AbcSize: Metrics/BlockLength: Max: 100 Exclude: + # TODO: extract more of the bottling logic + - "dev-cmd/bottle.rb" - "test/**/*" Metrics/BlockNesting: Max: 5 @@ -33,8 +35,10 @@ Metrics/PerceivedComplexity: Metrics/MethodLength: Max: 260 Metrics/ModuleLength: - Max: 610 + Max: 500 Exclude: + # TODO: extract more of the bottling logic + - "dev-cmd/bottle.rb" - "test/**/*" Naming/PredicateName: diff --git a/Library/Homebrew/dev-cmd/bottle.rb b/Library/Homebrew/dev-cmd/bottle.rb index 41c71ac682..b317e19715 100644 --- a/Library/Homebrew/dev-cmd/bottle.rb +++ b/Library/Homebrew/dev-cmd/bottle.rb @@ -640,93 +640,109 @@ module Homebrew break if all_bottle end - if args.write? - if all_bottle - all_bottle_hash = nil - bottle_hash["bottle"]["tags"].each do |tag, tag_hash| - filename = Bottle::Filename.new( - formula_name, - bottle_hash["formula"]["pkg_version"], - tag, - bottle_hash["bottle"]["rebuild"], - ) - - if all_bottle_hash.nil? - all_bottle_tag_hash = tag_hash.dup - - all_filename = Bottle::Filename.new( - formula_name, - bottle_hash["formula"]["pkg_version"], - "all", - bottle_hash["bottle"]["rebuild"], - ) - - all_bottle_tag_hash["filename"] = all_filename.url_encode - all_bottle_tag_hash["local_filename"] = all_filename.to_s - cellar = all_bottle_tag_hash.delete("cellar") - - all_bottle_formula_hash = bottle_hash.dup - all_bottle_formula_hash["bottle"]["cellar"] = cellar - all_bottle_formula_hash["bottle"]["tags"] = { all: all_bottle_tag_hash } - - all_bottle_hash = { formula_name => all_bottle_formula_hash } - - puts "Copying #{filename} to #{all_filename}" if args.verbose? - FileUtils.cp filename.to_s, all_filename.to_s - - puts "Writing #{all_filename.json}" if args.verbose? - all_local_json_path = Pathname(all_filename.json) - all_local_json_path.unlink if all_local_json_path.exist? - all_local_json_path.write(JSON.pretty_generate(all_bottle_hash)) - end - - puts "Removing #{filename} and #{filename.json}" if args.verbose? - FileUtils.rm_f [filename.to_s, filename.json] - end - end - - Homebrew.install_bundler_gems! - require "utils/ast" - - path = Pathname.new((HOMEBREW_REPOSITORY/bottle_hash["formula"]["path"]).to_s) - formula = Formulary.factory(path) - formula_ast = Utils::AST::FormulaAST.new(path.read) - checksums = old_checksums(formula, formula_ast, bottle_hash, args: args) - update_or_add = checksums.nil? ? "add" : "update" - - checksums&.each(&bottle.method(:sha256)) - output = bottle_output(bottle) - puts output - - case update_or_add - when "update" - formula_ast.replace_bottle_block(output) - when "add" - formula_ast.add_bottle_block(output) - end - path.atomic_write(formula_ast.process) - - unless args.no_commit? - Utils::Git.set_name_email!(committer: args.committer.blank?) - Utils::Git.setup_gpg! - - if (committer = args.committer) - committer = Utils.parse_author!(committer) - ENV["GIT_COMMITTER_NAME"] = committer[:name] - ENV["GIT_COMMITTER_EMAIL"] = committer[:email] - end - - short_name = formula_name.split("/", -1).last - pkg_version = bottle_hash["formula"]["pkg_version"] - - path.parent.cd do - safe_system "git", "commit", "--no-edit", "--verbose", - "--message=#{short_name}: #{update_or_add} #{pkg_version} bottle.", - "--", path - end - end - else + unless args.write? puts bottle_output(bottle) + next + end + + path = HOMEBREW_REPOSITORY/bottle_hash["formula"]["path"] + formula = Formulary.factory(path) + old_bottle_spec = formula.bottle_specification + + no_bottle_changes = if old_bottle_spec && + bottle_hash["formula"]["pkg_version"] == formula.pkg_version.to_s && + bottle.rebuild != old_bottle_spec.rebuild && + bottle.root_url == old_bottle_spec.root_url + bottle.collector.keys.all? do |tag| + next false if bottle.collector[tag][:cellar] != old_bottle_spec.collector[tag][:cellar] + + bottle.collector[tag][:checksum].hexdigest == old_bottle_spec.collector[tag][:checksum].hexdigest + end + end + + all_bottle_hash = nil + bottle_hash["bottle"]["tags"].each do |tag, tag_hash| + filename = Bottle::Filename.new( + formula_name, + bottle_hash["formula"]["pkg_version"], + tag, + bottle_hash["bottle"]["rebuild"], + ) + + if all_bottle && all_bottle_hash.nil? + all_bottle_tag_hash = tag_hash.dup + + all_filename = Bottle::Filename.new( + formula_name, + bottle_hash["formula"]["pkg_version"], + "all", + bottle_hash["bottle"]["rebuild"], + ) + + all_bottle_tag_hash["filename"] = all_filename.url_encode + all_bottle_tag_hash["local_filename"] = all_filename.to_s + cellar = all_bottle_tag_hash.delete("cellar") + + all_bottle_formula_hash = bottle_hash.dup + all_bottle_formula_hash["bottle"]["cellar"] = cellar + all_bottle_formula_hash["bottle"]["tags"] = { all: all_bottle_tag_hash } + + all_bottle_hash = { formula_name => all_bottle_formula_hash } + + puts "Copying #{filename} to #{all_filename}" if args.verbose? + FileUtils.cp filename.to_s, all_filename.to_s + + puts "Writing #{all_filename.json}" if args.verbose? + all_local_json_path = Pathname(all_filename.json) + all_local_json_path.unlink if all_local_json_path.exist? + all_local_json_path.write(JSON.pretty_generate(all_bottle_hash)) + end + + if all_bottle || no_bottle_changes + puts "Removing #{filename} and #{filename.json}" if args.verbose? + FileUtils.rm_f [filename.to_s, filename.json] + end + end + + next if no_bottle_changes + + Homebrew.install_bundler_gems! + require "utils/ast" + + formula_ast = Utils::AST::FormulaAST.new(path.read) + checksums = old_checksums(formula, formula_ast, bottle_hash, args: args) + update_or_add = checksums.nil? ? "add" : "update" + + checksums&.each(&bottle.method(:sha256)) + output = bottle_output(bottle) + puts output + + case update_or_add + when "update" + formula_ast.replace_bottle_block(output) + when "add" + formula_ast.add_bottle_block(output) + end + path.atomic_write(formula_ast.process) + + next if args.no_commit? + + Utils::Git.set_name_email!(committer: args.committer.blank?) + Utils::Git.setup_gpg! + + if (committer = args.committer) + committer = Utils.parse_author!(committer) + ENV["GIT_COMMITTER_NAME"] = committer[:name] + ENV["GIT_COMMITTER_EMAIL"] = committer[:email] + end + + short_name = formula_name.split("/", -1).last + pkg_version = bottle_hash["formula"]["pkg_version"] + + path.parent.cd do + safe_system "git", "commit", "--no-edit", "--verbose", + "--message=#{short_name}: #{update_or_add} #{pkg_version} bottle.", + "--", path end end end diff --git a/Library/Homebrew/dev-cmd/pr-upload.rb b/Library/Homebrew/dev-cmd/pr-upload.rb index eb960a94e9..e21b98d9dd 100644 --- a/Library/Homebrew/dev-cmd/pr-upload.rb +++ b/Library/Homebrew/dev-cmd/pr-upload.rb @@ -84,9 +84,7 @@ module Homebrew end end - def json_files_and_bottles_hash(args) - json_files = Dir["*.bottle.json"] - odie "No bottle JSON files found in the current working directory" if json_files.blank? + def bottles_hash_from_json_files(json_files, args) puts "Reading JSON files: #{json_files.join(", ")}" if args.verbose? bottles_hash = json_files.reduce({}) do |hash, json_file| @@ -99,13 +97,15 @@ module Homebrew end end - [json_files, bottles_hash] + bottles_hash end def pr_upload args = pr_upload_args.parse - json_files, bottles_hash = json_files_and_bottles_hash(args) + json_files = Dir["*.bottle.json"] + odie "No bottle JSON files found in the current working directory" if json_files.blank? + bottles_hash = bottles_hash_from_json_files(json_files, args) bottle_args = ["bottle", "--merge", "--write"] bottle_args << "--verbose" if args.verbose? @@ -144,9 +144,15 @@ module Homebrew safe_system HOMEBREW_BREW_FILE, *bottle_args + json_files = Dir["*.bottle.json"] + if json_files.blank? + puts "No bottle JSON files after merge, no upload needed!" + return + end + # Reload the JSON files (in case `brew bottle --merge` generated # `all: $SHA256` bottles) - _, bottles_hash = json_files_and_bottles_hash(args) + bottles_hash = bottles_hash_from_json_files(json_files, args) # Check the bottle commits did not break `brew audit` unless args.no_commit?