From dfdf8f924d5f47da03dfb0a7bc48a2690661f329 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Sun, 6 Sep 2020 08:00:12 +0200 Subject: [PATCH] Make it possible to configure the cache path via a CLI option or an env variable (#8639) This commit introduces the --cache-root command line option and support for the RUBOCOP_CACHE_ROOT env variable to control the cache root directory. It takes precedence over everything else (even the config file) which makes it perfectly suited for CI usage where it's very helpful to control the cache path (without interference from the config file) to consistently be able to save and restore rubocop's cache files. --- CHANGELOG.md | 5 +++++ config/default.yml | 2 ++ docs/modules/ROOT/pages/usage/caching.adoc | 21 +++++++++++------ lib/rubocop/options.rb | 20 ++++++++++++++++- lib/rubocop/result_cache.rb | 6 ++++- lib/rubocop/runner.rb | 5 ++++- spec/rubocop/options_spec.rb | 26 +++++++++++++++++++--- spec/rubocop/result_cache_spec.rb | 26 ++++++++++++++++++++++ 8 files changed, 98 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a0743aeea4..1ce6cd87336 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### New features + +* New option `--cache-root` and support for the `RUBOCOP_CACHE_ROOT` environment variable. Both can be used to override the `AllCops: CacheRootDirectory` config, especially in a CI setting. ([@sascha-wolf][]) + ### Bug fixes * [#8627](https://github.com/rubocop-hq/rubocop/issues/8627): Fix a false positive for `Lint/DuplicateRequire` when same feature argument but different require method. ([@koic][]) @@ -4837,3 +4841,4 @@ [@Lykos]: https://github.com/Lykos [@jaimerave]: https://github.com/jaimerave [@Skipants]: https://github.com/Skipants +[@sascha-wolf]: https://github.com/sascha-wolf diff --git a/config/default.yml b/config/default.yml index 71e4a20a5e6..ddcaa3ddc05 100644 --- a/config/default.yml +++ b/config/default.yml @@ -117,6 +117,8 @@ AllCops: # CacheRootDirectory is ~ (nil), which it is by default, the root will be # taken from the environment variable `$XDG_CACHE_HOME` if it is set, or if # `$XDG_CACHE_HOME` is not set, it will be `$HOME/.cache/`. + # The CacheRootDirectory can be overwritten by passing the `--cache-root` command + # line option or by setting `$RUBOCOP_CACHE_ROOT` environment variable. CacheRootDirectory: ~ # It is possible for a malicious user to know the location of RuboCop's cache # directory by looking at CacheRootDirectory, and create a symlink in its diff --git a/docs/modules/ROOT/pages/usage/caching.adoc b/docs/modules/ROOT/pages/usage/caching.adoc index a9afb4e92b2..6269ca8de44 100644 --- a/docs/modules/ROOT/pages/usage/caching.adoc +++ b/docs/modules/ROOT/pages/usage/caching.adoc @@ -33,13 +33,20 @@ overrides the setting. By default, the cache is stored in either `$XDG_CACHE_HOME/$UID/rubocop_cache` if `$XDG_CACHE_HOME` is set or in -`$HOME/.cache/rubocop_cache/` if it's not. The configuration parameter -`AllCops: CacheRootDirectory` can be used to set the root to a -different path. One reason to use this option could be that there's a -network disk where users on different machines want to have a common -RuboCop cache. Another could be that a Continuous Integration system -allows directories, but not a temporary directory, to be saved between -runs. +`$HOME/.cache/rubocop_cache/` if it's not. + +The root can be set to a different path in a number of ways (from +**highest** precedence to **lowest**): + +* the `--cache-root` command line option +* the `$RUBOCOP_CACHE_ROOT` environment variable +* the `AllCops: CacheRootDirectory` configuration parameter + +One reason to set the cache root could be that there's a network disk +where users on different machines want to have a common RuboCop cache. +Another could be that a Continuous Integration system allows +directories, but not a temporary directory, to be saved between runs, +or that the system caches certain folders by default. == Cache Pruning diff --git a/lib/rubocop/options.rb b/lib/rubocop/options.rb index 77b1a08530b..cecfd6ac64b 100644 --- a/lib/rubocop/options.rb +++ b/lib/rubocop/options.rb @@ -69,6 +69,7 @@ def define_options add_severity_option(opts) add_flags_with_optional_args(opts) + add_cache_options(opts) add_boolean_flags(opts) add_aliases(opts) @@ -164,10 +165,16 @@ def add_flags_with_optional_args(opts) end end + def add_cache_options(opts) + option(opts, '-C', '--cache FLAG') + option(opts, '--cache-root DIR') do + @validator.validate_cache_enabled_for_cache_root + end + end + # rubocop:disable Metrics/MethodLength, Metrics/AbcSize def add_boolean_flags(opts) option(opts, '-F', '--fail-fast') - option(opts, '-C', '--cache FLAG') option(opts, '-d', '--debug') option(opts, '-D', '--[no-]display-cop-names') option(opts, '-E', '--extra-details') @@ -392,6 +399,13 @@ def validate_exclude_limit_option # of option order. raise OptionParser::MissingArgument end + + def validate_cache_enabled_for_cache_root + return unless @options[:cache] == 'false' + + raise OptionArgumentError, '--cache-root can not be used with ' \ + '--cache false' + end end # This module contains help texts for command line options. @@ -464,6 +478,10 @@ module OptionsHelp cache: ["Use result caching (FLAG=true) or don't", '(FLAG=false), default determined by', 'configuration parameter AllCops: UseCache.'], + cache_root: ['Set the cache root directory.', + 'Takes precedence over the configuration', + 'parameter AllCops: CacheRootDirectory and', + 'the $RUBOCOP_CACHE_ROOT environment variable.'], debug: 'Display debug info.', display_cop_names: ['Display cop names in offense messages.', 'Default is true.'], diff --git a/lib/rubocop/result_cache.rb b/lib/rubocop/result_cache.rb index c72229cf0a5..1f85d9b59cd 100644 --- a/lib/rubocop/result_cache.rb +++ b/lib/rubocop/result_cache.rb @@ -67,7 +67,8 @@ def remove_files(files, dirs, remove_count) end def self.cache_root(config_store) - root = config_store.for_pwd.for_all_cops['CacheRootDirectory'] + root = ENV['RUBOCOP_CACHE_ROOT'] + root ||= config_store.for_pwd.for_all_cops['CacheRootDirectory'] root ||= if ENV.key?('XDG_CACHE_HOME') # Include user ID in the path to make sure the user has write # access. @@ -82,7 +83,10 @@ def self.allow_symlinks_in_cache_location?(config_store) config_store.for_pwd.for_all_cops['AllowSymlinksInCacheRootDirectory'] end + attr :path + def initialize(file, team, options, config_store, cache_root = nil) + cache_root ||= options[:cache_root] cache_root ||= ResultCache.cache_root(config_store) @allow_symlinks_in_cache_location = ResultCache.allow_symlinks_in_cache_location?(config_store) diff --git a/lib/rubocop/runner.rb b/lib/rubocop/runner.rb index c053155a6a4..44b23ee246b 100644 --- a/lib/rubocop/runner.rb +++ b/lib/rubocop/runner.rb @@ -81,7 +81,10 @@ def inspect_files(files) # OPTIMIZE: Calling `ResultCache.cleanup` takes time. This optimization # mainly targets editors that integrates RuboCop. When RuboCop is run # by an editor, it should be inspecting only one file. - ResultCache.cleanup(@config_store, @options[:debug]) if files.size > 1 && cached_run? + if files.size > 1 && cached_run? + ResultCache.cleanup(@config_store, @options[:debug], @options[:cache_root]) + end + formatter_set.finished(inspected_files.freeze) formatter_set.close_output_files end diff --git a/spec/rubocop/options_spec.rb b/spec/rubocop/options_spec.rb index b3cbb37750e..3b5bed02115 100644 --- a/spec/rubocop/options_spec.rb +++ b/spec/rubocop/options_spec.rb @@ -103,12 +103,16 @@ def abs(path) --show-cops [COP1,COP2,...] Shows the given cops, or all cops by default, and their configurations for the current directory. - -F, --fail-fast Inspect files in order of modification - time and stop after the first file - containing offenses. -C, --cache FLAG Use result caching (FLAG=true) or don't (FLAG=false), default determined by configuration parameter AllCops: UseCache. + --cache-root DIR Set the cache root directory. + Takes precedence over the configuration + parameter AllCops: CacheRootDirectory and + the $RUBOCOP_CACHE_ROOT environment variable. + -F, --fail-fast Inspect files in order of modification + time and stop after the first file + containing offenses. -d, --debug Display debug info. -D, --[no-]display-cop-names Display cop names in offense messages. Default is true. @@ -297,6 +301,22 @@ def abs(path) end end + describe '--cache-root' do + it 'fails if no argument is given' do + expect { options.parse %w[--cache-root] } + .to raise_error(OptionParser::MissingArgument) + end + + it 'fails if also `--cache false` is given' do + expect { options.parse %w[--cache false --cache-root /some/dir] } + .to raise_error(RuboCop::OptionArgumentError) + end + + it 'accepts a path as argument' do + expect { options.parse %w[--cache-root /some/dir] }.not_to raise_error + end + end + describe '--disable-uncorrectable' do it 'accepts together with --auto-correct' do expect { options.parse %w[--auto-correct --disable-uncorrectable] } diff --git a/spec/rubocop/result_cache_spec.rb b/spec/rubocop/result_cache_spec.rb index ece925173e2..aabed8deceb 100644 --- a/spec/rubocop/result_cache_spec.rb +++ b/spec/rubocop/result_cache_spec.rb @@ -212,6 +212,16 @@ def abs(path) end end + context 'when --cache-root is given' do + it 'takes the cache_root from the options' do + cache2 = described_class.new(file, team, + { cache_root: 'some/path' }, + config_store) + + expect(cache2.path).to start_with('some/path') + end + end + context 'when --format is given' do let(:options2) { { format: 'simple' } } @@ -382,6 +392,22 @@ def abs(path) cacheroot = described_class.cache_root(config_store) expect(cacheroot).to eq(File.join('/opt', 'rubocop_cache')) end + + context 'and RUBOCOP_CACHE_ROOT is set' do + around do |example| + ENV['RUBOCOP_CACHE_ROOT'] = '/tmp/cache-from-env' + begin + example.run + ensure + ENV.delete('RUBOCOP_CACHE_ROOT') + end + end + + it 'contains the root from RUBOCOP_CACHE_ROOT' do + cacheroot = described_class.cache_root(config_store) + expect(cacheroot).to eq(File.join('/tmp/cache-from-env', 'rubocop_cache')) + end + end end end end