From 16901a674f09d6b29dcfe3d178cb34bd171a8012 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 123796b4564c8..517070f5ba919 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 63e5fdee88d86..7451f2124f547 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 bd8bc73a5c3bf..a44279eced579 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 be12eb049c0a6..54398793e7104 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 8696c5cc6baa8..40e489cf8f274 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 6733fc0b34b22..923fd1dae5dae 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 b26c77ca3a3ce..c194290abb870 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -206,10 +206,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 @@ -505,7 +504,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 066438b32e523..52c14b638f9e3 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 200756b4cf980..238f4573e33a5 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 9e20bef10bfed..5ddb61ec74ae9 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? + 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) + # Don't print annotations during tests, too messy to handle these. + return if ENV.fetch("HOMEBREW_TESTS", false) + + 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