Enable more rubocop cops (#9980)

* Remove self.check in python3.rb as tests were not actuallly being run

* Enable Lint/ImplicitStringConcatenation cop

* Enable Layout/CommentIndentation cop

* Remove unnecessary configuration of Layout/IndentationStyle to EnforcedStyle: spaces, as this is already the default

* Enable Layout/LeadingCommentSpace cop

* Enable Layout/SpaceInsideBlockBraces cop

* Enable Layout/SpaceInsideParens cop

* Enable Layout/TrailingEmptyLines cop

* Enable Lint/LiteralAsCondition cop

* Document the current issue stopping us from enabling Style/OptionalBooleanParameter

* Stop downloading our rubocop config when installing ruby_rubocop
This commit is contained in:
Maximilian Downey Twiss
2024-06-18 06:19:11 +10:00
committed by GitHub
parent fcb36066e2
commit 33901368d7
27 changed files with 54 additions and 118 deletions

View File

@@ -20,9 +20,6 @@ AllCops:
Security/Eval:
Enabled: false
Lint/ImplicitStringConcatenation:
Enabled: false
Lint/ShadowingOuterLocalVariable:
Enabled: false
@@ -39,9 +36,6 @@ Layout/HashAlignment:
# End temporarily disabled cops
Layout/CommentIndentation:
Enabled: false
Layout/EmptyLineAfterGuardClause:
Enabled: false
@@ -49,27 +43,9 @@ Layout/FirstHashElementIndentation:
Enabled: false
# EnforcedStyle: consistent
Layout/IndentationStyle:
EnforcedStyle: spaces
Layout/LeadingCommentSpace:
Enabled: false
Layout/LineLength:
Enabled: false
Layout/SpaceInsideBlockBraces:
Enabled: false
Layout/SpaceInsideParens:
Enabled: false
Layout/TrailingEmptyLines:
Enabled: false
Lint/LiteralAsCondition:
Enabled: false
Style/RedundantReturn:
Enabled: false
@@ -88,6 +64,7 @@ Style/GuardClause:
Style/DocumentDynamicEvalDefinition:
Enabled: false
# https://github.com/rubocop/rubocop/issues/12132
Style/OptionalBooleanParameter:
Enabled: false

View File

@@ -106,7 +106,7 @@ class ExitMessage
def self.handle_messages(msg)
puts msg
# Delete printed message from array & only print each message once.
@messages.reject! {|x| x.include? msg }
@messages.reject! { |x| x.include? msg }
end
def self.print
@@ -138,7 +138,7 @@ def load_json
@device = JSON.load_file(json_path, symbolize_names: true)
# symbolize also values
@device.transform_values! {|val| val.is_a?(String) ? val.to_sym : val }
@device.transform_values! { |val| val.is_a?(String) ? val.to_sym : val }
end
def save_json(json_object)
@@ -586,7 +586,7 @@ def unpack(meta)
FileUtils.mkdir_p @extract_dir, verbose: @fileutils_verbose
build_cachefile = File.join(CREW_CACHE_DIR, "#{@pkg.name}-#{@pkg.version}-build-#{@device[:architecture]}.tar.zst")
if CREW_CACHE_BUILD && File.file?(build_cachefile) && File.file?("#{build_cachefile}.sha256") && ( system "cd #{CREW_CACHE_DIR} && sha256sum -c #{build_cachefile}.sha256" )
if CREW_CACHE_BUILD && File.file?(build_cachefile) && File.file?("#{build_cachefile}.sha256") && (system "cd #{CREW_CACHE_DIR} && sha256sum -c #{build_cachefile}.sha256")
@pkg.cached_build = true
puts "Extracting cached build directory from #{build_cachefile}".lightgreen
system "tar -Izstd -x#{@verbose}f #{build_cachefile} -C #{CREW_BREW_DIR}", exception: true
@@ -624,10 +624,10 @@ def unpack(meta)
puts "Empty archive: #{meta[:filename]}".orange
end
target_dir = if entries.length == 1 && File.directory?(entries.first)
# Use `extract_dir/dir_in_archive` if there is only one directory.
# Use `extract_dir/dir_in_archive` if there is only one directory.
entries.first
else
# Use `extract_dir` otherwise
# Use `extract_dir` otherwise
@extract_dir
end
else
@@ -735,7 +735,7 @@ def determine_conflicts(dir, pkg)
puts 'There is a conflict with the same file in another package:'.orange
puts conflicts.to_s.orange
end
conflicts.map! { |x| x.to_s.partition(':').last}
conflicts.map! { |x| x.to_s.partition(':').last }
return conflicts
end
@@ -1013,7 +1013,7 @@ def shrink_dir(dir)
end
end
def install_files(src, dst = File.join( CREW_PREFIX, src.delete_prefix('./usr/local') ))
def install_files(src, dst = File.join(CREW_PREFIX, src.delete_prefix('./usr/local')))
if Dir.exist?(src)
if File.executable?("#{CREW_PREFIX}/bin/crew-mvdir") && !CREW_DISABLE_MVDIR
system "crew-mvdir #{@short_verbose} #{src} #{dst}", exception: true
@@ -1147,7 +1147,7 @@ def resolve_dependencies
# compare dependency version with required range (if installed)
@dependencies.each do |dep|
dep_name = dep.keys[0]
dep_info = @device[:installed_packages].select {|pkg| pkg[:name] == dep_name } [0]
dep_info = @device[:installed_packages].select { |pkg| pkg[:name] == dep_name } [0]
# skip if dependency is not installed
next unless dep_info
@@ -1181,7 +1181,7 @@ def resolve_dependencies
puts 'The following packages also need to be installed: '
@dependencies.each do |dep|
abort "Dependency #{dep} was not found.".lightred unless File.file?( File.join(CREW_PACKAGES_PATH, "#{dep}.rb") )
abort "Dependency #{dep} was not found.".lightred unless File.file?(File.join(CREW_PACKAGES_PATH, "#{dep}.rb"))
end
puts @dependencies.join(' ')
@@ -1667,7 +1667,7 @@ def build_command(args)
# Process preflight block to see if package should be built
pre_flight
if !@pkg.is_fake? && PackageUtils.compatible?(@pkg) && @pkg.source?(ARCH) && ( @pkg.no_source_build? || @pkg.source_url.to_s.upcase != 'SKIP' ) && !@pkg.no_compile_needed?
if !@pkg.is_fake? && PackageUtils.compatible?(@pkg) && @pkg.source?(ARCH) && (@pkg.no_source_build? || @pkg.source_url.to_s.upcase != 'SKIP') && !@pkg.no_compile_needed?
resolve_dependencies_and_build
else
puts 'Unable to build a fake package. Skipping build.'.lightred if @pkg.is_fake?

View File

@@ -44,4 +44,3 @@ class Command
puts "Disk usage: #{human_size(size)}".lightgreen
end
end

View File

@@ -24,7 +24,7 @@ class Command
if verbose
installed_packages['======='] = '======='
installed_packages['Package'] = 'Version'
first_col_width = installed_packages.keys.max {|a, b| a.size <=> b.size }.size
first_col_width = installed_packages.keys.max { |a, b| a.size <=> b.size }.size
installed_packages.sort.to_h.each do |package, version|
puts "#{package.ljust(first_col_width)} #{version}".lightgreen
end

View File

@@ -64,4 +64,3 @@ class Command
end
end
end

View File

@@ -19,7 +19,7 @@ class Autotools < Package
end
abort 'configure script not found!'.lightred unless File.file?('configure')
FileUtils.chmod('+x', 'configure')
if `grep -q /usr/bin/file configure`
if system('grep -q /usr/bin/file configure')
puts 'Using filefix.'.orange
system 'filefix'
end

View File

@@ -20,7 +20,7 @@ class Pip < Package
@py_pkg_deps = @py_pkg_versioned_pypi_hash['info']['requires_dist']
# We don't want extra packages listed in the dependency list, since
# those are optional.
@py_pkg_deps.reject! {|x| x.include?('extra ==')} unless @py_pkg_deps.to_s.empty?
@py_pkg_deps.reject! { |x| x.include?('extra ==') } unless @py_pkg_deps.to_s.empty?
unless @py_pkg_deps.to_s.empty?
puts "Installing #{@py_pkg} python dependencies with pip...".orange
@py_pkg_deps.each do |pip_dep|

View File

@@ -4,14 +4,14 @@ class Python < Package
property :python_build_options, :python_install_options, :python_install_extras, :no_svem
def self.build
#@required_pip_modules = %w[build installer setuptools wheel pyproject_hooks]
#@pip_list = `pip list --exclude pip`
#@required_pip_modules.each do |pip_pkg|
#unless @pip_list.include?(pip_pkg)
#puts "Installing #{pip_pkg} using pip..."
#system "MAKEFLAGS=-j#{CREW_NPROC} pip install #{pip_pkg}"
#end
#end
# @required_pip_modules = %w[build installer setuptools wheel pyproject_hooks]
# @pip_list = `pip list --exclude pip`
# @required_pip_modules.each do |pip_pkg|
# unless @pip_list.include?(pip_pkg)
# puts "Installing #{pip_pkg} using pip..."
# system "MAKEFLAGS=-j#{CREW_NPROC} pip install #{pip_pkg}"
# end
# end
if File.file?('setup.py')
puts "Python build options being used: #{PY3_SETUP_BUILD_OPTIONS} #{@python_build_options}".orange
system "MAKEFLAGS=-j#{CREW_NPROC} python3 setup.py build #{PY3_SETUP_BUILD_OPTIONS} #{@python_build_options}"

View File

@@ -13,7 +13,7 @@ class String
cyan: 36,
lightgray: 37,
# colors with bold varient available
# colors with bold varient available
gray: { normal: 90, bold: 30 },
lightred: { normal: 91, bold: 31 },
lightgreen: { normal: 92, bold: 32 },
@@ -58,4 +58,4 @@ class String
return colorize(color_code, use_bold ? 1 : 0)
end
end
end
end

View File

@@ -50,7 +50,7 @@ CREW_LIB_SUFFIX = ARCH.eql?('x86_64') && Dir.exist?('/lib64') ? '64' : ''
ARCH_LIB = "lib#{CREW_LIB_SUFFIX}"
# Glibc version can be found from the output of libc.so.6
LIBC_VERSION = ENV.fetch('LIBC_VERSION', Etc.confstr(Etc::CS_GNU_LIBC_VERSION).split.last )
LIBC_VERSION = ENV.fetch('LIBC_VERSION', Etc.confstr(Etc::CS_GNU_LIBC_VERSION).split.last)
CREW_PREFIX = ENV.fetch('CREW_PREFIX', '/usr/local')

View File

@@ -2,5 +2,5 @@ require_relative 'color'
require_relative 'const'
def crewlog(str)
caller_locations(1, 1).first.tap {|loc| puts "#{loc.path}:#{loc.lineno}:#{caller_locations(3).first.label}:#{str}".lightpurple if CREW_VERBOSE}
caller_locations(1, 1).first.tap { |loc| puts "#{loc.path}:#{loc.lineno}:#{caller_locations(3).first.label}:#{str}".lightpurple if CREW_VERBOSE }
end

View File

@@ -121,7 +121,7 @@ pkg_update_arr.each do |pkg|
end
File.write "#{CREW_CONFIG_PATH}/device.json.new", JSON.pretty_generate(JSON.parse(@device.to_json))
@device = JSON.load_file("#{CREW_CONFIG_PATH}/device.json.new", symbolize_names: true)
@device.transform_values! {|val| val.is_a?(String) ? val.to_sym : val }
@device.transform_values! { |val| val.is_a?(String) ? val.to_sym : val }
raise StandardError, 'Failed to replace pkg name...'.lightred unless @device[:installed_packages].any? { |elem| elem[:name] == pkg[:pkg_rename] }
# Ok to write working device.json
File.write "#{CREW_CONFIG_PATH}/device.json", JSON.pretty_generate(JSON.parse(@device.to_json))
@@ -134,7 +134,7 @@ pkg_update_arr.each do |pkg|
end
# Reload json file.
@device = JSON.load_file("#{CREW_CONFIG_PATH}/device.json", symbolize_names: true)
@device.transform_values! {|val| val.is_a?(String) ? val.to_sym : val }
@device.transform_values! { |val| val.is_a?(String) ? val.to_sym : val }
# Ok to remove backup and temporary json files.
FileUtils.rm_f "#{CREW_CONFIG_PATH}/device.json.bak"
FileUtils.rm_f "#{CREW_CONFIG_PATH}/device.json.new"

View File

@@ -178,7 +178,7 @@ class Package
# lambda for comparing the given range with installed version
ver_check = lambda do |installed_ver|
unless Gem::Version.new(installed_ver).send( operator.to_sym, Gem::Version.new(target_ver) )
unless Gem::Version.new(installed_ver).send(operator.to_sym, Gem::Version.new(target_ver))
# print error if the range is not fulfilled
warn <<~EOT.lightred
Package #{name} depends on '#{dep_name}' (#{operator} #{target_ver}), however version '#{installed_ver}' is currently installed :/

View File

@@ -21,7 +21,7 @@ class Selector
end
# substitute expressions in the message ("%{variable}")
@prompt = prompt.transform_values {|p| format(p, { total_opts: @options.size, default: @options[0][:value] }) }
@prompt = prompt.transform_values { |p| format(p, { total_opts: @options.size, default: @options[0][:value] }) }
end
def show_prompt

View File

@@ -54,9 +54,7 @@ class Filecmd < Package
--enable-bzlib \
--enable-xzlib \
--enable-fsect-man5 \
--disable-libseccomp" # libseccomp is disabled because
# it causes file to return "Bad system call" errors when
# not run with root.
--disable-libseccomp" # libseccomp is disabled because it causes file to return "Bad system call" errors when not run with root.
# Build a static file binary for use in case needed with glibc brokenness.
Dir.mkdir 'builddir-static'

View File

@@ -14,4 +14,3 @@ class Gcr < Package
depends_on 'gcr_3'
depends_on 'gcr_4'
end

View File

@@ -27,8 +27,8 @@ class Glibc_build223 < Package
})
def self.patch
# Patch to avoid old ld issue on glibc 2.23 by using ld configure
# portion from https://github.com/bminor/glibc/blob/master/configure
# Patch to avoid old ld issue on glibc 2.23 by using ld configure
# portion from https://github.com/bminor/glibc/blob/master/configure
@glibc_223_i686_patch = <<~'GLIBC_223_HEREDOC'
--- a/configure 2021-12-22 11:42:36.689574968 -0500
+++ b/configure 2021-12-22 11:58:43.052504544 -0500

View File

@@ -29,8 +29,8 @@ class Glibc_build227 < Package
})
def self.patch
# Patch to avoid old ld issue on glibc 2.23 by using ld configure
# portion from https://github.com/bminor/glibc/blob/master/configure
# Patch to avoid old ld issue on glibc 2.23 by using ld configure
# portion from https://github.com/bminor/glibc/blob/master/configure
@glibc_223_i686_patch = <<~'GLIBC_223_HEREDOC'
--- a/configure 2021-12-22 11:42:36.689574968 -0500
+++ b/configure 2021-12-22 11:58:43.052504544 -0500

View File

@@ -41,7 +41,7 @@ class Jdk < Package
EOT
end
options = @avail_jdk_ver.map {|majver, ver| { value: "jdk#{majver}", description: "Oracle JDK #{ver}" } }
options = @avail_jdk_ver.map { |majver, ver| { value: "jdk#{majver}", description: "Oracle JDK #{ver}" } }
depends_on Selector.new(options).show_prompt
end
end

View File

@@ -35,7 +35,7 @@ class Jdk11 < Package
EOT
end
return if File.exist?( URI(source_url).path )
return if File.exist?(URI(source_url).path)
# check if we should prompt user to the archive page or download page based on #{@_ver}
# download page only contains latest version while archive page only contains older versions
@@ -78,4 +78,4 @@ class Jdk11 < Package
# remove jdk archive after installed
FileUtils.rm_f URI(source_url).path
end
end
end

View File

@@ -40,7 +40,7 @@ class Jdk17 < Package
EOT
end
return if File.exist?( URI(source_url).path )
return if File.exist?(URI(source_url).path)
abort <<~EOT.orange
@@ -75,4 +75,4 @@ class Jdk17 < Package
# remove jdk archive after installed
FileUtils.rm_f URI(source_url).path
end
end
end

View File

@@ -34,7 +34,7 @@ class Jdk18 < Package
EOT
end
return if File.exist?( URI(source_url).path )
return if File.exist?(URI(source_url).path)
abort <<~EOT.orange
@@ -69,4 +69,4 @@ class Jdk18 < Package
# remove jdk archive after installed
FileUtils.rm_f URI(source_url).path
end
end
end

View File

@@ -58,7 +58,7 @@ class Jdk8 < Package
EOT
end
return if File.exist?( URI( source_url.key(ARCH.to_sym) ).path )
return if File.exist?(URI(source_url.key(ARCH.to_sym)).path)
# check if we should prompt user to the archive page or download page based on #{version}
# download page only contains latest version while archive page only contains older versions
@@ -98,6 +98,6 @@ class Jdk8 < Package
def self.postinstall
# remove jdk archive after installed
FileUtils.rm_f URI( source_url.key(ARCH.to_sym) ).path
FileUtils.rm_f URI(source_url.key(ARCH.to_sym)).path
end
end

View File

@@ -32,9 +32,9 @@ class Luajit_lgi < Package
system "make LUA_INCDIR=#{@lua_include} \
LUA_CFLAGS=#{@lua_cflags}"
# system "meson setup #{CREW_MESON_OPTIONS} \
#-Dlua-pc=luajit \
#-Dlua-bin=#{CREW_PREFIX}/bin/luajit \
#-Dtests=false \
# -Dlua-pc=luajit \
# -Dlua-bin=#{CREW_PREFIX}/bin/luajit \
# -Dtests=false \
# builddir"
# system 'meson configure --no-pager builddir'
# system 'mold -run samu -C builddir'

View File

@@ -93,31 +93,6 @@ class Python3 < Package
end
end
def self.check
# Chromebooks do not allow SIGHUP, so disable SIGHUP test
system 'sed', '-i', 'Lib/test/test_asyncio/test_events.py', '-e', "/Don't have SIGHUP/s/win32/linux/"
system 'sed', '-i', 'Lib/test/test_asyncio/test_subprocess.py', '-e', "/Don't have SIGHUP/s/win32/linux/"
# Chromebooks return EINVAL for F_NOTIFY DN_MULTISHOT, so disable test_fcntl_64_bit
system 'sed', '-i', 'Lib/test/test_fcntl.py', '-e', '/ARM Linux returns EINVAL for F_NOTIFY DN_MULTISHOT/s/$/\
@unittest.skipIf(platform.system() == '"'Linux'"', "Chromeos returns EINVAL for F_NOTIFY DN_MULTISHOT")/'
# imaplib test used to use a kind of security hole on a server in university and the hole is closed now.
# See https://bugs.python.org/issue30175 and https://github.com/python/cpython/pull/1320/files for details.
system 'sed', '-i', 'Lib/test/test_imaplib.py',
'-e', '/test_logincapa_with_client_certfile/i\ \ \ \ @unittest.skipIf(True,\
"bpo-30175: FIXME: cyrus.andrew.cmu.edu doesn\'t accept "\
"our randomly generated client x509 certificate anymore")',
'-e', '/test_logincapa_with_client_ssl_context/i\ \ \ \ @unittest.skipIf(True,\
"bpo-30175: FIXME: cyrus.andrew.cmu.edu doesn\'t accept "\
"our randomly generated client x509 certificate anymore")'
# Using /tmp breaks test_distutils, test_subprocess
# Proxy setting breaks test_httpservers, test_ssl,
# test_urllib, test_urllib2, test_urllib2_localnet
# system "http_proxy= https_proxy= ftp_proxy= make test"
end
def self.install
Dir.chdir 'builddir' do
system 'make', "DESTDIR=#{CREW_DEST_DIR}", 'install'

View File

@@ -9,8 +9,7 @@ class Ruby_rubocop < RUBY
version '1.63.2-ruby-3.3'
license 'MIT'
compatibility 'all'
source_url 'https://github.com/chromebrew/chromebrew/raw/master/.rubocop.yml'
source_sha256 '38f6c68465c7224bdd7bcb1b37f7b49283f46def71dc56553069e749afab4b85'
source_url 'SKIP'
depends_on 'libyaml'
depends_on 'xdg_base'
@@ -18,14 +17,4 @@ class Ruby_rubocop < RUBY
conflicts_ok
no_compile_needed
no_fhs
ruby_install_extras <<~EOF
FileUtils.mkdir_p "#{CREW_DEST_PREFIX}/.config/rubocop/" if Gem::Version.new(RUBY_VERSION.to_s) < Gem::Version.new('3.3')
FileUtils.install '.rubocop.yml', "#{CREW_DEST_PREFIX}/.config/rubocop/config.yml", mode: 0o644
EOF
def self.postinstall
puts "Chromebrew rubocop config file was installed at #{CREW_PREFIX}/.config/rubocop/config.yml".lightblue
puts 'This can be overridden by a ~/.rubocop.yml'.lightblue
end
end

View File

@@ -40,10 +40,10 @@ def main(pkg)
puts "Checking for the runtime dependencies of #{pkg}..."
if @opt_use_crew_dest_dir
define_singleton_method('pkgfilelist') {File.join(CREW_DEST_DIR, 'filelist')}
define_singleton_method('pkgfilelist') { File.join(CREW_DEST_DIR, 'filelist') }
abort('Pkg was not built.') unless File.exist?(pkgfilelist)
else
define_singleton_method('pkgfilelist') {"#{CREW_PREFIX}/etc/crew/meta/#{pkg}.filelist"}
define_singleton_method('pkgfilelist') { "#{CREW_PREFIX}/etc/crew/meta/#{pkg}.filelist" }
# Package needs to be installed for package filelist to be populated.
unless File.exist?(pkgfilelist)
puts "Installing #{pkg} because it is not installed."
@@ -81,7 +81,7 @@ def main(pkg)
# Look at files in CREW_DEST_DIR instead of assuming the package is
# normally installed, which lets us avoid installing the package if it
# was just built.
pkgfiles.map! {|item| item.prepend(CREW_DEST_DIR)} if @opt_use_crew_dest_dir
pkgfiles.map! { |item| item.prepend(CREW_DEST_DIR) } if @opt_use_crew_dest_dir
FileUtils.rm_rf("/tmp/deps/#{pkg}")
# Remove files we don't care about, such as man files and non-binaries.
@@ -101,7 +101,7 @@ def main(pkg)
# Figure out which Chromebrew packages provide the relevant deps.
pkgdeps = pkgdepsfiles.map do |file|
whatprovidesfxn(file, pkg)
end.sort.reject { |i| i.include?(pkg) }.map { |i| i.split("\n")}.flatten.uniq
end.sort.reject { |i| i.include?(pkg) }.map { |i| i.split("\n") }.flatten.uniq
# Massage the glibc entries in the dependency list.
pkgdeps = pkgdeps.map { |i| i.gsub(/glibc_build.*/, 'glibc') }.uniq
@@ -126,10 +126,10 @@ def main(pkg)
missingpkgdeps.each do |adddep|
puts "\n Adding deps: #{adddep}"
gawk_cmd = if File.foreach("#{CREW_PREFIX}/lib/crew/packages/#{pkg}.rb").grep(/depends_on/).any?
# This files contains dependencies already, so add new deps after existing dependencies.
# This files contains dependencies already, so add new deps after existing dependencies.
"gawk -i inplace -v dep=\" depends_on '#{adddep}' # R\" 'FNR==NR{ if (/depends_on/) p=NR; next} 1; FNR==p{ print dep }' #{CREW_PREFIX}/lib/crew/packages/#{pkg}.rb #{CREW_PREFIX}/lib/crew/packages/#{pkg}.rb"
else
# This files doesn't contain deps, so just add new deps.
# This files doesn't contain deps, so just add new deps.
"gawk -i inplace -v dep=\" depends_on '#{adddep}' # R\" 'FNR==NR{ if (/})/) p=NR; next} 1; FNR==p{ print \"\\n\" dep }' #{CREW_PREFIX}/lib/crew/packages/#{pkg}.rb #{CREW_PREFIX}/lib/crew/packages/#{pkg}.rb"
end
system(gawk_cmd)