From f3a73099d6993842e6edb031adfb9dfc946fd126 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Wed, 8 May 2024 16:59:50 +0100 Subject: [PATCH] extend/kernel: make `opoo`/`odie`/etc. print GitHub Actions notes. We already do this for deprecations but these may make warnings and errors from Homebrew easier to spot in GitHub Actions logs. While we're here, cleanup other cases that should have used `GitHub::Actions::Annotation` but didn't and provide some helpers and tweaks there necessary for our use case here. --- Library/Homebrew/cask/installer.rb | 3 +- Library/Homebrew/dev-cmd/audit.rb | 2 +- Library/Homebrew/dev-cmd/pr-pull.rb | 2 +- Library/Homebrew/dev-cmd/tests.rb | 3 ++ Library/Homebrew/diagnostic.rb | 2 +- Library/Homebrew/extend/kernel.rb | 11 ++++++-- Library/Homebrew/extend/os/mac/diagnostic.rb | 4 +-- Library/Homebrew/formula_installer.rb | 5 ++-- Library/Homebrew/style.rb | 2 +- Library/Homebrew/utils/github/actions.rb | 29 +++++++++++++------- 10 files changed, 39 insertions(+), 24 deletions(-) diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index 123796b4564c83..9e6e1265bd023e 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -136,10 +136,9 @@ def check_deprecate_disable case deprecate_disable_type when :deprecated - puts "::warning::#{message_full}" if ENV["GITHUB_ACTIONS"] opoo message_full when :disabled - puts "::error::#{message_full}" if ENV["GITHUB_ACTIONS"] + puts GitHub::Actions::Annotation.new(:error, message) if GitHub::Actions.env_set? raise CaskCannotBeInstalledError.new(@cask, message) end end diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 63e5fdee88d86f..7451f2124f5470 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -308,7 +308,7 @@ def run ofail "#{errors_summary}." end - return unless ENV["GITHUB_ACTIONS"] + return unless GitHub::Actions.env_set? annotations = formula_problems.merge(cask_problems).flat_map do |(_, path), problems| problems.map do |problem| diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index bd8bc73a5c3bf7..a44279eced5792 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -170,7 +170,7 @@ def run safe_system HOMEBREW_BREW_FILE, *upload_args end ensure - if args.retain_bottle_dir? && ENV["GITHUB_ACTIONS"] + if args.retain_bottle_dir? && GitHub::Actions.env_set? ohai "Bottle files retained at:", dir File.open(ENV.fetch("GITHUB_OUTPUT"), "a") do |f| f.puts "bottle_path=#{dir}" diff --git a/Library/Homebrew/dev-cmd/tests.rb b/Library/Homebrew/dev-cmd/tests.rb index 60845d6cc2e659..76ff28e5784628 100644 --- a/Library/Homebrew/dev-cmd/tests.rb +++ b/Library/Homebrew/dev-cmd/tests.rb @@ -246,6 +246,9 @@ def setup_environment! ENV["HOMEBREW_TEST_ONLINE"] = "1" if args.online? ENV["HOMEBREW_SORBET_RUNTIME"] = "1" + # We don't want tests to change behaviour based on where they are run. + ENV.delete("GITHUB_ACTIONS") + # TODO: remove this and fix tests when possible. ENV["HOMEBREW_NO_INSTALL_FROM_API"] = "1" ENV.delete("HOMEBREW_INTERNAL_JSON_V3") diff --git a/Library/Homebrew/diagnostic.rb b/Library/Homebrew/diagnostic.rb index be12eb049c0a62..54398793e71044 100644 --- a/Library/Homebrew/diagnostic.rb +++ b/Library/Homebrew/diagnostic.rb @@ -925,7 +925,7 @@ def check_cask_install_location def check_cask_staging_location # Skip this check when running CI since the staging path is not writable for security reasons - return if ENV["GITHUB_ACTIONS"] + return if GitHub::Actions.env_set? path = Cask::Caskroom.path diff --git a/Library/Homebrew/extend/kernel.rb b/Library/Homebrew/extend/kernel.rb index 8696c5cc6baa83..436370494722b4 100644 --- a/Library/Homebrew/extend/kernel.rb +++ b/Library/Homebrew/extend/kernel.rb @@ -64,6 +64,7 @@ def oh1(title, truncate: :auto) def opoo(message) Tty.with($stderr) do |stderr| stderr.puts Formatter.warning(message, label: "Warning") + puts GitHub::Actions::Annotation.new(:warning, message) if GitHub::Actions.env_set? end end @@ -73,6 +74,7 @@ def opoo(message) def onoe(message) Tty.with($stderr) do |stderr| stderr.puts Formatter.error(message, label: "Error") + puts GitHub::Actions::Annotation.new(:error, message) if GitHub::Actions.env_set? end end @@ -146,11 +148,14 @@ def odeprecated(method, replacement = nil, next unless (match = line.match(HOMEBREW_TAP_PATH_REGEX)) tap = Tap.fetch(match[:user], match[:repo]) - tap_message = +"\nPlease report this issue to the #{tap} tap (not Homebrew/brew or Homebrew/homebrew-core)" + tap_message = +"\nPlease report this issue to the #{tap.full_name} tap" + tap_message += " (not Homebrew/brew or Homebrew/homebrew-core)" unless tap.official? tap_message += ", or even better, submit a PR to fix it" if replacement tap_message << ":\n #{line.sub(/^(.*:\d+):.*$/, '\1')}\n\n" break end + file, line, = backtrace.first.split(":") + line = line.to_i if line.present? message = +"Calling #{method} is #{verb}! #{replacement_message}" message << tap_message if tap_message @@ -158,12 +163,12 @@ def odeprecated(method, replacement = nil, disable = true if disable_for_developers && Homebrew::EnvConfig.developer? if disable || Homebrew.raise_deprecation_exceptions? - puts "::error::#{message}" if ENV["GITHUB_ACTIONS"] + puts GitHub::Actions::Annotation.new(:error, message, file:, line:) if GitHub::Actions.env_set? exception = MethodDeprecatedError.new(message) exception.set_backtrace(backtrace) raise exception elsif !Homebrew.auditing? - puts "::warning::#{message}" if ENV["GITHUB_ACTIONS"] + puts GitHub::Actions::Annotation.new(:warning, message, file:, line:) if GitHub::Actions.env_set? opoo message end end diff --git a/Library/Homebrew/extend/os/mac/diagnostic.rb b/Library/Homebrew/extend/os/mac/diagnostic.rb index 6733fc0b34b229..923fd1dae5dae2 100644 --- a/Library/Homebrew/extend/os/mac/diagnostic.rb +++ b/Library/Homebrew/extend/os/mac/diagnostic.rb @@ -134,7 +134,7 @@ def check_xcode_up_to_date # `brew test-bot` runs `brew doctor` in the CI for the Homebrew/brew # repository. This only needs to support whatever CI providers # Homebrew/brew is currently using. - return if ENV["GITHUB_ACTIONS"] + return if GitHub::Actions.env_set? # With fake El Capitan for Portable Ruby, we are intentionally not using Xcode 8. # This is because we are not using the CLT and Xcode 8 has the 10.12 SDK. @@ -165,7 +165,7 @@ def check_clt_up_to_date # `brew test-bot` runs `brew doctor` in the CI for the Homebrew/brew # repository. This only needs to support whatever CI providers # Homebrew/brew is currently using. - return if ENV["GITHUB_ACTIONS"] + return if GitHub::Actions.env_set? <<~EOS A newer Command Line Tools release is available. diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 1475a595ff5544..97696ad47b9546 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -205,10 +205,9 @@ def prelude case deprecate_disable_type when :deprecated - puts "::warning::#{message}" if ENV["GITHUB_ACTIONS"] opoo message when :disabled - puts "::error::#{message}" if ENV["GITHUB_ACTIONS"] + puts GitHub::Actions::Annotation.new(:error, message) if GitHub::Actions.env_set? raise CannotInstallFormulaError, message end end @@ -504,7 +503,7 @@ def check_conflicts raise if Homebrew::EnvConfig.developer? - $stderr.puts "Please report this issue to the #{formula.tap} tap (not Homebrew/brew or Homebrew/homebrew-core)!" + $stderr.puts "Please report this issue to the #{formula.tap.full_name} tap (not Homebrew/brew or Homebrew/homebrew-core)!" false else f.linked_keg.exist? && f.opt_prefix.exist? diff --git a/Library/Homebrew/style.rb b/Library/Homebrew/style.rb index 066438b32e523a..52c14b638f9e38 100644 --- a/Library/Homebrew/style.rb +++ b/Library/Homebrew/style.rb @@ -15,7 +15,7 @@ module Style def self.check_style_and_print(files, **options) success = check_style_impl(files, :print, **options) - if ENV["GITHUB_ACTIONS"] && !success + if GitHub::Actions.env_set? && !success check_style_json(files, **options).each do |path, offenses| offenses.each do |o| line = o.location.line diff --git a/Library/Homebrew/utils/github/actions.rb b/Library/Homebrew/utils/github/actions.rb index 9e20bef10bfedb..2addec5d78ded1 100644 --- a/Library/Homebrew/utils/github/actions.rb +++ b/Library/Homebrew/utils/github/actions.rb @@ -35,6 +35,11 @@ def self.format_multiline_string(name, value) EOS end + sig { returns(T::Boolean) } + def self.env_set? + ENV.fetch("GITHUB_ACTIONS", false).present? + end + # Helper class for formatting annotations on GitHub Actions. class Annotation ANNOTATION_TYPES = [:notice, :warning, :error].freeze @@ -52,7 +57,7 @@ def self.path_relative_to_workspace(path) params( type: Symbol, message: String, - file: T.any(String, Pathname), + file: T.nilable(T.any(String, Pathname)), title: T.nilable(String), line: T.nilable(Integer), end_line: T.nilable(Integer), @@ -60,12 +65,12 @@ def self.path_relative_to_workspace(path) end_column: T.nilable(Integer), ).void } - def initialize(type, message, file:, title: nil, line: nil, end_line: nil, column: nil, end_column: nil) + def initialize(type, message, file: nil, title: nil, line: nil, end_line: nil, column: nil, end_column: nil) raise ArgumentError, "Unsupported type: #{type.inspect}" if ANNOTATION_TYPES.exclude?(type) @type = type @message = Tty.strip_ansi(message) - @file = self.class.path_relative_to_workspace(file) + @file = self.class.path_relative_to_workspace(file) if file.present? @title = Tty.strip_ansi(title) if title @line = Integer(line) if line @end_line = Integer(end_line) if end_line @@ -76,15 +81,17 @@ def initialize(type, message, file:, title: nil, line: nil, end_line: nil, colum sig { returns(String) } def to_s metadata = @type.to_s - metadata << " file=#{Actions.escape(@file.to_s)}" + if @file + metadata << " file=#{Actions.escape(@file.to_s)}" - if @line - metadata << ",line=#{@line}" - metadata << ",endLine=#{@end_line}" if @end_line + if @line + metadata << ",line=#{@line}" + metadata << ",endLine=#{@end_line}" if @end_line - if @column - metadata << ",col=#{@column}" - metadata << ",endColumn=#{@end_column}" if @end_column + if @column + metadata << ",col=#{@column}" + metadata << ",endColumn=#{@end_column}" if @end_column + end end end @@ -97,6 +104,8 @@ def to_s # the `GITHUB_WORKSPACE` directory or if no `file` is specified. sig { returns(T::Boolean) } def relevant? + return true if file.blank? + @file.descend.next.to_s != ".." end end