Skip to content

Commit

Permalink
extend/kernel: make opoo/odie/etc. print GitHub Actions notes.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
MikeMcQuaid committed May 9, 2024
1 parent 6ba8404 commit 16901a6
Show file tree
Hide file tree
Showing 10 changed files with 54 additions and 26 deletions.
3 changes: 1 addition & 2 deletions Library/Homebrew/cask/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/dev-cmd/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/dev-cmd/pr-pull.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Check warning on line 173 in Library/Homebrew/dev-cmd/pr-pull.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/dev-cmd/pr-pull.rb#L173

Added line #L173 was not covered by tests
ohai "Bottle files retained at:", dir
File.open(ENV.fetch("GITHUB_OUTPUT"), "a") do |f|
f.puts "bottle_path=#{dir}"
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/diagnostic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 9 additions & 3 deletions Library/Homebrew/extend/kernel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -146,24 +148,28 @@ 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
message.freeze

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
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/extend/os/mac/diagnostic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions Library/Homebrew/formula_installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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?
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/style.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/test/extend/kernel_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 33 additions & 10 deletions Library/Homebrew/utils/github/actions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -52,20 +71,20 @@ 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),
column: T.nilable(Integer),
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
Expand All @@ -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

Expand All @@ -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
Expand Down

0 comments on commit 16901a6

Please sign in to comment.