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 123796b456..517070f5ba 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -136,10 +136,9 @@ on_request: true) 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 63e5fdee88..7451f2124f 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -308,7 +308,7 @@ module Homebrew 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 bd8bc73a5c..a44279eced 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -170,7 +170,7 @@ module Homebrew 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 be12eb049c..54398793e7 100644 --- a/Library/Homebrew/diagnostic.rb +++ b/Library/Homebrew/diagnostic.rb @@ -925,7 +925,7 @@ module Homebrew 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 8696c5cc6b..40e489cf8f 100644 --- a/Library/Homebrew/extend/kernel.rb +++ b/Library/Homebrew/extend/kernel.rb @@ -64,6 +64,7 @@ module Kernel 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 @@ module Kernel 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 @@ module Kernel 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 @@ module Kernel 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 6733fc0b34..923fd1dae5 100644 --- a/Library/Homebrew/extend/os/mac/diagnostic.rb +++ b/Library/Homebrew/extend/os/mac/diagnostic.rb @@ -134,7 +134,7 @@ module Homebrew # `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 @@ module Homebrew # `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 b26c77ca3a..c194290abb 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -206,10 +206,9 @@ class FormulaInstaller 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 @@ on_request: installed_on_request?, options:) 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 066438b32e..52c14b638f 100644 --- a/Library/Homebrew/style.rb +++ b/Library/Homebrew/style.rb @@ -15,7 +15,7 @@ module Homebrew 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 200756b4cf..238f4573e3 100644 --- a/Library/Homebrew/test/extend/kernel_spec.rb +++ b/Library/Homebrew/test/extend/kernel_spec.rb @@ -230,12 +230,12 @@ RSpec.describe Kernel do 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 9e20bef10b..5ddb61ec74 100644 --- a/Library/Homebrew/utils/github/actions.rb +++ b/Library/Homebrew/utils/github/actions.rb @@ -35,6 +35,25 @@ module GitHub 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 @@ module GitHub 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 @@ module GitHub 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 @@ module GitHub 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 @@ module GitHub # 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