[Merged by Bors] - feat(Cache): extend argument-parsing to allow module names and file names#21822
[Merged by Bors] - feat(Cache): extend argument-parsing to allow module names and file names#21822joneugster wants to merge 52 commits intomasterfrom
Conversation
…rdcoded_package_dirs
…hardcoded_package_dirs
PR summary 4c5c94fd27Import 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
|
Co-authored-by: Eric Wieser <[email protected]>
…e_hardcoded_package_dirs
…rdcoded_package_dirs
…hardcoded_package_dirs
…er/cache_parse_args
|
This PR/issue depends on: |
|
|
||
| Note: An argument like `Archive` is treated as module, not a path. | ||
| -/ | ||
| def leanModulesFromSpec (sp : SearchPath) (argₛ : String) : |
There was a problem hiding this comment.
Arguably this is ill-defined; is Foo.Bar.lean a reference to ./Foo.Bar.lean or Foo/Bar/lean.lean?
There was a problem hiding this comment.
(maybe the answer is to allow a leading ./ to disambiguate)
There was a problem hiding this comment.
currently it's a reference to ./Foo.Bar.lean especially since Mathlib.lean should be parsed that way. One add ./ but requiring it would not work for Mathlib.lean
Would it be desired to just try out both options with something like to following?
if arg.extension == "lean" then
let mut mods := #[]
-- try if a file `Foo/Bar/lean.lean` exists
let mod := argₛ.toName
let sourceFile ← Lean.findLean sp mod
if ← sourceFile.pathExists then
mods := mods.append #[(mod, sourceFile)]
-- try if a file `./Foo.Bar.lean` exists
let mod₂ : Name := arg.withExtension "" |>.components.foldl .str .anonymous
if (← arg.pathExists) then
mods := mods.append #[(mod₂, arg)]
return .ok modsThere was a problem hiding this comment.
Arguably this is ill-defined; is
Foo.Bar.leana reference to./Foo.Bar.leanorFoo/Bar/lean.lean?
Alternatively, just assume that no one is going to do this. If they do, they can file a bug report, and we can tell them to stop being annoying. :-)
There was a problem hiding this comment.
At any rate we should make sure what we implement is consistent with whatever lean4#2756 decides.
joneugster
left a comment
There was a problem hiding this comment.
Few comments to provide context
|
bors merge |
…ames (#21822) Accept both `Mathlib.Init` and `Mathlib/Init.lean` as arguments. Co-authored-by: Eric Wieser <[email protected]>
|
Pull request successfully merged into master. Build succeeded: |
Accept both
Mathlib.InitandMathlib/Init.leanas arguments.Note: I noticed a bug in walking directories which will not be present after #21834, so I put an error message "not implemented yet" here and will add the feature in the PR following #21834
getFileImportsto use search path and return module names and source file location #21815