Conversation
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinuxWindows |
|
I forgot to remove the (old) cache initialization: ruff/crates/ruff_cli/src/commands/run.rs Lines 47 to 65 in 5526699 |
8c3fa7a to
d96d998
Compare
|
Can you add an example on how to use Ruff in CI, e.g. together with the github cache action? Would it make sense to automatically detect CI and then use a different directory? |
Falling back to the old .ruff_cache directory in the current directory if the global cache directory can't be determined. For reference a number of other Python tools also use global caches (by default): * Black: https://github.com/psf/black/blob/35722dff623f3cdf5018e3f1183cd4e02e91caa8/src/black/cache.py#L21-L34. * Pip: https://github.com/pypa/pip/blob/8a1eea4aaedb1fb1c6b4c652cd0c43502f05ff37/src/pip/_internal/locations/base.py#L13, https://github.com/pypa/pip/blob/8a1eea4aaedb1fb1c6b4c652cd0c43502f05ff37/src/pip/_internal/utils/appdirs.py#L16. * Pylint: https://github.com/pylint-dev/pylint/blob/507dfc5ebd0cd9712ac4840cae3bea8623206554/doc/faq.rst#where-is-the-persistent-data-stored-to-compare-between-successive-runs.
This avoids an allocation in the code and in the usage of global the sub directory isn't really needed.
Uses ensure_tag of the cachedir crate instead of doing the check ourselves. Also uses std::fs::write to write the .gitignore file.
Not relevant to global cache directories.
10ef4d6 to
6e62323
Compare
When running on the CI you probably want to se the
I'm not sure if we can consistently detect this, but even if we could than we have the problem what directory to use. I don't think GitHub Actions has a default directory that is always cache, I think you need to explicitly set this. Maybe an example GitHub Actions config file can tackle both issues. |
|
@MichaReiser I believe you were still on the fence on whether or not a global cache directory would be an improvement, have you made a decision on this? |
Thanks for pinging me. Not really. My concern is that this change introduces additional complexity and potentially breaks the workflows of users and, thus, the cost of the change may be too height. My concerns are:
|
|
But... this is not a me decision. If the team is in favor then go for it. |
The above two concerns are valid. Don't really have a good suggestion to resolve these.
We could recommend to set
Maybe you can discuss this within the team? I think I can still merge this but, I think consensus is important. |
Will do. I'll bring it up in our next standup. |
We should probably just provide a GitHub Action with reasonable defaults anyway. |
|
I brought this PR up in today's standup, and we decided not to merge this as of today because moving to a global cache directory only provides limited benefits in our view and comes with a few downsides. This isn't saying that we don't want to move to a global cache directory eventually. It's just that we want to have more time to consider all the tradeoffs to decide this for good. Thanks again @Thomasdezeeuw for working on this and all your caching improvements. |
Summary
This is based on #5117.
Updates #1292.
Falling back to the old .ruff_cache directory in the current directory
if the global cache directory can't be determined.
For reference a number of other Python tools also use global caches (by
default):
Python projects that don't:
__pycache__).Other tools:
Test Plan
Existing tests should keep working.