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: support Lake for building Lean core oleans #3886

Merged
merged 16 commits into from
Jun 13, 2024

Conversation

david-christiansen
Copy link
Contributor

@david-christiansen david-christiansen commented Apr 12, 2024

This is from a pairtriple programming session with @tydeu and @mhuisi.

If stage 1 is built with -DUSE_LAKE=ON, the CMake run will generate lakefile.toml files for the root, src, and tests. These Lake configuration files can then be used to build core oleans. While they do not yet allow Lake to be used to build the Lean binaries. they do allow Lake to be used for working interactively with the Lean source. In our preliminary experiments, this allowed updates to Init.Data.Nat to be noticed automatically when reloading downstream files, rather than requiring a full manual compiler rebuild. This will make it easier to work on the system.

As part of this change, Lake is added to stage 0. This allows Lake to function in src, which uses the stage 0 toolchain.

Add lakefile.toml in the right places so that working interactively on
the Lean repository will have the same features WRT the language
server as ordinary Lean projects.

This also requires adding Lake to stage0.
(doesn't work yet, but we ran out of time pair programming)
@tydeu tydeu self-assigned this Apr 16, 2024
@Kha
Copy link
Member

Kha commented Apr 17, 2024

The lakefile.toml will have to be generated by CMake to correctly heed LEAN_MAKE_OPTS/LEANC_OPTS/...

@tydeu
Copy link
Member

tydeu commented Apr 17, 2024

@Kha Right now, this is just for interactive editing, not for building the binaries or shared libraries, so we do not need those options at the moment.

@Kha
Copy link
Member

Kha commented Apr 17, 2024

Okay, I would not have expected that to be a desirable intermediate state if it forces me to redo the build in cmake whenever I want to actually run and test Lean. It also begs the question of how to opt into the current workflow when the lakefile.toml always exists.

@tydeu
Copy link
Member

tydeu commented Apr 17, 2024

@Kha

Okay, I would not have expected that to be a desirable intermediate state if it forces me to redo the build in cmake whenever I want to actually run and test Lean.

While it does require an additional run of CMake to run/test Lean, it does not require redoing work, as Make can successfully make use of elaboration results (as there are no relevant LEAN_OPTS to standard dev builds).

@tydeu tydeu changed the title WIP: Support Lake for building Lean source feat: support Lake for building Lean core oleans May 2, 2024
@tydeu tydeu added the full-ci label May 2, 2024
@tydeu
Copy link
Member

tydeu commented May 2, 2024

I am flummoxed by the linker errors in the CMake build of stage0 Lake. It builds successfully on my machine.

@tydeu tydeu marked this pull request as ready for review May 3, 2024 08:35
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label May 3, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented May 3, 2024

Mathlib CI status (docs):

  • ❗ Std/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 092ca8530a6f272b7cb19235750479cdffab11de --onto b470eb522bfd68ca96938c23f6a1bce79da8a99f. (2024-05-03 08:38:43)
  • ❗ Std/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 092ca8530a6f272b7cb19235750479cdffab11de --onto 00cf5771f31e7de7f6ab3089cb4b9ffa606a35ed. (2024-05-03 19:35:36)
  • ✅ Mathlib branch lean-pr-testing-3886 has successfully built against this PR. (2024-06-11 02:53:48) View Log
  • ✅ Mathlib branch lean-pr-testing-3886 has successfully built against this PR. (2024-06-11 09:44:23) View Log
  • ✅ Mathlib branch lean-pr-testing-3886 has successfully built against this PR. (2024-06-13 17:26:42) View Log

@Kha Kha removed the full-ci label May 24, 2024
@Kha
Copy link
Member

Kha commented Jun 2, 2024

I added a -DUSE_LAKE=ON CMake flag to opt into the new behavior and to fix the hard-coded path so if that looks good and the stage0 update is removed to be redone on master, I think this PR is ready to be merged.

It wasn't clear to me why the additional lakefile.tomls in the root and in tests/ are necessary, so I've removed them for now.

@tydeu
Copy link
Member

tydeu commented Jun 3, 2024

@Kha

It wasn't clear to me why the additional lakefile.tomls in the root and in tests/ are necessary, so I've removed them for now.

They are necessary for the multiple roots of the VS Code workspace. Each root needs to have a lakefile for the server to make use of Lake at that root. Without them, Lake will not be used for Lean files in the root (outside src/) or in tests/.

@Kha
Copy link
Member

Kha commented Jun 4, 2024

Why is that relevant? tests/ is not a Lake project, even running the full build from src/ in it does not help as it will not actually build the executables. I'd rather have it not to work at all than stop half-way to a successful build.

@tydeu
Copy link
Member

tydeu commented Jun 4, 2024

@Kha

Why is that relevant? tests/ is not a Lake project, even running the full build from src/ in it does not help as it will not actually build the executables.

It is not for building (e.g., the executables). It is for interactively editing the tests using the modules built from src (via Lake in the server). For example, when we pair programming this at the retreat, we added a definition in Init.Data.Nat and the immediately saw the change propagate to a test file in tests.

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jun 11, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 11, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Jun 11, 2024
@Kha Kha added the release-ci Enable all CI checks for a PR, like is done for releases label Jun 11, 2024
@Kha
Copy link
Member

Kha commented Jun 11, 2024

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit a25f4b4.
There were no significant changes against commit 287d46e.

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jun 11, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 11, 2024
@tydeu
Copy link
Member

tydeu commented Jun 11, 2024

@Kha What are you attempting to benchmark? Without the stage0 update, this PR has no practical effect.

@Kha
Copy link
Member

Kha commented Jun 13, 2024

@tydeu The update-stage0 size increase but I noticed we don't actually have a benchmark for that :) . Do you know how much it was?

@Kha Kha enabled auto-merge June 13, 2024 15:22
@Kha Kha added this pull request to the merge queue Jun 13, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jun 13, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 13, 2024
Merged via the queue into leanprover:master with commit 8fef03d Jun 13, 2024
22 checks passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR release-ci Enable all CI checks for a PR, like is done for releases toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants