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

Cache does not depend on content of Cops #2886

Closed
ptarjan opened this issue Feb 24, 2016 · 10 comments
Closed

Cache does not depend on content of Cops #2886

ptarjan opened this issue Feb 24, 2016 · 10 comments

Comments

@ptarjan
Copy link
Contributor

ptarjan commented Feb 24, 2016

If you run rubocop on a file, then change a cop to do something different, then re-run rubocop it will just return the previous result. -C false fixes it, but that is pretty bizarre and unexpected for cop authors. Even worse, cop changes in CI will continue running old versions until the files are touched.

I don't know what the current cache key is, but can we include a SHA of the cops that run too in there?

@rrosenblum
Copy link
Contributor

Can you elaborate on this a little? If you change the configuration in the .rubocop.yml file, the that should bust the cache.

I did encounter a similar, possibly the same, issue recently with a custom cop. If you modify the contents of a custom cop. RuboCop doesn't seem to account for the contents of custom cops.

@ptarjan
Copy link
Contributor Author

ptarjan commented Feb 24, 2016

Yes the problem is with custom cops changing and the cache not noticing.

@alexdowad
Copy link
Contributor

@ptarjan Are you talking about changing the source code of a custom com, or the YAML configuration?

All files in the entire RC source directory are hashed and those hashes are mixed in to the overall cache key, so changes to the RC source itself bust the cache. But if the source code for your custom cop is somewhere else, it wouldn't go into the cache key.

@ptarjan
Copy link
Contributor Author

ptarjan commented Feb 25, 2016

I have a top level 'require:' in the rubocop.yml. That file has many require_relative for ruby files which have the actual cops themselves. If there are any changes to the included Cops, they don't bust the cache.

Paul

Sent from my iPhone

On Feb 25, 2016, at 11:39 AM, Alex Dowad [email protected] wrote:

@ptarjan Are you talking about changing the source code of a custom com, or the YAML configuration?

All files in the entire RC source directory are hashed and those hashes are mixed in to the overall cache key, so changes to the RC source itself bust the cache. But if the source code for your custom cop is somewhere else, it wouldn't go into the cache key.


Reply to this email directly or view it on GitHub.

@alexdowad
Copy link
Contributor

If there are any changes...

Please be clear; you are talking about changes to their source code, not configuration. Right?

@ptarjan
Copy link
Contributor Author

ptarjan commented Feb 25, 2016

Correct. Source code changes. Nothing changed in the .yml files.

We wrote a new Cop, added it to the main 'require'd file, and out CI said it passed on the whole codebase. It turned out that it was just using the cached values without the new Cop. Then we tried to fix the Cop and those changes weren't noticed either.

So changes to the required file and changes to the custom Cop didn't bust the cache.

Sent from my iPhone

On Feb 25, 2016, at 11:50 AM, Alex Dowad [email protected] wrote:

If there are any changes...

Please be clear; you are talking about changes to their source code, not configuration. Right?


Reply to this email directly or view it on GitHub.

@alexdowad
Copy link
Contributor

Hmm.

So do we automatically turn off caching if any custom cop is being used... or do we try to find and read its source code, hash it, and mix it into the cache key?

@ptarjan
Copy link
Contributor Author

ptarjan commented Feb 25, 2016

I would prefer the latter. You can cut some corners like not allowing require in the top level file so you don't have to chase down a require hierarchy. As long as an error is emitted for that it would work for us to move all the requires to be in the top level yml.

Sent from my iPhone

On Feb 25, 2016, at 12:07 PM, Alex Dowad [email protected] wrote:

Hmm.

So do we automatically turn off caching if any custom cop is being used... or do we try to find and read its source code, hash it, and mix it into the cache key?


Reply to this email directly or view it on GitHub.

@doy
Copy link

doy commented Feb 25, 2016

Another option could just be to require any custom cops to also specify versions, and hash those together with the overall rubocop version. It's a bit less pleasing than figuring it out automatically, but having a manual escape valve for this in an obvious place seems like it'd be a good thing too.

@jonas054
Copy link
Collaborator

I think $LOADED_FEATURES will contain all files we're currently including in the checksum of the RuboCop source, except bin/rubocop, plus all custom cops. So we can use that variable instead of searching through the lib directory tree.

https://github.com/bbatsov/rubocop/blob/v0.37.2/lib/rubocop/result_cache.rb#L125

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants