From 27a5558e77acf349503e868cea0018ce9b17720c Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Thu, 9 May 2024 13:19:14 +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/diagnostic.rb | 2 +- Library/Homebrew/extend/kernel.rb | 12 ++++-- Library/Homebrew/extend/os/mac/diagnostic.rb | 4 +- Library/Homebrew/formula_installer.rb | 6 +-- Library/Homebrew/style.rb | 2 +- Library/Homebrew/test/extend/kernel_spec.rb | 4 +- Library/Homebrew/utils/github/actions.rb | 43 +++++++++++++++----- 10 files changed, 54 insertions(+), 26 deletions(-) diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index 123796b4564c83..517070f5ba9192 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"] + GitHub::Actions.puts_annotation_if_env_set(:error, message) 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/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..40e489cf8f274e 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") + GitHub::Actions.puts_annotation_if_env_set(:warning, message) end end @@ -73,6 +74,7 @@ def opoo(message) def onoe(message) Tty.with($stderr) do |stderr| stderr.puts Formatter.error(message, label: "Error") + GitHub::Actions.puts_annotation_if_env_set(:error, message) 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,13 @@ 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? + GitHub::Actions.puts_annotation_if_env_set(:error, message, file:, line:) exception = MethodDeprecatedError.new(message) exception.set_backtrace(backtrace) raise exception elsif !Homebrew.auditing? - puts "::warning::#{message}" if ENV["GITHUB_ACTIONS"] + GitHub::Actions.puts_annotation_if_env_set(:warning, message, file:, line:) 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..fe9c047dde8365 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"] + GitHub::Actions.puts_annotation_if_env_set(:error, message) raise CannotInstallFormulaError, message end end @@ -504,7 +503,8 @@ 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".squeeze(" ") + $stderr.puts " (not Homebrew/brew or Homebrew/homebrew-core)!" unless formula.core_formula? 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/test/extend/kernel_spec.rb b/Library/Homebrew/test/extend/kernel_spec.rb index 200756b4cf9801..238f4573e33a5f 100644 --- a/Library/Homebrew/test/extend/kernel_spec.rb +++ b/Library/Homebrew/test/extend/kernel_spec.rb @@ -230,12 +230,12 @@ def esc(code) expect do odeprecated( "method", "replacement", - caller: ["#{HOMEBREW_LIBRARY}/Taps/homebrew/homebrew-core/"], + caller: ["#{HOMEBREW_LIBRARY}/Taps/playbrew/homebrew-play/"], disable: true ) end.to raise_error( MethodDeprecatedError, - %r{method.*replacement.*homebrew/core.*/Taps/homebrew/homebrew-core/}m, + %r{method.*replacement.*playbrew/homebrew-play.*/Taps/playbrew/homebrew-play/}m, ) end end diff --git a/Library/Homebrew/utils/github/actions.rb b/Library/Homebrew/utils/github/actions.rb index 9e20bef10bfedb..6306adbba03181 100644 --- a/Library/Homebrew/utils/github/actions.rb +++ b/Library/Homebrew/utils/github/actions.rb @@ -35,6 +35,25 @@ def self.format_multiline_string(name, value) EOS end + sig { returns(T::Boolean) } + def self.env_set? + # Don't print annotations during tests because it's too messy to handle. + return false if ENV.fetch("HOMEBREW_TESTS", false) + + ENV.fetch("GITHUB_ACTIONS", false).present? + end + + sig { + params( + type: Symbol, message: String, + file: T.nilable(T.any(String, Pathname)), + line: T.nilable(Integer) + ).void + } + def self.puts_annotation_if_env_set(type, message, file: nil, line: nil) + puts Annotation.new(type, message) if env_set? + end + # Helper class for formatting annotations on GitHub Actions. class Annotation ANNOTATION_TYPES = [:notice, :warning, :error].freeze @@ -52,7 +71,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 +79,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 +95,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 +118,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