From 4c42d717fde99ce4b301bf958b853252d29735a3 Mon Sep 17 00:00:00 2001 From: Seeker Date: Thu, 28 Jan 2021 12:36:44 -0800 Subject: [PATCH 1/3] rubocops: add cops to check bottle format, indentation, and order Co-authored-by: Rylan Polster Co-authored-by: Seeker --- Library/Homebrew/rubocops.rb | 4 +- Library/Homebrew/rubocops/bottle.rb | 198 +++++++++++++++ .../rubocops/shared/helper_functions.rb | 7 +- .../bottle/bottle_digest_indentation_spec.rb | 128 ++++++++++ .../rubocops/bottle/bottle_format_spec.rb | 148 +++++++++++ .../test/rubocops/bottle/bottle_order_spec.rb | 240 ++++++++++++++++++ .../bottle/bottle_tag_indentation_spec.rb | 98 +++++++ 7 files changed, 821 insertions(+), 2 deletions(-) create mode 100644 Library/Homebrew/rubocops/bottle.rb create mode 100644 Library/Homebrew/test/rubocops/bottle/bottle_digest_indentation_spec.rb create mode 100644 Library/Homebrew/test/rubocops/bottle/bottle_format_spec.rb create mode 100644 Library/Homebrew/test/rubocops/bottle/bottle_order_spec.rb create mode 100644 Library/Homebrew/test/rubocops/bottle/bottle_tag_indentation_spec.rb diff --git a/Library/Homebrew/rubocops.rb b/Library/Homebrew/rubocops.rb index 26432eefb3..794be945d0 100644 --- a/Library/Homebrew/rubocops.rb +++ b/Library/Homebrew/rubocops.rb @@ -12,6 +12,8 @@ require "rubocop-rails" require "rubocop-rspec" require "rubocop-sorbet" +require "rubocops/unless_multiple_conditions" + require "rubocops/formula_desc" require "rubocops/components_order" require "rubocops/components_redundancy" @@ -32,6 +34,6 @@ require "rubocops/files" require "rubocops/keg_only" require "rubocops/version" require "rubocops/deprecate_disable" -require "rubocops/unless_multiple_conditions" +require "rubocops/bottle" require "rubocops/rubocop-cask" diff --git a/Library/Homebrew/rubocops/bottle.rb b/Library/Homebrew/rubocops/bottle.rb new file mode 100644 index 0000000000..e1e4cd01c5 --- /dev/null +++ b/Library/Homebrew/rubocops/bottle.rb @@ -0,0 +1,198 @@ +# typed: true +# frozen_string_literal: true + +require "rubocops/extend/formula" + +module RuboCop + module Cop + module FormulaAudit + # This cop audits the `bottle` block in formulae. + # + # @api private + class BottleFormat < FormulaCop + extend AutoCorrector + + def audit_formula(_node, _class_node, _parent_class_node, body_node) + bottle_node = find_block(body_node, :bottle) + return if bottle_node.nil? + + sha256_nodes = find_method_calls_by_name(bottle_node.body, :sha256) + cellar_node = find_node_method_by_name(bottle_node.body, :cellar) + cellar_source = cellar_node&.first_argument&.source + + if sha256_nodes.present? && cellar_node.present? + offending_node(cellar_node) + problem "`cellar` should be a parameter to `sha256`" do |corrector| + corrector.remove(range_by_whole_lines(cellar_node.source_range, include_final_newline: true)) + end + end + + sha256_nodes.each do |sha256_node| + sha256_hash = sha256_node.last_argument + sha256_pairs = sha256_hash.pairs + next if sha256_pairs.count != 1 + + sha256_pair = sha256_pairs.first + sha256_key = sha256_pair.key + sha256_value = sha256_pair.value + next unless sha256_value.sym_type? + + tag = sha256_value.value + digest_source = sha256_key.source + sha256_line = if cellar_source.present? + "sha256 cellar: #{cellar_source}, #{tag}: #{digest_source}" + else + "sha256 #{tag}: #{digest_source}" + end + + offending_node(sha256_node) + problem "`sha256` should use new syntax" do |corrector| + corrector.replace(sha256_node.source_range, sha256_line) + end + end + end + end + + # This cop audits the indentation of the bottle tags in the `bottle` block in formulae. + # + # @api private + class BottleTagIndentation < FormulaCop + extend AutoCorrector + + def audit_formula(_node, _class_node, _parent_class_node, body_node) + bottle_node = find_block(body_node, :bottle) + return if bottle_node.nil? + + sha256_nodes = find_method_calls_by_name(bottle_node.body, :sha256) + + max_tag_column = 0 + sha256_nodes.each do |sha256_node| + sha256_hash = sha256_node.last_argument + tag_column = T.let(sha256_hash.pairs.last.source_range.column, Integer) + + max_tag_column = tag_column if tag_column > max_tag_column + end + # This must be in a separate loop to make sure max_tag_column is truly the maximum + sha256_nodes.each do |sha256_node| # rubocop:disable Style/CombinableLoops + sha256_hash = sha256_node.last_argument + hash = sha256_hash.pairs.last + tag_column = hash.source_range.column + + next if tag_column == max_tag_column + + offending_node(hash) + problem "Align bottle tags" do |corrector| + new_line = " " * (max_tag_column - tag_column) + hash.source + corrector.replace(hash.source_range, new_line) + end + end + end + end + + # This cop audits the indentation of the sha256 digests in the`bottle` block in formulae. + # + # @api private + class BottleDigestIndentation < FormulaCop + extend AutoCorrector + + def audit_formula(_node, _class_node, _parent_class_node, body_node) + bottle_node = find_block(body_node, :bottle) + return if bottle_node.nil? + + sha256_nodes = find_method_calls_by_name(bottle_node.body, :sha256) + + max_digest_column = 0 + sha256_nodes.each do |sha256_node| + sha256_hash = sha256_node.last_argument + digest_column = T.let(sha256_hash.pairs.last.value.source_range.column, Integer) + + max_digest_column = digest_column if digest_column > max_digest_column + end + # This must be in a separate loop to make sure max_digest_column is truly the maximum + sha256_nodes.each do |sha256_node| # rubocop:disable Style/CombinableLoops + sha256_hash = sha256_node.last_argument + hash = sha256_hash.pairs.last.value + digest_column = hash.source_range.column + + next if digest_column == max_digest_column + + offending_node(hash) + problem "Align bottle digests" do |corrector| + new_line = " " * (max_digest_column - digest_column) + hash.source + corrector.replace(hash.source_range, new_line) + end + end + end + end + + # This cop audits the order of the `bottle` block in formulae. + # + # @api private + class BottleOrder < FormulaCop + extend AutoCorrector + + def audit_formula(_node, _class_node, _parent_class_node, body_node) + bottle_node = find_block(body_node, :bottle) + return if bottle_node.nil? + return if bottle_node.child_nodes.blank? + + non_sha256_nodes = [] + sha256_nodes = [] + + bottle_block_method_calls = if bottle_node.child_nodes.last.begin_type? + bottle_node.child_nodes.last.child_nodes + else + [bottle_node.child_nodes.last] + end + + bottle_block_method_calls.each do |node| + if node.method_name == :sha256 + sha256_nodes << node + else + non_sha256_nodes << node + end + end + + arm64_nodes = [] + intel_nodes = [] + + sha256_nodes.each do |node| + version = sha256_bottle_tag node + if version.to_s.start_with? "arm64" + arm64_nodes << node + else + intel_nodes << node + end + end + + return if sha256_order(sha256_nodes) == sha256_order(arm64_nodes + intel_nodes) + + offending_node(bottle_node) + problem "ARM bottles should be listed before Intel bottles" do |corrector| + lines = ["bottle do"] + lines += non_sha256_nodes.map { |node| " #{node.source}" } + lines += arm64_nodes.map { |node| " #{node.source}" } + lines += intel_nodes.map { |node| " #{node.source}" } + lines << " end" + corrector.replace(bottle_node.source_range, lines.join("\n")) + end + end + + def sha256_order(nodes) + nodes.map do |node| + sha256_bottle_tag node + end + end + + def sha256_bottle_tag(node) + hash_pair = node.last_argument.pairs.last + if hash_pair.key.sym_type? + hash_pair.key.value + else + hash_pair.value.value + end + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/shared/helper_functions.rb b/Library/Homebrew/rubocops/shared/helper_functions.rb index 9d5b3e581a..63858e3697 100644 --- a/Library/Homebrew/rubocops/shared/helper_functions.rb +++ b/Library/Homebrew/rubocops/shared/helper_functions.rb @@ -113,7 +113,12 @@ module RuboCop def find_method_calls_by_name(node, method_name) return if node.nil? - node.each_child_node(:send).select { |method_node| method_name == method_node.method_name } + nodes = node.each_child_node(:send).select { |method_node| method_name == method_node.method_name } + + # The top level node can be a method + nodes << node if node.send_type? && node.method_name == method_name + + nodes end # Returns an array of method call nodes matching method_name in every descendant of node. diff --git a/Library/Homebrew/test/rubocops/bottle/bottle_digest_indentation_spec.rb b/Library/Homebrew/test/rubocops/bottle/bottle_digest_indentation_spec.rb new file mode 100644 index 0000000000..01e5c3e5b3 --- /dev/null +++ b/Library/Homebrew/test/rubocops/bottle/bottle_digest_indentation_spec.rb @@ -0,0 +1,128 @@ +# typed: false +# frozen_string_literal: true + +require "rubocops/bottle" + +describe RuboCop::Cop::FormulaAudit::BottleDigestIndentation do + subject(:cop) { described_class.new } + + it "reports no offenses for `bottle :uneeded`" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle :unneeded + end + RUBY + end + + it "reports no offenses for properly aligned digests in `bottle` blocks without cellars" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 arm64_big_sur: "aaaaaaaa" + sha256 big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + sha256 arm64_big_sur: "aaaaaaaa" + end + end + RUBY + end + + it "reports no offenses for properly aligned tags in `bottle` blocks with cellars" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 cellar: :any, arm64_big_sur: "aaaaaaaa" + sha256 cellar: "/usr/local/Cellar", big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + sha256 cellar: :any, arm64_big_sur: "aaaaaaaa" + end + end + RUBY + end + + it "reports and corrects misaligned digests in `bottle` block" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 arm64_big_sur: "aaaaaaaa" + sha256 big_sur: "faceb00c" + ^^^^^^^^^^ Align bottle digests + sha256 catalina: "deadbeef" + ^^^^^^^^^^ Align bottle digests + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 arm64_big_sur: "aaaaaaaa" + sha256 big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + end + + it "reports and corrects misaligned digests in `bottle` block with cellars" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 cellar: :any, arm64_big_sur: "aaaaaaaa" + sha256 cellar: "/usr/local/Cellar", big_sur: "faceb00c" + ^^^^^^^^^^ Align bottle digests + sha256 catalina: "deadbeef" + ^^^^^^^^^^ Align bottle digests + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 cellar: :any, arm64_big_sur: "aaaaaaaa" + sha256 cellar: "/usr/local/Cellar", big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + end +end diff --git a/Library/Homebrew/test/rubocops/bottle/bottle_format_spec.rb b/Library/Homebrew/test/rubocops/bottle/bottle_format_spec.rb new file mode 100644 index 0000000000..2a080b0ab3 --- /dev/null +++ b/Library/Homebrew/test/rubocops/bottle/bottle_format_spec.rb @@ -0,0 +1,148 @@ +# typed: false +# frozen_string_literal: true + +require "rubocops/bottle" + +describe RuboCop::Cop::FormulaAudit::BottleFormat do + subject(:cop) { described_class.new } + + it "reports no offenses for `bottle :uneeded`" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle :unneeded + end + RUBY + end + + it "reports and corrects old `sha256` syntax in `bottle` block without cellars" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + sha256 "faceb00c" => :big_sur + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `sha256` should use new syntax + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + sha256 big_sur: "faceb00c" + end + end + RUBY + + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 "faceb00c" => :big_sur + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `sha256` should use new syntax + sha256 "deadbeef" => :catalina + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `sha256` should use new syntax + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + end + + it "reports and corrects old `sha256` syntax in `bottle` block without cellars" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + cellar :any + ^^^^^^^^^^^ `cellar` should be a parameter to `sha256` + rebuild 4 + sha256 "faceb00c" => :big_sur + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `sha256` should use new syntax + sha256 "deadbeef" => :catalina + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `sha256` should use new syntax + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 cellar: :any, big_sur: "faceb00c" + sha256 cellar: :any, catalina: "deadbeef" + end + end + RUBY + + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + cellar :any + ^^^^^^^^^^^ `cellar` should be a parameter to `sha256` + sha256 "faceb00c" => :big_sur + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `sha256` should use new syntax + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + sha256 cellar: :any, big_sur: "faceb00c" + end + end + RUBY + + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + cellar "/usr/local/Cellar" + ^^^^^^^^^^^^^^^^^^^^^^^^^^ `cellar` should be a parameter to `sha256` + rebuild 4 + sha256 "faceb00c" => :big_sur + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `sha256` should use new syntax + sha256 "deadbeef" => :catalina + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `sha256` should use new syntax + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 cellar: "/usr/local/Cellar", big_sur: "faceb00c" + sha256 cellar: "/usr/local/Cellar", catalina: "deadbeef" + end + end + RUBY + end +end diff --git a/Library/Homebrew/test/rubocops/bottle/bottle_order_spec.rb b/Library/Homebrew/test/rubocops/bottle/bottle_order_spec.rb new file mode 100644 index 0000000000..356a6ac48e --- /dev/null +++ b/Library/Homebrew/test/rubocops/bottle/bottle_order_spec.rb @@ -0,0 +1,240 @@ +# typed: false +# frozen_string_literal: true + +require "rubocops/bottle" + +describe RuboCop::Cop::FormulaAudit::BottleOrder do + subject(:cop) { described_class.new } + + it "reports no offenses for `bottle :uneeded`" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle :unneeded + end + RUBY + end + + it "reports no offenses for a properly ordered bottle block" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 arm64_something_else: "aaaaaaaa" + sha256 arm64_big_sur: "aaaaaaaa" + sha256 big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 cellar: :any, arm64_something_else: "aaaaaaaa" + sha256 cellar: :any_skip_relocation, arm64_big_sur: "aaaaaaaa" + sha256 cellar: "/usr/local/Cellar", big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + end + + it "reports no offenses for a properly ordered bottle block with a single bottle" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + sha256 big_sur: "faceb00c" + end + end + RUBY + + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + sha256 cellar: :any, big_sur: "faceb00c" + end + end + RUBY + end + + it "reports no offenses for a properly ordered bottle block with only arm/intel bottles" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 arm64_catalina: "aaaaaaaa" + sha256 arm64_big_sur: "aaaaaaaa" + end + end + RUBY + + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 arm64_big_sur: "aaaaaaaa" + end + end + RUBY + + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 big_sur: "faceb00c" + end + end + RUBY + end + + it "reports and corrects arm bottles below intel bottles" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + ^^^^^^^^^ ARM bottles should be listed before Intel bottles + rebuild 4 + sha256 big_sur: "faceb00c" + sha256 catalina: "deadbeef" + sha256 arm64_big_sur: "aaaaaaaa" + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 arm64_big_sur: "aaaaaaaa" + sha256 big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + end + + it "reports and corrects multiple arm bottles below intel bottles" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + ^^^^^^^^^ ARM bottles should be listed before Intel bottles + rebuild 4 + sha256 big_sur: "faceb00c" + sha256 arm64_catalina: "aaaaaaaa" + sha256 catalina: "deadbeef" + sha256 arm64_big_sur: "aaaaaaaa" + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 arm64_catalina: "aaaaaaaa" + sha256 arm64_big_sur: "aaaaaaaa" + sha256 big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + end + + it "reports and corrects arm bottles with cellars below intel bottles" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + ^^^^^^^^^ ARM bottles should be listed before Intel bottles + rebuild 4 + sha256 cellar: "/usr/local/Cellar", big_sur: "faceb00c" + sha256 catalina: "deadbeef" + sha256 cellar: :any, arm64_big_sur: "aaaaaaaa" + sha256 cellar: :any_skip_relocation, arm64_catalina: "aaaaaaaa" + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 cellar: :any, arm64_big_sur: "aaaaaaaa" + sha256 cellar: :any_skip_relocation, arm64_catalina: "aaaaaaaa" + sha256 cellar: "/usr/local/Cellar", big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + end + + it "reports and corrects arm bottles below intel bottles with old bottle syntax" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + ^^^^^^^^^ ARM bottles should be listed before Intel bottles + cellar :any + sha256 "faceb00c" => :big_sur + sha256 "aaaaaaaa" => :arm64_big_sur + sha256 "aaaaaaaa" => :arm64_catalina + sha256 "deadbeef" => :catalina + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + cellar :any + sha256 "aaaaaaaa" => :arm64_big_sur + sha256 "aaaaaaaa" => :arm64_catalina + sha256 "faceb00c" => :big_sur + sha256 "deadbeef" => :catalina + end + end + RUBY + end +end diff --git a/Library/Homebrew/test/rubocops/bottle/bottle_tag_indentation_spec.rb b/Library/Homebrew/test/rubocops/bottle/bottle_tag_indentation_spec.rb new file mode 100644 index 0000000000..c094c8d48e --- /dev/null +++ b/Library/Homebrew/test/rubocops/bottle/bottle_tag_indentation_spec.rb @@ -0,0 +1,98 @@ +# typed: false +# frozen_string_literal: true + +require "rubocops/bottle" + +describe RuboCop::Cop::FormulaAudit::BottleTagIndentation do + subject(:cop) { described_class.new } + + it "reports no offenses for `bottle :uneeded`" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle :unneeded + end + RUBY + end + + it "reports no offenses for `bottle` blocks without cellars" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 arm64_big_sur: "aaaaaaaa" + sha256 big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + sha256 big_sur: "faceb00c" + end + end + RUBY + end + + it "reports no offenses for properly aligned tags in `bottle` blocks with cellars" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 cellar: :any, arm64_big_sur: "aaaaaaaa" + sha256 cellar: "/usr/local/Cellar", big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + sha256 cellar: :any, arm64_big_sur: "aaaaaaaa" + end + end + RUBY + end + + it "reports and corrects misaligned tags in `bottle` block with cellars" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 cellar: :any, arm64_big_sur: "aaaaaaaa" + ^^^^^^^^^^^^^^^^^^^^^^^^^ Align bottle tags + sha256 cellar: "/usr/local/Cellar", big_sur: "faceb00c" + sha256 catalina: "deadbeef" + ^^^^^^^^^^^^^^^^^^^^ Align bottle tags + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + bottle do + rebuild 4 + sha256 cellar: :any, arm64_big_sur: "aaaaaaaa" + sha256 cellar: "/usr/local/Cellar", big_sur: "faceb00c" + sha256 catalina: "deadbeef" + end + end + RUBY + end +end From 727ac9b47f0b0726e1bc18bf5d2d4aa7817b1a14 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Tue, 2 Feb 2021 15:09:39 -0500 Subject: [PATCH 2/3] bottle: write bottles with new syntax --- Library/Homebrew/dev-cmd/bottle.rb | 40 +++- Library/Homebrew/test/dev-cmd/bottle_spec.rb | 190 +++++++++++++++++-- 2 files changed, 208 insertions(+), 22 deletions(-) diff --git a/Library/Homebrew/dev-cmd/bottle.rb b/Library/Homebrew/dev-cmd/bottle.rb index 03cae3a60d..5d4af842d9 100644 --- a/Library/Homebrew/dev-cmd/bottle.rb +++ b/Library/Homebrew/dev-cmd/bottle.rb @@ -194,24 +194,50 @@ module Homebrew !absolute_symlinks_start_with_string.empty? end - def generate_sha256_line(tag, digest, cellar) + def cellar_parameter_needed?(cellar) default_cellars = [ Homebrew::DEFAULT_MACOS_CELLAR, Homebrew::DEFAULT_MACOS_ARM_CELLAR, Homebrew::DEFAULT_LINUX_CELLAR, ] + cellar.present? && default_cellars.exclude?(cellar) + end + + def generate_sha256_line(tag, digest, cellar, tag_column, digest_column) + line = "sha256 " + tag_column += line.length + digest_column += line.length if cellar.is_a?(Symbol) - %Q(sha256 cellar: :#{cellar}, #{tag}: "#{digest}") - elsif cellar.present? && default_cellars.exclude?(cellar) - %Q(sha256 cellar: "#{cellar}", #{tag}: "#{digest}") - else - %Q(sha256 #{tag}: "#{digest}") + line += "cellar: :#{cellar}," + elsif cellar_parameter_needed?(cellar) + line += %Q(cellar: "#{cellar}",) end + line += " " * (tag_column - line.length) + line += "#{tag}:" + line += " " * (digest_column - line.length) + %Q(#{line}"#{digest}") end def bottle_output(bottle) + cellars = bottle.checksums.map do |checksum| + cellar = checksum["cellar"] + next unless cellar_parameter_needed? cellar + + case cellar + when String + %Q("#{cellar}") + when Symbol + ":#{cellar}" + end + end.compact + tag_column = cellars.empty? ? 0 : "cellar: #{cellars.max_by(&:length)}, ".length + + tags = bottle.checksums.map { |checksum| checksum["tag"] } + # Start where the tag ends, add the max length of the tag, add two for the `: ` + digest_column = tag_column + tags.max_by(&:length).length + 2 + sha256_lines = bottle.checksums.map do |checksum| - generate_sha256_line(checksum["tag"], checksum["digest"], checksum["cellar"]) + generate_sha256_line(checksum["tag"], checksum["digest"], checksum["cellar"], tag_column, digest_column) end erb_binding = bottle.instance_eval { binding } erb_binding.local_variable_set(:sha256_lines, sha256_lines) diff --git a/Library/Homebrew/test/dev-cmd/bottle_spec.rb b/Library/Homebrew/test/dev-cmd/bottle_spec.rb index 3aa399ddb1..9b0c8a523b 100644 --- a/Library/Homebrew/test/dev-cmd/bottle_spec.rb +++ b/Library/Homebrew/test/dev-cmd/bottle_spec.rb @@ -96,7 +96,7 @@ describe "brew bottle" do ==> testball bottle do root_url "#{HOMEBREW_BOTTLE_DEFAULT_DOMAIN}" - sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" + sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" sha256 cellar: :any_skip_relocation, catalina: "5334dd344986e46b2aa4f0471cac7b0914bd7de7cb890a34415771788d03f2ac" end EOS @@ -110,7 +110,7 @@ describe "brew bottle" do bottle do root_url "#{HOMEBREW_BOTTLE_DEFAULT_DOMAIN}" - sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" + sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" sha256 cellar: :any_skip_relocation, catalina: "5334dd344986e46b2aa4f0471cac7b0914bd7de7cb890a34415771788d03f2ac" end @@ -133,7 +133,7 @@ describe "brew bottle" do EOS end - it "replaces the bottle block in a formula that already has a bottle block" do + it "replaces the bottle block in a formula that already has a bottle block in the old format" do core_tap.path.cd do system "git", "init" setup_test_formula "testball", bottle_block: <<~EOS @@ -159,7 +159,7 @@ describe "brew bottle" do ==> testball bottle do root_url "#{HOMEBREW_BOTTLE_DEFAULT_DOMAIN}" - sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" + sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" sha256 cellar: :any_skip_relocation, catalina: "5334dd344986e46b2aa4f0471cac7b0914bd7de7cb890a34415771788d03f2ac" end EOS @@ -175,7 +175,69 @@ describe "brew bottle" do bottle do root_url "#{HOMEBREW_BOTTLE_DEFAULT_DOMAIN}" - sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" + sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" + sha256 cellar: :any_skip_relocation, catalina: "5334dd344986e46b2aa4f0471cac7b0914bd7de7cb890a34415771788d03f2ac" + end + + def install + (prefix/"foo"/"test").write("test") if build.with? "foo" + prefix.install Dir["*"] + (buildpath/"test.c").write \ + "#include \\nint main(){printf(\\"test\\");return 0;}" + bin.mkpath + system ENV.cc, "test.c", "-o", bin/"test" + end + + + + # something here + + end + EOS + end + + it "replaces the bottle block in a formula that already has a bottle block" do + core_tap.path.cd do + system "git", "init" + setup_test_formula "testball", bottle_block: <<~EOS + + bottle do + root_url "#{HOMEBREW_BOTTLE_DEFAULT_DOMAIN}" + sha256 cellar: :any_skip_relocation, big_sur: "6b276491297d4052538bd2fd22d5129389f27d90a98f831987236a5b90511b98" + sha256 cellar: :any_skip_relocation, catalina: "16cf230afdfcb6306c208d169549cf8773c831c8653d2c852315a048960d7e72" + end + EOS + system "git", "add", "--all" + system "git", "commit", "-m", "testball 0.1" + end + + expect { + brew "bottle", + "--merge", + "--write", + "#{TEST_TMPDIR}/testball-1.0.big_sur.bottle.json", + "#{TEST_TMPDIR}/testball-1.0.catalina.bottle.json" + }.to output(<<~EOS).to_stdout + ==> testball + bottle do + root_url "#{HOMEBREW_BOTTLE_DEFAULT_DOMAIN}" + sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" + sha256 cellar: :any_skip_relocation, catalina: "5334dd344986e46b2aa4f0471cac7b0914bd7de7cb890a34415771788d03f2ac" + end + EOS + + expect((core_tap.path/"Formula/testball.rb").read).to eq <<~EOS + class Testball < Formula + desc "Some test" + homepage "https://brew.sh/testball" + url "file://#{tarball}" + sha256 "#{tarball.sha256}" + + option "with-foo", "Build with foo" + + bottle do + root_url "#{HOMEBREW_BOTTLE_DEFAULT_DOMAIN}" + sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" sha256 cellar: :any_skip_relocation, catalina: "5334dd344986e46b2aa4f0471cac7b0914bd7de7cb890a34415771788d03f2ac" end @@ -214,7 +276,7 @@ describe "brew bottle" do }.to output("Error: `--keep-old` was passed but there was no existing bottle block!\n").to_stderr end - it "updates the bottle block in a formula that already has a bottle block when using --keep-old" do + it "updates the bottle block in a formula that already has a bottle block (old format) when using --keep-old" do core_tap.path.cd do system "git", "init" setup_test_formula "testball", bottle_block: <<~EOS @@ -240,9 +302,9 @@ describe "brew bottle" do ==> testball bottle do root_url "#{HOMEBREW_BOTTLE_DEFAULT_DOMAIN}" - sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" - sha256 cellar: :any_skip_relocation, catalina: "5334dd344986e46b2aa4f0471cac7b0914bd7de7cb890a34415771788d03f2ac" - sha256 cellar: :any, high_sierra: "6971b6eebf4c00eaaed72a1104a49be63861eabc95d679a0c84040398e320059" + sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" + sha256 cellar: :any_skip_relocation, catalina: "5334dd344986e46b2aa4f0471cac7b0914bd7de7cb890a34415771788d03f2ac" + sha256 cellar: :any, high_sierra: "6971b6eebf4c00eaaed72a1104a49be63861eabc95d679a0c84040398e320059" end EOS @@ -257,10 +319,74 @@ describe "brew bottle" do bottle do root_url "#{HOMEBREW_BOTTLE_DEFAULT_DOMAIN}" - sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" - sha256 cellar: :any_skip_relocation, catalina: "5334dd344986e46b2aa4f0471cac7b0914bd7de7cb890a34415771788d03f2ac" + sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" + sha256 cellar: :any_skip_relocation, catalina: "5334dd344986e46b2aa4f0471cac7b0914bd7de7cb890a34415771788d03f2ac" + sha256 cellar: :any, high_sierra: "6971b6eebf4c00eaaed72a1104a49be63861eabc95d679a0c84040398e320059" + end + + def install + (prefix/"foo"/"test").write("test") if build.with? "foo" + prefix.install Dir["*"] + (buildpath/"test.c").write \ + "#include \\nint main(){printf(\\"test\\");return 0;}" + bin.mkpath + system ENV.cc, "test.c", "-o", bin/"test" + end + + + + # something here + + end + EOS + end + + it "updates the bottle block in a formula that already has a bottle block when using --keep-old" do + core_tap.path.cd do + system "git", "init" + setup_test_formula "testball", bottle_block: <<~EOS + + bottle do + root_url "#{HOMEBREW_BOTTLE_DEFAULT_DOMAIN}" sha256 cellar: :any, high_sierra: "6971b6eebf4c00eaaed72a1104a49be63861eabc95d679a0c84040398e320059" end + EOS + system "git", "add", "--all" + system "git", "commit", "-m", "testball 0.1" + end + + expect { + brew "bottle", + "--merge", + "--write", + "--keep-old", + "#{TEST_TMPDIR}/testball-1.0.big_sur.bottle.json", + "#{TEST_TMPDIR}/testball-1.0.catalina.bottle.json" + }.to output(<<~EOS).to_stdout + ==> testball + bottle do + root_url "#{HOMEBREW_BOTTLE_DEFAULT_DOMAIN}" + sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" + sha256 cellar: :any_skip_relocation, catalina: "5334dd344986e46b2aa4f0471cac7b0914bd7de7cb890a34415771788d03f2ac" + sha256 cellar: :any, high_sierra: "6971b6eebf4c00eaaed72a1104a49be63861eabc95d679a0c84040398e320059" + end + EOS + + expect((core_tap.path/"Formula/testball.rb").read).to eq <<~EOS + class Testball < Formula + desc "Some test" + homepage "https://brew.sh/testball" + url "file://#{tarball}" + sha256 "#{tarball.sha256}" + + option "with-foo", "Build with foo" + + bottle do + root_url "#{HOMEBREW_BOTTLE_DEFAULT_DOMAIN}" + sha256 cellar: :any_skip_relocation, big_sur: "a0af7dcbb5c83f6f3f7ecd507c2d352c1a018f894d51ad241ce8492fa598010f" + sha256 cellar: :any_skip_relocation, catalina: "5334dd344986e46b2aa4f0471cac7b0914bd7de7cb890a34415771788d03f2ac" + sha256 cellar: :any, high_sierra: "6971b6eebf4c00eaaed72a1104a49be63861eabc95d679a0c84040398e320059" + end def install (prefix/"foo"/"test").write("test") if build.with? "foo" @@ -438,7 +564,7 @@ describe "brew bottle" do describe "::generate_sha256_line" do it "generates a string without cellar" do - expect(homebrew.generate_sha256_line(:catalina, "deadbeef", nil)).to eq( + expect(homebrew.generate_sha256_line(:catalina, "deadbeef", nil, 0, 10)).to eq( <<~RUBY.chomp, sha256 catalina: "deadbeef" RUBY @@ -446,7 +572,7 @@ describe "brew bottle" do end it "generates a string with cellar symbol" do - expect(homebrew.generate_sha256_line(:catalina, "deadbeef", :any)).to eq( + expect(homebrew.generate_sha256_line(:catalina, "deadbeef", :any, 14, 24)).to eq( <<~RUBY.chomp, sha256 cellar: :any, catalina: "deadbeef" RUBY @@ -454,7 +580,7 @@ describe "brew bottle" do end it "generates a string with default cellar path" do - expect(homebrew.generate_sha256_line(:catalina, "deadbeef", Homebrew::DEFAULT_LINUX_CELLAR)).to eq( + expect(homebrew.generate_sha256_line(:catalina, "deadbeef", Homebrew::DEFAULT_LINUX_CELLAR, 0, 10)).to eq( <<~RUBY.chomp, sha256 catalina: "deadbeef" RUBY @@ -462,12 +588,46 @@ describe "brew bottle" do end it "generates a string with non-default cellar path" do - expect(homebrew.generate_sha256_line(:catalina, "deadbeef", "/home/test")).to eq( + expect(homebrew.generate_sha256_line(:catalina, "deadbeef", "/home/test", 22, 32)).to eq( <<~RUBY.chomp, sha256 cellar: "/home/test", catalina: "deadbeef" RUBY ) end + + context "with offsets" do + it "generates a string without cellar" do + expect(homebrew.generate_sha256_line(:catalina, "deadbeef", nil, 0, 15)).to eq( + <<~RUBY.chomp, + sha256 catalina: "deadbeef" + RUBY + ) + end + + it "generates a string with cellar symbol" do + expect(homebrew.generate_sha256_line(:catalina, "deadbeef", :any, 20, 35)).to eq( + <<~RUBY.chomp, + sha256 cellar: :any, catalina: "deadbeef" + RUBY + ) + end + + it "generates a string with default cellar path" do + expect(homebrew.generate_sha256_line(:catalina, "deadbeef", Homebrew::DEFAULT_LINUX_CELLAR, 14, 30)).to eq( + <<~RUBY.chomp, + sha256 catalina: "deadbeef" + RUBY + ) + end + + it "generates a string with non-default cellar path" do + expect(homebrew.generate_sha256_line(:catalina, "deadbeef", "/home/test", 25, 36)).to eq( + <<~RUBY.chomp, + sha256 cellar: "/home/test", catalina: "deadbeef" + RUBY + ) + end + end end end end From 6e3011a8cc639980fc90c8f05d29fc8c4499f959 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Tue, 2 Feb 2021 15:09:48 -0500 Subject: [PATCH 3/3] docs: update bottle documentation --- docs/Bottles.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/Bottles.md b/docs/Bottles.md index c7de6e026a..642a39dc29 100644 --- a/docs/Bottles.md +++ b/docs/Bottles.md @@ -22,9 +22,10 @@ A simple (and typical) example: ```ruby bottle do - sha256 "4921af80137af9cc3d38fd17c9120da882448a090b0a8a3a19af3199b415bfca" => :sierra - sha256 "c71db15326ee9196cd98602e38d0b7fb2b818cdd48eede4ee8eb827d809e09ba" => :el_capitan - sha256 "85cc828a96735bdafcf29eb6291ca91bac846579bcef7308536e0c875d6c81d7" => :yosemite + sha256 arm64_big_sur: "a9ae578b05c3da46cedc07dd428d94a856aeae7f3ef80a0f405bf89b8cde893a" + sha256 big_sur: "5dc376aa20241233b76e2ec2c1d4e862443a0250916b2838a1ff871e8a6dc2c5" + sha256 catalina: "924afbbc16549d8c2b80544fd03104ff8c17a4b1460238e3ed17a1313391a2af" + sha256 mojave: "678d338adc7d6e8c352800fe03fc56660c796bd6da23eda2b1411fed18bd0d8d" end ``` @@ -33,11 +34,11 @@ A full example: ```ruby bottle do root_url "https://example.com" - cellar "/opt/homebrew/Cellar" rebuild 4 - sha256 "4921af80137af9cc3d38fd17c9120da882448a090b0a8a3a19af3199b415bfca" => :sierra - sha256 "c71db15326ee9196cd98602e38d0b7fb2b818cdd48eede4ee8eb827d809e09ba" => :el_capitan - sha256 "85cc828a96735bdafcf29eb6291ca91bac846579bcef7308536e0c875d6c81d7" => :yosemite + sha256 cellar: "/opt/homebrew/Cellar", arm64_big_sur: "a9ae578b05c3da46cedc07dd428d94a856aeae7f3ef80a0f405bf89b8cde893a" + sha256 cellar: :any, big_sur: "5dc376aa20241233b76e2ec2c1d4e862443a0250916b2838a1ff871e8a6dc2c5" + sha256 catalina: "924afbbc16549d8c2b80544fd03104ff8c17a4b1460238e3ed17a1313391a2af" + sha256 mojave: "678d338adc7d6e8c352800fe03fc56660c796bd6da23eda2b1411fed18bd0d8d" end ```