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

feat: detect default target on runLinter #811

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

austinletson
Copy link
Contributor

@austinletson austinletson commented May 26, 2024

Now runLinter will try to detect all root modules of the default build targets of type lean_exe or lean_lib in Lake's environment using the same resolveDefaultPackageTarget function as lake build.

Closes #795

@austinletson austinletson force-pushed the runLinter-default-module-detection branch from 128dc05 to 05c6ecd Compare May 26, 2024 13:55
Previously, the default behavior for `runLinter` was to lint
`Batteries`.
Now `runLinter` will try to detect the either
the first lean_exe or first lean_lib
in Lake's environment which is also a default_target
@austinletson austinletson force-pushed the runLinter-default-module-detection branch from 05c6ecd to a9fd06a Compare May 26, 2024 13:56
@austinletson austinletson changed the title feat: detect the first default target lean lib on runLinter feat: detect default target on runLinter May 26, 2024
scripts/runLinter.lean Outdated Show resolved Hide resolved
@austinletson
Copy link
Contributor Author

awaiting-review

@github-actions github-actions bot added the awaiting-review This PR is ready for review; the author thinks it is ready to be merged. label May 29, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Jul 17, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Jul 24, 2024
@kim-em
Copy link
Collaborator

kim-em commented Jul 26, 2024

I'm a bit worried about adding a Lake import after seeing the troubles described at leanprover-community/import-graph#20

@austinletson
Copy link
Contributor Author

austinletson commented Jul 29, 2024

I'm a bit worried about adding a Lake import after seeing the troubles described at leanprover-community/import-graph#20

Hmm, this does seem like it could cause the same issue. Is there an example of a safe way to access Lake's internal API? I based this PR on this zulip thread about accessing Lake dependencies in for Mathlibs lake exe mk_all.

Another thought, does the fact that runLinter.lean is built separately from the rest of std as a lean_exe help us out at all?

@austinletson
Copy link
Contributor Author

austinletson commented Jul 29, 2024

Related to this, I created a Zulip thread asking about a Lake command that would provide module and dependency information so we wouldn't have to use Lake's internals in cases such as these. (link)

@tydeu
Copy link
Contributor

tydeu commented Jul 30, 2024

@semorrison

I'm a bit worried about adding a Lake import after seeing the troubles described at leanprover-community/import-graph#20

The problem there just appears to be that a shared library for Lake is not included in the currently Lean distro. That seems like it could be remedied relatively easily if necessary.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Aug 5, 2024
@kim-em
Copy link
Collaborator

kim-em commented Aug 5, 2024

@semorrison

I'm a bit worried about adding a Lake import after seeing the troubles described at leanprover-community/import-graph#20

The problem there just appears to be that a shared library for Lake is not included in the currently Lean distro. That seems like it could be remedied relatively easily if necessary.

@tydeu, yes please.

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. merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

runLinter should determine the target module by asking Lake
5 participants