-
Notifications
You must be signed in to change notification settings - Fork 355
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
feat(Cache): add miniget
command, a minimal version of get
#20568
base: master
Are you sure you want to change the base?
Conversation
This downloads only cache files for every Lean file in some subdirectory.
PR summary 6373327380Import changes for modified filesNo significant changes to the import graph Import changes for all files
Declarations diff
You can run this locally as follows## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>
## more verbose report:
./scripts/declarations_diff.sh long <optional_commit> The doc-module for No changes to technical debt.You can run this locally as
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just change get
to behave in this way?
miniget
commandget
only download the cache for every file imported from a .lean
file
Good idea! I have changed |
Cache/Main.lean
Outdated
If no arguments are given, 'get', 'get!' and 'get-' download information about all files imported | ||
in some .lean file in the current directory or a subdirectory thereof (ignoring .lake folders). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this the same as lake exe cache get .
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question! Running this in sphere-eversion (i.e., a project downstream of mathlib) yields uncaught exception: Unknown package directory for .
- so I would say no :-)
By the way: running this in |
This step adds about a second to my I think we should disable it if we're in Mathlib. |
Fair point. I don't think Cache knows about the current workspace, though - so don't see how to easily disable this. Perhaps it's better to make this a separate command, after all? What do you think? |
@@ -83,8 +102,22 @@ def main (args : List String) : IO Unit := do | |||
let goodCurl ← pure !curlArgs.contains (args.headD "") <||> validateCurl | |||
if leanTarArgs.contains (args.headD "") then validateLeanTar | |||
let get (args : List String) (force := false) (decompress := true) := do | |||
let hashMap ← if args.isEmpty then pure hashMap else hashMemo.filterByFilePaths (toPaths args) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't hashMemo
already contain the dependency information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It only contains this for all files it has memoized this for. This is mathlib and all extraRoots
(i.e., arguments passed to it). Running just lake exe cache get
in a dependent project does not parse the files in the downstream project.
Thinking about this again and having discussed with @fpvandoorn, it seems better to not change the default behaviour of cache, and make this a separate command instead. I'll re-instance |
get
only download the cache for every file imported from a .lean
fileminiget
command, a minimal version of get
Add a
miniget
command, a more minimal version of theget
command. Instead of downloading the entire mathlib cache, it finds all .lean files in the current directory (excluding .lake directorires), reads all "import Mathlib X.Y.Z" lines in them, and runslake exe cache get X/Y/Z.lean
for these lines In short, this just downloads the cache necessary for a certain project.Inspired by a zulip comment of
kim-em
.A (hopefully less controversial) part of #20567.