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

fix: nix: add platform dependent flag to lib target #3811

Merged
merged 7 commits into from
Jun 28, 2024

Conversation

lenianiva
Copy link
Contributor

@lenianiva lenianiva commented Mar 30, 2024

Closes #3810

@lenianiva
Copy link
Contributor Author

lenianiva commented Mar 30, 2024

WIP: Although both libInit_shared and leanshared can build, currently there's a missing symbol error:

$ nix build .#packages.x86_64-darwin.leanc

gives

ld: warning: directory not found for option '-L/nix/store/6bk086vc6m88274aapc6md587l7p1s4d-lean-bin-tools/lib/lean'
Undefined symbols for architecture x86_64:
  "_lean_initialize", referenced from:
      _main in Leanc.o
ld: symbol(s) not found for architecture x86_64
clang-11: error: linker command failed with exit code 1 (use -v to see invocation)

@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 Mar 30, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Mar 30, 2024

Mathlib CI status (docs):

  • ❗ Mathlib CI can not be attempted yet, as the nightly-testing-2024-03-28 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Mathlib CI should run now. (2024-03-30 22:09:37)
  • ❗ Std CI can not be attempted yet, as the nightly-testing-2024-04-25 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Std CI should run now. (2024-04-26 23:19:12)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 366f3ac272b6a12b19cabd8573652fc138804bdf --onto ea46bf2839ad1c98d3a0c3e5caad8a81f812934c. (2024-06-10 16:49:43)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 30a922a7e914a37c98e2c4ab88440e347084172f --onto 5c978a2e2437e30e2c01a9b027b2944d709919c9. (2024-06-28 03:31:48)

@jetjinser
Copy link

Any progress? I'm in the same predicament and I really want it to work on my x86_64-darwin. Is there anything I can do to help?

I finally found that missing symbol _lean_initialize is in leancpp, and this dependency is not in Leanc.executable.

Frankly speaking, I am a bit confused because I don't understand how lean is built 😔.

@nomeata
Copy link
Collaborator

nomeata commented May 3, 2024

From my side the status is that I'm rooting for you here, and fixes can be merged, but usage of nix beyond what we need for the CI setup need to be taken care of by those that use it. Hope you understand.

@lenianiva
Copy link
Contributor Author

Any progress? I'm in the same predicament and I really want it to work on my x86_64-darwin. Is there anything I can do to help?

I finally found that missing symbol _lean_initialize is in leancpp, and this dependency is not in Leanc.executable.

Frankly speaking, I am a bit confused because I don't understand how lean is built 😔.

can you poke around the build targets of nix/bootstrap.nix a bit to see what is going on? When I tried to fix it I looked at the symbol table of libInit_shared and leanc etc. to see if they have the symbols required, but I don't know why it still doesn't compile.

@lenianiva lenianiva marked this pull request as draft June 10, 2024 19:46
@lenianiva
Copy link
Contributor Author

I'll periodically merge master into this branch and see if anything changes, but I have spent >8h on this and I could not make more progress

@lenianiva
Copy link
Contributor Author

lenianiva commented Jun 18, 2024

Any progress? I'm in the same predicament and I really want it to work on my x86_64-darwin. Is there anything I can do to help?

I finally found that missing symbol _lean_initialize is in leancpp, and this dependency is not in Leanc.executable.

Frankly speaking, I am a bit confused because I don't understand how lean is built 😔.

How is leancpp not a dependency of Leanc.executable? The linker flags of leanshared has -lleancpp, and leanshared is a dependency of leanc.

Lean is built with bootstrapping, and there is a function in nix/bootstrap.nix which builds a single stage. There is some problem with this function so the relevant symbols for _lean_initialize didn't go into leanc

@lenianiva lenianiva marked this pull request as ready for review June 28, 2024 03:17
@lenianiva
Copy link
Contributor Author

Any progress? I'm in the same predicament and I really want it to work on my x86_64-darwin. Is there anything I can do to help?

I finally found that missing symbol _lean_initialize is in leancpp, and this dependency is not in Leanc.executable.

Frankly speaking, I am a bit confused because I don't understand how lean is built 😔.

I fixed it!

@jetjinser
Copy link

Any progress? I'm in the same predicament and I really want it to work on my x86_64-darwin. Is there anything I can do to help?
I finally found that missing symbol _lean_initialize is in leancpp, and this dependency is not in Leanc.executable.
Frankly speaking, I am a bit confused because I don't understand how lean is built 😔.

I fixed it!

marvelous! 😍

@nomeata
Copy link
Collaborator

nomeata commented Jun 28, 2024

@jetjinser, did you have a chance to review and test the fix?

@jetjinser
Copy link

@jetjinser, did you have a chance to review and test the fix?

Yes! It actually built successfully on my x86_64-darwin machine!
It looks like nix specifies the correct lib on darwin.

@nomeata nomeata changed the title fix: Add platform dependent flag to lib target in nix fix: nix: add platform dependent flag to lib target Jun 28, 2024
@nomeata nomeata enabled auto-merge June 28, 2024 10:40
@nomeata nomeata added this pull request to the merge queue Jun 28, 2024
Merged via the queue into leanprover:master with commit b8dd515 Jun 28, 2024
15 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Jun 29, 2024
Adds linkage to `Std` so the build behaviour on darwin is in line with
linux

I'm not sure why linking with `Std` is needed. I deleted it in the
previous patch #3811 and Lean
still builds and runs. @tydeu mentioned this issue so I created this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

Shared library fails to build on macOS with Nix
5 participants