mirror of
https://github.com/Homebrew/brew.git
synced 2026-08-12 22:29:27 +04:00
Merge pull request #23435 from Homebrew/speedup-style-readall
Speed up `brew style` and `brew readall`
This commit is contained in:
@@ -121,11 +121,14 @@ module Cask
|
||||
|
||||
@default_config = T.let(config || Config.new, Config)
|
||||
|
||||
@config = T.let(if config_path.exist?
|
||||
Config.from_json(File.read(config_path), ignore_invalid_keys: true)
|
||||
else
|
||||
@default_config
|
||||
end, Config)
|
||||
@config = T.let(
|
||||
if config_path.exist?
|
||||
Config.from_json(File.read(config_path), ignore_invalid_keys: true)
|
||||
else
|
||||
@default_config
|
||||
end,
|
||||
Config,
|
||||
)
|
||||
refresh
|
||||
end
|
||||
|
||||
|
||||
@@ -38,7 +38,7 @@ module Homebrew
|
||||
Homebrew.with_no_api_env do
|
||||
if args.syntax? && args.no_named?
|
||||
scan_files = "#{HOMEBREW_LIBRARY_PATH}/**/*.rb"
|
||||
ruby_files = Dir.glob(scan_files).grep_v(%r{/(vendor)/})
|
||||
ruby_files = Dir.glob(scan_files).grep_v(%r{/(vendor)/}).map { Pathname(it) }
|
||||
|
||||
Homebrew.failed = true unless Readall.valid_ruby_syntax?(ruby_files)
|
||||
end
|
||||
|
||||
@@ -12,8 +12,15 @@ module OS
|
||||
|
||||
requires_ancestor { Kernel }
|
||||
|
||||
sig { params(tap: ::Tap, os_name: T.nilable(Symbol), arch: T.nilable(Symbol)).returns(T::Boolean) }
|
||||
def valid_casks?(tap, os_name: nil, arch: ::Hardware::CPU.type)
|
||||
sig {
|
||||
params(
|
||||
tap: ::Tap,
|
||||
os_name: T.nilable(Symbol),
|
||||
arch: T.nilable(Symbol),
|
||||
files: T.nilable(T::Array[::Pathname]),
|
||||
).returns(T::Boolean)
|
||||
}
|
||||
def valid_casks?(tap, os_name: nil, arch: ::Hardware::CPU.type, files: nil)
|
||||
return super if os_name == :linux
|
||||
|
||||
current_macos_version = if os_name.is_a?(Symbol)
|
||||
@@ -23,7 +30,7 @@ module OS
|
||||
end
|
||||
|
||||
success = T.let(true, T::Boolean)
|
||||
tap.cask_files.each do |file|
|
||||
(files || tap.cask_files).each do |file|
|
||||
cask = ::Cask::CaskLoader.load(file)
|
||||
|
||||
# Fine to have missing URLs for unsupported macOS
|
||||
|
||||
+154
-32
@@ -3,34 +3,59 @@
|
||||
|
||||
require "formula"
|
||||
require "cask/cask_loader"
|
||||
require "system_command"
|
||||
require "tempfile"
|
||||
require "utils/output"
|
||||
|
||||
# Helper module for validating syntax in taps.
|
||||
module Readall
|
||||
extend T::Generic
|
||||
extend Cachable
|
||||
extend SystemCommand::Mixin
|
||||
extend Utils::Output::Mixin
|
||||
|
||||
Cache = type_template { { fixed: T::Hash[Symbol, T.untyped] } }
|
||||
|
||||
private_class_method :cache
|
||||
|
||||
MIN_FILES_PER_WORKER = 4
|
||||
private_constant :MIN_FILES_PER_WORKER
|
||||
|
||||
# Buffers Ruby compile warnings from {.syntax_errors_or_warnings?} so they
|
||||
# can be filtered before being printed to `$stderr`.
|
||||
module WarningBuffer
|
||||
sig { params(message: String, category: T.nilable(Symbol)).void }
|
||||
def warn(message, category: nil)
|
||||
buffer = Readall.warning_buffer
|
||||
buffer ? buffer << message : super
|
||||
end
|
||||
end
|
||||
private_constant :WarningBuffer
|
||||
Warning.singleton_class.prepend(WarningBuffer)
|
||||
|
||||
@warning_buffer = T.let(nil, T.nilable(T::Array[String]))
|
||||
|
||||
class << self
|
||||
sig { returns(T.nilable(T::Array[String])) }
|
||||
attr_accessor :warning_buffer
|
||||
end
|
||||
|
||||
sig { params(ruby_files: T::Array[Pathname]).returns(T::Boolean) }
|
||||
def self.valid_ruby_syntax?(ruby_files)
|
||||
failed = T.let(false, T::Boolean)
|
||||
ruby_files.each do |ruby_file|
|
||||
# As a side effect, print syntax errors/warnings to `$stderr`.
|
||||
failed = true if syntax_errors_or_warnings?(ruby_file)
|
||||
parallel_slices_valid?(ruby_files) do |files|
|
||||
failed = T.let(false, T::Boolean)
|
||||
files.each do |ruby_file|
|
||||
# As a side effect, print syntax errors/warnings to `$stderr`.
|
||||
failed = true if syntax_errors_or_warnings?(ruby_file)
|
||||
end
|
||||
!failed
|
||||
end
|
||||
!failed
|
||||
end
|
||||
|
||||
sig { params(alias_dir: Pathname, formula_dir: Pathname).returns(T::Boolean) }
|
||||
def self.valid_aliases?(alias_dir, formula_dir)
|
||||
return true unless alias_dir.directory?
|
||||
|
||||
formula_basenames = Set.new(formula_dir.glob("**/*.rb").map { |formula_file| formula_file.basename.to_s })
|
||||
|
||||
failed = T.let(false, T::Boolean)
|
||||
alias_dir.each_child do |f|
|
||||
if !f.symlink?
|
||||
@@ -41,7 +66,7 @@ module Readall
|
||||
failed = true
|
||||
end
|
||||
|
||||
if formula_dir.glob("**/#{f.basename}.rb").any?(&:exist?)
|
||||
if formula_basenames.include?("#{f.basename}.rb")
|
||||
onoe "Formula duplicating alias: #{f}"
|
||||
failed = true
|
||||
end
|
||||
@@ -49,12 +74,16 @@ module Readall
|
||||
!failed
|
||||
end
|
||||
|
||||
sig { params(tap: Tap, bottle_tag: T.nilable(Utils::Bottles::Tag)).returns(T::Boolean) }
|
||||
def self.valid_formulae?(tap, bottle_tag: nil)
|
||||
sig {
|
||||
params(
|
||||
tap: Tap, bottle_tag: T.nilable(Utils::Bottles::Tag), files: T.nilable(T::Array[Pathname]),
|
||||
).returns(T::Boolean)
|
||||
}
|
||||
def self.valid_formulae?(tap, bottle_tag: nil, files: nil)
|
||||
cache[:valid_formulae] ||= {}
|
||||
|
||||
success = T.let(true, T::Boolean)
|
||||
tap.formula_files.each do |file|
|
||||
(files || tap.formula_files).each do |file|
|
||||
valid = cache[:valid_formulae][file]
|
||||
next if valid == true || valid&.include?(bottle_tag)
|
||||
|
||||
@@ -82,8 +111,12 @@ module Readall
|
||||
success
|
||||
end
|
||||
|
||||
sig { params(tap: Tap, os_name: T.nilable(Symbol), arch: T.nilable(Symbol)).returns(T::Boolean) }
|
||||
def self.valid_casks?(tap, os_name: nil, arch: nil)
|
||||
sig {
|
||||
params(
|
||||
tap: Tap, os_name: T.nilable(Symbol), arch: T.nilable(Symbol), files: T.nilable(T::Array[Pathname]),
|
||||
).returns(T::Boolean)
|
||||
}
|
||||
def self.valid_casks?(tap, os_name: nil, arch: nil, files: nil)
|
||||
validating_linux = if os_name.nil?
|
||||
Homebrew::SimulateSystem.current_os == :linux
|
||||
else
|
||||
@@ -95,7 +128,7 @@ module Readall
|
||||
os_and_arch += " on #{(arch == :intel) ? "Intel x86_64" : "ARM64"}" if arch
|
||||
|
||||
success = T.let(true, T::Boolean)
|
||||
tap.cask_files.each do |file|
|
||||
(files || tap.cask_files).each do |file|
|
||||
next if file.read.match?(/^\s*depends_on(?:\s*\(\s*|\s+)(?::macos\b|macos:)/)
|
||||
|
||||
cask = if arch
|
||||
@@ -155,40 +188,129 @@ module Readall
|
||||
success = false unless valid_aliases
|
||||
end
|
||||
|
||||
if no_simulate
|
||||
success = false unless valid_formulae?(tap)
|
||||
success = false unless valid_casks?(tap)
|
||||
else
|
||||
os_arch_combinations.each do |os, arch|
|
||||
bottle_tag = Utils::Bottles::Tag.new(system: os, arch:)
|
||||
next unless bottle_tag.valid_combination?
|
||||
items = tap.formula_files.map { |file| [:formula, file] } +
|
||||
tap.cask_files.map { |file| [:cask, file] }
|
||||
|
||||
Homebrew::SimulateSystem.with(os:, arch:) do
|
||||
success = false unless valid_formulae?(tap, bottle_tag:)
|
||||
success = false unless valid_casks?(tap, os_name: os, arch:)
|
||||
all_files_valid = parallel_slices_valid?(items) do |slice|
|
||||
formula_files = slice.filter_map { |type, file| file if type == :formula }
|
||||
cask_files = slice.filter_map { |type, file| file if type == :cask }
|
||||
|
||||
slice_success = T.let(true, T::Boolean)
|
||||
if no_simulate
|
||||
slice_success = false unless valid_formulae?(tap, files: formula_files)
|
||||
slice_success = false unless valid_casks?(tap, files: cask_files)
|
||||
else
|
||||
os_arch_combinations.each do |os, arch|
|
||||
bottle_tag = Utils::Bottles::Tag.new(system: os, arch:)
|
||||
next unless bottle_tag.valid_combination?
|
||||
|
||||
Homebrew::SimulateSystem.with(os:, arch:) do
|
||||
slice_success = false unless valid_formulae?(tap, bottle_tag:, files: formula_files)
|
||||
slice_success = false unless valid_casks?(tap, os_name: os, arch:, files: cask_files)
|
||||
end
|
||||
end
|
||||
end
|
||||
slice_success
|
||||
end
|
||||
success = false unless all_files_valid
|
||||
|
||||
success
|
||||
end
|
||||
|
||||
sig { params(filename: Pathname).returns(T::Boolean) }
|
||||
private_class_method def self.syntax_errors_or_warnings?(filename)
|
||||
# Retrieve messages about syntax errors/warnings printed to `$stderr`.
|
||||
_, err, status = system_command(RUBY_PATH, args: ["-c", "-w", filename], print_stderr: false).to_a
|
||||
# Compile in-process (much faster than spawning `ruby -c -w` per file),
|
||||
# buffering compile warnings so they can be filtered.
|
||||
error = T.let(nil, T.nilable(String))
|
||||
warnings = self.warning_buffer = []
|
||||
old_verbose = $VERBOSE
|
||||
$VERBOSE = true
|
||||
begin
|
||||
RubyVM::InstructionSequence.compile_file(filename.to_s)
|
||||
rescue ScriptError, ArgumentError => e
|
||||
error = "#{e.message.chomp}\n"
|
||||
ensure
|
||||
$VERBOSE = old_verbose
|
||||
self.warning_buffer = nil
|
||||
end
|
||||
|
||||
# Ignore unnecessary warning about named capture conflicts.
|
||||
# See https://bugs.ruby-lang.org/issues/12359.
|
||||
messages = err.lines
|
||||
.grep_v(/named capture conflicts a local variable/)
|
||||
.join
|
||||
messages = warnings.grep_v(/named capture conflicts a local variable/).join
|
||||
messages += error if error
|
||||
|
||||
$stderr.print messages
|
||||
|
||||
# Only syntax errors result in a non-zero status code. To detect syntax
|
||||
# warnings we also need to inspect the output to `$stderr`.
|
||||
!status.success? || !messages.chomp.empty?
|
||||
# Both syntax errors and syntax warnings count as failures.
|
||||
!messages.chomp.empty?
|
||||
end
|
||||
|
||||
sig {
|
||||
type_parameters(:U).params(
|
||||
items: T::Array[T.type_parameter(:U)],
|
||||
_block: T.proc.params(arg0: T::Array[T.type_parameter(:U)]).returns(T::Boolean),
|
||||
).returns(T::Boolean)
|
||||
}
|
||||
private_class_method def self.parallel_slices_valid?(items, &_block)
|
||||
require "hardware"
|
||||
|
||||
worker_count = [Hardware::CPU.cores, items.length / MIN_FILES_PER_WORKER].min
|
||||
return yield(items) if worker_count <= 1
|
||||
|
||||
workers = items.each_slice((items.length.to_f / worker_count).ceil).map do |slice|
|
||||
reader, writer = IO.pipe
|
||||
stdout_file = Tempfile.new("readall-stdout")
|
||||
stderr_file = Tempfile.new("readall-stderr")
|
||||
pid = Process.fork do
|
||||
reader.close
|
||||
success = begin
|
||||
# Capture output so parallel workers cannot interleave lines.
|
||||
$stdout = stdout_file.to_io
|
||||
$stderr = stderr_file.to_io
|
||||
yield(slice)
|
||||
rescue Interrupt
|
||||
false
|
||||
# Report any worker exception as a validation failure.
|
||||
rescue Exception => e # rubocop:disable Lint/RescueException
|
||||
$stderr.puts e.full_message
|
||||
false
|
||||
ensure
|
||||
$stdout.flush
|
||||
$stderr.flush
|
||||
end
|
||||
writer.write(Marshal.dump(success))
|
||||
writer.close
|
||||
exit!(true)
|
||||
end
|
||||
writer.close
|
||||
[pid, reader, stdout_file, stderr_file]
|
||||
end
|
||||
|
||||
success = T.let(true, T::Boolean)
|
||||
workers.each do |pid, reader, stdout_file, stderr_file|
|
||||
worker_success = begin
|
||||
# The data being loaded was written by our own forked child process.
|
||||
Marshal.load(reader) # rubocop:disable Security/MarshalLoad
|
||||
rescue EOFError
|
||||
nil
|
||||
end
|
||||
reader.close
|
||||
Process.wait(pid)
|
||||
|
||||
[stdout_file, stderr_file].each(&:rewind)
|
||||
$stdout.print stdout_file.read
|
||||
$stderr.print stderr_file.read
|
||||
[stdout_file, stderr_file].each(&:close!)
|
||||
|
||||
case worker_success
|
||||
when nil
|
||||
onoe "readall worker exited unexpectedly!"
|
||||
success = false
|
||||
when false
|
||||
success = false
|
||||
end
|
||||
end
|
||||
success
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
+150
-51
@@ -3,6 +3,7 @@
|
||||
|
||||
require "shellwords"
|
||||
require "source_location"
|
||||
require "stringio"
|
||||
require "system_command"
|
||||
require "tap"
|
||||
require "utils/output"
|
||||
@@ -88,9 +89,53 @@ module Homebrew
|
||||
end
|
||||
end
|
||||
|
||||
rubocop_result = if files.present? && ruby_files.empty?
|
||||
(output_type == :json) ? [] : true
|
||||
else
|
||||
rubocop_needed = files.blank? || ruby_files.any?
|
||||
shell_needed = files.blank? || shell_files.any?
|
||||
|
||||
actionlint_files = github_workflow_files if files.blank? && actionlint_files.blank?
|
||||
has_actionlint_workflow = actionlint_files.any? do |path|
|
||||
path.to_s.end_with?("/.github/workflows/actionlint.yml")
|
||||
end
|
||||
odebug "actionlint workflow detected. Skipping actionlint checks." if has_actionlint_workflow
|
||||
actionlint_needed = files.blank? || (!has_actionlint_workflow && actionlint_files.any?)
|
||||
|
||||
# Resolve the linter executables (installing them if necessary) before
|
||||
# spawning threads so those threads cannot race to install formulae.
|
||||
shellcheck_path = (shellcheck if shell_needed || actionlint_needed)
|
||||
shfmt_path = (shfmt_executable if shell_needed)
|
||||
actionlint_path = (actionlint if actionlint_needed)
|
||||
|
||||
shellcheck_out = StringIO.new
|
||||
shellcheck_err = StringIO.new
|
||||
shfmt_out = StringIO.new
|
||||
shfmt_err = StringIO.new
|
||||
actionlint_out = StringIO.new
|
||||
actionlint_err = StringIO.new
|
||||
|
||||
# Run the shell and GitHub Actions checks on background threads with
|
||||
# buffered output while RuboCop runs on the main thread.
|
||||
shell_thread = Thread.new do
|
||||
shellcheck_result = if shell_needed
|
||||
run_shellcheck(shell_files, output_type, fix:, shellcheck_path:,
|
||||
out: shellcheck_out, err: shellcheck_err)
|
||||
elsif output_type == :json
|
||||
[]
|
||||
else
|
||||
true
|
||||
end
|
||||
# `shellcheck --fix` and `shfmt --write` may touch the same files so
|
||||
# they must not run concurrently with each other.
|
||||
shfmt_result = !shell_needed || run_shfmt!(shell_files, fix:, shfmt_path:,
|
||||
out: shfmt_out, err: shfmt_err)
|
||||
[shellcheck_result, shfmt_result]
|
||||
end
|
||||
actionlint_thread = Thread.new do
|
||||
!actionlint_needed ||
|
||||
run_actionlint!(actionlint_files, actionlint_path:, shellcheck_path:,
|
||||
out: actionlint_out, err: actionlint_err)
|
||||
end
|
||||
|
||||
rubocop_result = if rubocop_needed
|
||||
run_rubocop(ruby_files, output_type,
|
||||
fix:,
|
||||
todo:,
|
||||
@@ -98,24 +143,23 @@ module Homebrew
|
||||
display_cop_names:,
|
||||
reset_cache:,
|
||||
debug:, verbose:)
|
||||
end
|
||||
|
||||
shellcheck_result = if files.present? && shell_files.empty?
|
||||
(output_type == :json) ? [] : true
|
||||
elsif output_type == :json
|
||||
[]
|
||||
else
|
||||
run_shellcheck(shell_files, output_type, fix:)
|
||||
true
|
||||
end
|
||||
|
||||
shfmt_result = files.present? && shell_files.empty?
|
||||
shfmt_result ||= run_shfmt!(shell_files, fix:)
|
||||
shellcheck_result, shfmt_result = shell_thread.value
|
||||
actionlint_result = actionlint_thread.value
|
||||
|
||||
actionlint_files = github_workflow_files if files.blank? && actionlint_files.blank?
|
||||
has_actionlint_workflow = actionlint_files.any? do |path|
|
||||
path.to_s.end_with?("/.github/workflows/actionlint.yml")
|
||||
[
|
||||
[shellcheck_out, shellcheck_err],
|
||||
[shfmt_out, shfmt_err],
|
||||
[actionlint_out, actionlint_err],
|
||||
].each do |out, err|
|
||||
$stdout.print out.string
|
||||
$stderr.print err.string
|
||||
end
|
||||
odebug "actionlint workflow detected. Skipping actionlint checks." if has_actionlint_workflow
|
||||
actionlint_result = files.present? && (has_actionlint_workflow || actionlint_files.empty?)
|
||||
actionlint_result ||= run_actionlint!(actionlint_files)
|
||||
|
||||
if output_type == :json
|
||||
Offenses.new(
|
||||
@@ -200,7 +244,7 @@ module Homebrew
|
||||
HOMEBREW_CACHE.mkpath
|
||||
cache_dir = HOMEBREW_CACHE.realpath/"style"
|
||||
cache_env = if (!cache_dir.exist? && cache_dir.parent.writable?) || cache_dir.writable?
|
||||
args << "--parallel" unless fix
|
||||
args << "--parallel"
|
||||
|
||||
FileUtils.rm_rf cache_dir if reset_cache
|
||||
|
||||
@@ -239,10 +283,17 @@ module Homebrew
|
||||
end
|
||||
|
||||
sig {
|
||||
params(files: T::Array[Pathname], output_type: Symbol, fix: T::Boolean)
|
||||
.returns(T.nilable(T.any(T::Boolean, T::Array[T::Hash[String, T.untyped]])))
|
||||
params(
|
||||
files: T::Array[Pathname],
|
||||
output_type: Symbol,
|
||||
fix: T::Boolean,
|
||||
shellcheck_path: T.nilable(Pathname),
|
||||
out: T.any(IO, StringIO),
|
||||
err: T.any(IO, StringIO),
|
||||
).returns(T.nilable(T.any(T::Boolean, T::Array[T::Hash[String, T.untyped]])))
|
||||
}
|
||||
def self.run_shellcheck(files, output_type, fix: false)
|
||||
def self.run_shellcheck(files, output_type, fix: false, shellcheck_path: nil, out: $stdout, err: $stderr)
|
||||
shellcheck_path ||= shellcheck
|
||||
files = shell_scripts if files.blank?
|
||||
|
||||
files = files.map(&:realpath) # use absolute file paths
|
||||
@@ -252,8 +303,6 @@ module Homebrew
|
||||
"--enable=all",
|
||||
"--external-sources",
|
||||
"--source-path=#{HOMEBREW_LIBRARY}",
|
||||
"--",
|
||||
*files,
|
||||
]
|
||||
|
||||
if fix
|
||||
@@ -264,17 +313,23 @@ module Homebrew
|
||||
# -p0 (--strip=0) : do not strip path prefixes, since we are at root directory
|
||||
# NOTE: We use short flags for compatibility.
|
||||
patch_command = %w[patch -g 0 -f -d / -p0]
|
||||
patches = system_command(shellcheck, args: ["--format=diff", *args]).stdout
|
||||
patches = shellcheck_chunks(shellcheck_path, files, ["--format=diff", *args]).map(&:stdout).join
|
||||
Utils.safe_popen_write(*patch_command) { |p| p.write(patches) } if patches.present?
|
||||
end
|
||||
|
||||
case output_type
|
||||
when :print
|
||||
system shellcheck, "--format=tty", *args
|
||||
$CHILD_STATUS.success?
|
||||
print_args = ["--format=tty", *args]
|
||||
print_args << "--color=always" if Tty.color?
|
||||
results = shellcheck_chunks(shellcheck_path, files, print_args)
|
||||
results.each do |result|
|
||||
out.print result.stdout
|
||||
err.print result.stderr
|
||||
end
|
||||
results.all?(&:success?)
|
||||
when :json
|
||||
result = system_command shellcheck, args: ["--format=json", *args]
|
||||
json = json_result!(result)
|
||||
results = shellcheck_chunks(shellcheck_path, files, ["--format=json", *args])
|
||||
json = results.flat_map { |result| json_result!(result) }
|
||||
|
||||
# Convert to same format as RuboCop offenses.
|
||||
severity_hash = { "style" => "refactor", "info" => "convention" }
|
||||
@@ -312,8 +367,37 @@ module Homebrew
|
||||
end
|
||||
end
|
||||
|
||||
sig { params(files: T::Array[Pathname], fix: T::Boolean).returns(T::Boolean) }
|
||||
def self.run_shfmt!(files, fix: false)
|
||||
sig {
|
||||
params(
|
||||
shellcheck_path: Pathname,
|
||||
files: T::Array[Pathname],
|
||||
args: T::Array[String],
|
||||
).returns(T::Array[SystemCommand::Result])
|
||||
}
|
||||
private_class_method def self.shellcheck_chunks(shellcheck_path, files, args)
|
||||
require "hardware"
|
||||
|
||||
chunk_count = [Hardware::CPU.cores, files.length].min
|
||||
return [] if chunk_count.zero?
|
||||
|
||||
files.each_slice((files.length.to_f / chunk_count).ceil).map do |chunk|
|
||||
Thread.new do
|
||||
system_command shellcheck_path, args: [*args, "--", *chunk], print_stderr: false
|
||||
end
|
||||
end.map(&:value)
|
||||
end
|
||||
|
||||
sig {
|
||||
params(
|
||||
files: T::Array[Pathname],
|
||||
fix: T::Boolean,
|
||||
shfmt_path: T.nilable(Pathname),
|
||||
out: T.any(IO, StringIO),
|
||||
err: T.any(IO, StringIO),
|
||||
).returns(T::Boolean)
|
||||
}
|
||||
def self.run_shfmt!(files, fix: false, shfmt_path: nil, out: $stdout, err: $stderr)
|
||||
shfmt_path ||= shfmt_executable
|
||||
files = shell_scripts if files.blank?
|
||||
# Do not format completions and Dockerfile
|
||||
files.delete(HOMEBREW_REPOSITORY/"completions/bash/brew")
|
||||
@@ -322,23 +406,27 @@ module Homebrew
|
||||
args = ["--language-dialect", "bash", "--indent", "2", "--case-indent", "--", *files]
|
||||
args.unshift("--write") if fix # need to add before "--"
|
||||
|
||||
require "formula"
|
||||
shfmt_executable = T.cast(
|
||||
Formula["shfmt"].ensure_installed!(latest: true,
|
||||
reason: "formatting shell scripts",
|
||||
executable: "shfmt"),
|
||||
Pathname,
|
||||
)
|
||||
system(
|
||||
{ "HOMEBREW_SHFMT" => shfmt_executable.to_s },
|
||||
shfmt,
|
||||
*args,
|
||||
)
|
||||
$CHILD_STATUS.success?
|
||||
result = system_command shfmt,
|
||||
args:,
|
||||
env: { "HOMEBREW_SHFMT" => shfmt_path.to_s },
|
||||
print_stderr: false
|
||||
out.print result.stdout
|
||||
err.print result.stderr
|
||||
result.success?
|
||||
end
|
||||
|
||||
sig { params(files: T::Array[Pathname]).returns(T::Boolean) }
|
||||
def self.run_actionlint!(files)
|
||||
sig {
|
||||
params(
|
||||
files: T::Array[Pathname],
|
||||
actionlint_path: T.nilable(Pathname),
|
||||
shellcheck_path: T.nilable(Pathname),
|
||||
out: T.any(IO, StringIO),
|
||||
err: T.any(IO, StringIO),
|
||||
).returns(T::Boolean)
|
||||
}
|
||||
def self.run_actionlint!(files, actionlint_path: nil, shellcheck_path: nil, out: $stdout, err: $stderr)
|
||||
actionlint_path ||= actionlint
|
||||
shellcheck_path ||= shellcheck
|
||||
files = github_workflow_files if files.blank?
|
||||
|
||||
tap_configs = files.filter_map do |f|
|
||||
@@ -350,18 +438,21 @@ module Homebrew
|
||||
end.uniq
|
||||
|
||||
config_file = if tap_configs.one?
|
||||
tap_configs.first
|
||||
tap_configs.fetch(0)
|
||||
else
|
||||
HOMEBREW_REPOSITORY/".github/actionlint.yaml"
|
||||
end
|
||||
|
||||
# the ignore is to avoid false positives in e.g. actions, homebrew-test-bot
|
||||
system actionlint, "-shellcheck", shellcheck,
|
||||
"-config-file", config_file,
|
||||
"-ignore", "image: string; options: string",
|
||||
"-ignore", "label .* is unknown",
|
||||
*files
|
||||
$CHILD_STATUS.success?
|
||||
args = ["-shellcheck", shellcheck_path,
|
||||
"-config-file", config_file,
|
||||
"-ignore", "image: string; options: string",
|
||||
"-ignore", "label .* is unknown"]
|
||||
args << "-color" if Tty.color?
|
||||
result = system_command actionlint_path, args: [*args, *files], print_stderr: false
|
||||
out.print result.stdout
|
||||
err.print result.stderr
|
||||
result.success?
|
||||
end
|
||||
|
||||
sig { params(result: SystemCommand::Result).returns(T.untyped) }
|
||||
@@ -412,6 +503,14 @@ module Homebrew
|
||||
HOMEBREW_LIBRARY/"Homebrew/utils/shfmt.sh"
|
||||
end
|
||||
|
||||
sig { returns(Pathname) }
|
||||
private_class_method def self.shfmt_executable
|
||||
require "formula"
|
||||
T.cast(Formula["shfmt"].ensure_installed!(latest: true,
|
||||
reason: "formatting shell scripts",
|
||||
executable: "shfmt"), Pathname)
|
||||
end
|
||||
|
||||
sig { returns(Pathname) }
|
||||
def self.actionlint
|
||||
require "formula"
|
||||
|
||||
@@ -65,6 +65,83 @@ RSpec.describe Homebrew::Cmd::ReadallCmd do
|
||||
expect(success).to be false
|
||||
end
|
||||
|
||||
describe "Readall.valid_ruby_syntax?" do
|
||||
it "returns true for valid Ruby files" do
|
||||
file = mktmpdir/"valid.rb"
|
||||
file.write "puts 1\n"
|
||||
|
||||
success = T.let(false, T::Boolean)
|
||||
expect { success = Readall.valid_ruby_syntax?([file]) }.not_to output.to_stderr
|
||||
expect(success).to be true
|
||||
end
|
||||
|
||||
it "prints errors for files with invalid syntax" do
|
||||
file = mktmpdir/"invalid.rb"
|
||||
file.write "def foo(\n"
|
||||
|
||||
success = T.let(true, T::Boolean)
|
||||
expect { success = Readall.valid_ruby_syntax?([file]) }.to output(/syntax error/).to_stderr
|
||||
expect(success).to be false
|
||||
end
|
||||
|
||||
it "prints warnings for files with questionable syntax" do
|
||||
file = mktmpdir/"warning.rb"
|
||||
file.write "def foo\n bar = 1\n nil\nend\n"
|
||||
|
||||
success = T.let(true, T::Boolean)
|
||||
expect { success = Readall.valid_ruby_syntax?([file]) }.to output(/unused variable/).to_stderr
|
||||
expect(success).to be false
|
||||
end
|
||||
|
||||
it "aggregates failures across parallel worker processes" do
|
||||
dir = mktmpdir
|
||||
files = (1..9).map do |i|
|
||||
file = dir/"valid#{i}.rb"
|
||||
file.write "puts #{i}\n"
|
||||
file
|
||||
end
|
||||
bad_file = dir/"invalid.rb"
|
||||
bad_file.write "def foo(\n"
|
||||
files << bad_file
|
||||
|
||||
success = T.let(true, T::Boolean)
|
||||
expect { success = Readall.valid_ruby_syntax?(files) }.to output(/syntax error/).to_stderr
|
||||
expect(success).to be false
|
||||
end
|
||||
end
|
||||
|
||||
it "validates tap files in parallel worker processes" do
|
||||
tap_path = mktmpdir
|
||||
cask_files = (1..8).map do |i|
|
||||
file = tap_path/"Casks/linux-example#{i}.rb"
|
||||
file.dirname.mkpath
|
||||
file.write <<~RUBY
|
||||
cask "linux-example#{i}" do
|
||||
version "1.0"
|
||||
sha256 arm: "0000000000000000000000000000000000000000000000000000000000000000"
|
||||
url "https://example.invalid/x.tar.gz"
|
||||
name "Example"
|
||||
desc "Cask missing Linux stanzas"
|
||||
homepage "https://example.invalid/"
|
||||
binary "x"
|
||||
end
|
||||
RUBY
|
||||
file
|
||||
end
|
||||
|
||||
success = T.let(true, T::Boolean)
|
||||
expect do
|
||||
success = Homebrew::SimulateSystem.with(os: :linux) do
|
||||
Readall.valid_tap?(
|
||||
instance_double(Tap, formula_files: [], cask_files:),
|
||||
os_arch_combinations: [[:linux, :arm]],
|
||||
)
|
||||
end
|
||||
end.to output(a_string_matching(/(?=.*linux-example1\.rb)(?=.*linux-example8\.rb)/m)).to_stderr
|
||||
|
||||
expect(success).to be false
|
||||
end
|
||||
|
||||
it "explains nil sha256 values when loading tap casks on Linux" do
|
||||
tap_path = mktmpdir
|
||||
linux_cask_file = tap_path/"Casks/linux-example.rb"
|
||||
|
||||
@@ -52,11 +52,14 @@ RSpec.describe Homebrew::Style do
|
||||
end
|
||||
|
||||
describe ".run_actionlint!" do
|
||||
let(:actionlint_result) do
|
||||
instance_double(SystemCommand::Result, success?: true, stdout: "", stderr: "")
|
||||
end
|
||||
|
||||
before do
|
||||
allow(described_class).to receive_messages(actionlint: "actionlint", shellcheck: "shellcheck")
|
||||
# Run a trivial command so $CHILD_STATUS is non-nil after the stubbed `system` call.
|
||||
system("true")
|
||||
allow(described_class).to receive(:system).and_return(true)
|
||||
allow(Tty).to receive(:color?).and_return(false)
|
||||
allow(described_class).to receive(:system_command).and_return(actionlint_result)
|
||||
end
|
||||
|
||||
it "uses a tap's actionlint config when present" do
|
||||
@@ -69,13 +72,15 @@ RSpec.describe Homebrew::Style do
|
||||
tap_config = tap_path/".github/actionlint.yaml"
|
||||
tap_config.write "self-hosted-runner:\n labels: []\n"
|
||||
|
||||
expect(described_class).to receive(:system).with(
|
||||
"actionlint", "-shellcheck", "shellcheck",
|
||||
"-config-file", tap_config,
|
||||
"-ignore", "image: string; options: string",
|
||||
"-ignore", "label .* is unknown",
|
||||
workflow
|
||||
)
|
||||
expect(described_class).to receive(:system_command).with(
|
||||
"actionlint",
|
||||
args: ["-shellcheck", "shellcheck",
|
||||
"-config-file", tap_config,
|
||||
"-ignore", "image: string; options: string",
|
||||
"-ignore", "label .* is unknown",
|
||||
workflow],
|
||||
print_stderr: false,
|
||||
).and_return(actionlint_result)
|
||||
|
||||
described_class.run_actionlint!([workflow])
|
||||
end
|
||||
@@ -87,13 +92,15 @@ RSpec.describe Homebrew::Style do
|
||||
workflow = workflows_dir/"ci.yml"
|
||||
workflow.write "name: CI"
|
||||
|
||||
expect(described_class).to receive(:system).with(
|
||||
"actionlint", "-shellcheck", "shellcheck",
|
||||
"-config-file", HOMEBREW_REPOSITORY/".github/actionlint.yaml",
|
||||
"-ignore", "image: string; options: string",
|
||||
"-ignore", "label .* is unknown",
|
||||
workflow
|
||||
)
|
||||
expect(described_class).to receive(:system_command).with(
|
||||
"actionlint",
|
||||
args: ["-shellcheck", "shellcheck",
|
||||
"-config-file", HOMEBREW_REPOSITORY/".github/actionlint.yaml",
|
||||
"-ignore", "image: string; options: string",
|
||||
"-ignore", "label .* is unknown",
|
||||
workflow],
|
||||
print_stderr: false,
|
||||
).and_return(actionlint_result)
|
||||
|
||||
described_class.run_actionlint!([workflow])
|
||||
end
|
||||
@@ -111,13 +118,15 @@ RSpec.describe Homebrew::Style do
|
||||
workflow2 = tap2_path/".github/workflows/ci.yml"
|
||||
workflow2.write "name: CI"
|
||||
|
||||
expect(described_class).to receive(:system).with(
|
||||
"actionlint", "-shellcheck", "shellcheck",
|
||||
"-config-file", HOMEBREW_REPOSITORY/".github/actionlint.yaml",
|
||||
"-ignore", "image: string; options: string",
|
||||
"-ignore", "label .* is unknown",
|
||||
workflow1, workflow2
|
||||
)
|
||||
expect(described_class).to receive(:system_command).with(
|
||||
"actionlint",
|
||||
args: ["-shellcheck", "shellcheck",
|
||||
"-config-file", HOMEBREW_REPOSITORY/".github/actionlint.yaml",
|
||||
"-ignore", "image: string; options: string",
|
||||
"-ignore", "label .* is unknown",
|
||||
workflow1, workflow2],
|
||||
print_stderr: false,
|
||||
).and_return(actionlint_result)
|
||||
|
||||
described_class.run_actionlint!([workflow1, workflow2])
|
||||
end
|
||||
@@ -162,15 +171,47 @@ RSpec.describe Homebrew::Style do
|
||||
reason: "formatting shell scripts",
|
||||
executable: "shfmt")
|
||||
.and_return(Pathname.new("/usr/bin/shfmt"))
|
||||
system("true")
|
||||
|
||||
expect(described_class).to receive(:system).with(
|
||||
{ "HOMEBREW_SHFMT" => "/usr/bin/shfmt" },
|
||||
shfmt_result = instance_double(SystemCommand::Result, success?: true, stdout: "", stderr: "")
|
||||
expect(described_class).to receive(:system_command).with(
|
||||
HOMEBREW_LIBRARY/"Homebrew/utils/shfmt.sh",
|
||||
"--language-dialect", "bash", "--indent", "2", "--case-indent", "--", shell_file
|
||||
).and_return(true)
|
||||
args: ["--language-dialect", "bash", "--indent", "2", "--case-indent", "--", shell_file],
|
||||
env: { "HOMEBREW_SHFMT" => "/usr/bin/shfmt" },
|
||||
print_stderr: false,
|
||||
).and_return(shfmt_result)
|
||||
|
||||
described_class.run_shfmt!([shell_file])
|
||||
expect(described_class.run_shfmt!([shell_file])).to be true
|
||||
end
|
||||
end
|
||||
|
||||
describe ".run_shellcheck" do
|
||||
it "runs shellcheck in parallel chunks and merges their JSON results" do
|
||||
dir = mktmpdir
|
||||
log = dir/"shellcheck-args.log"
|
||||
fake_shellcheck = dir/"shellcheck"
|
||||
fake_shellcheck.write <<~SCRIPT
|
||||
#!/bin/bash
|
||||
echo "$*" >> "#{log}"
|
||||
echo "[]"
|
||||
SCRIPT
|
||||
fake_shellcheck.chmod 0755
|
||||
|
||||
files = (1..3).map do |i|
|
||||
file = dir/"script#{i}.sh"
|
||||
file.write "#!/bin/bash\n"
|
||||
file
|
||||
end
|
||||
|
||||
allow(Hardware::CPU).to receive(:cores).and_return(2)
|
||||
|
||||
offenses = described_class.run_shellcheck(files, :json, shellcheck_path: fake_shellcheck)
|
||||
|
||||
expect(offenses).to eq []
|
||||
chunks = log.read.lines
|
||||
expect(chunks.length).to eq 2
|
||||
first_chunk = chunks.find { |chunk| chunk.include?("script1.sh") }
|
||||
expect(first_chunk).to include("script2.sh")
|
||||
expect(first_chunk).not_to include("script3.sh")
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
Reference in New Issue
Block a user