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

chore: adjustments to runLinter so it can be used downstream #370

Merged
merged 2 commits into from
Nov 17, 2023
Merged

Conversation

kim-em
Copy link
Collaborator

@kim-em kim-em commented Nov 17, 2023

No description provided.

@kim-em kim-em added the awaiting-review This PR is ready for review; the author thinks it is ready to be merged. label Nov 17, 2023
@digama0
Copy link
Member

digama0 commented Nov 17, 2023

Is there anything we can do to make the default module be the current package instead of Std always? Otherwise I think this will have to be called as lake exe runLinter Mathlib from mathlib.

@kim-em
Copy link
Collaborator Author

kim-em commented Nov 17, 2023

Is there anything we can do to make the default module be the current package instead of Std always? Otherwise I think this will have to be called as lake exe runLinter Mathlib from mathlib.

@tydeu, any suggestions?

leanprover-community/mathlib4#8461 is already using lake exe runLinter Mathlib.

@digama0
Copy link
Member

digama0 commented Nov 17, 2023

I gave this a bit of a think and I think there is nothing that can be done with the current design of lake, unless the linter is turned into a script (which both changes the interface, and also makes it run interpreted). Options considered:

  • Influence elaboration somehow so that the name of the current project is embedded in the executable.
    • We can't do this right now because we have no way to influence elaboration other than through builtin options. It would be nice to have a way to declare defs that are reachable from elaboration, similar to autogenerated files like version.h in lean4 repo.
  • Call lake to get the name of the default_target(s) and the roots for it.
    • Can't do it because we didn't get lake dump-config.
  • Have lake pass us this information in env vars.
    • Lake doesn't tell us much other than the LEAN_SRC_PATH and other things tailored for lean use. It doesn't even say what the current project folder is.
  • Add extra hard-coded CLI args to the lake_exe in the lakefile.
    • This seems relatively simple to implement, but lake doesn't do it currently. I'll try to make a PR for this.

Given this, I'll just merge it as-is and we can pursue alternatives in a future PR.

@digama0 digama0 merged commit 385b5f9 into main Nov 17, 2023
@digama0 digama0 deleted the runLinter branch November 17, 2023 07:22
@tydeu
Copy link
Collaborator

tydeu commented Nov 17, 2023

While I think many of @digama0's feature suggestions are generally reasonable (so feel free to create issues for them!), lake exe runLinter Mathlib looks quite good to me! Nonetheless, one way to achieve a shorter command in the current setup would be a have a lint script that just builds and runs the linter on Mathlib (i.e., in a similar manner to the exe CLI command).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review This PR is ready for review; the author thinks it is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants