Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extend/kernel: make opoo/odie/etc. print GitHub Actions notes. #17255

Merged
merged 1 commit into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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
Loading