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: elan: use relative paths in wrapper script #294514

Merged
merged 2 commits into from
Mar 10, 2024

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented Mar 9, 2024

since 3.1.1, elan extracts to a temporary location and then moves to a final location, this threw the wrapper script off. Updating the patch to use paths relative to the wrapper script, so that it works independent of absolute location.

Tested like this:

~/build/lean/mathlib4 $ nix run ~/build/nixpkgs#elan -- run --install $(cat lean-toolchain) lake exec cache
warning: Git tree '/home/jojo/build/nixpkgs' is dirty
info: std: cloning https://github.com/leanprover/std4 to './.lake/packages/std'
info: Qq: cloning https://github.com/leanprover-community/quote4 to './.lake/packages/Qq'
info: aesop: cloning https://github.com/leanprover-community/aesop to './.lake/packages/aesop'
info: proofwidgets: cloning https://github.com/leanprover-community/ProofWidgets4 to './.lake/packages/proofwidgets'
info: Cli: cloning https://github.com/leanprover/lean4-cli to './.lake/packages/Cli'
info: importGraph: cloning https://github.com/leanprover-community/import-graph.git to './.lake/packages/importGraph'
info: [0/9] Downloading proofwidgets cloud release
info: [0/9] Unpacking proofwidgets cloud release
info: [1/9] Building Cache.IO
info: [2/9] Compiling Cache.IO
info: [2/9] Building Cache.Hashing
info: [3/9] Compiling Cache.Hashing
info: [3/9] Building Cache.Requests
info: [4/9] Compiling Cache.Requests
info: [4/9] Building Cache.Main
info: [6/9] Compiling Cache.Main
info: [9/9] Linking cache
Mathlib4 caching CLI
Usage: cache [COMMAND]

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

since 3.1.1, `elan` extracts to a temporary location and then moves to a
final location, this threw the wrapper script off. Updating the patch to
use paths relative to the wrapper script, so that it works independent
of absolute location.
@nomeata nomeata mentioned this pull request Mar 9, 2024
13 tasks
@collares
Copy link
Member

collares commented Mar 9, 2024

Can you remove the patch reverting the temporary directory creation (that is, revert #294040)? Thanks!

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Mar 9, 2024
@nomeata
Copy link
Contributor Author

nomeata commented Mar 9, 2024

Ah, I didn’t even see that there was another fix applied. Revert added to this branch!

@collares collares requested a review from marsam March 9, 2024 16:02
@ofborg ofborg bot requested a review from gebner March 9, 2024 16:15
@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Mar 10, 2024
@collares
Copy link
Member

The commit convention would be to start the commit message with elan: instead of fix: , but I don't think this warrants another round of changes. Thank you for fixing this!

@collares collares merged commit 79435d8 into NixOS:master Mar 10, 2024
20 checks passed
@nomeata
Copy link
Contributor Author

nomeata commented Mar 10, 2024

Sorry, I sometimes get confused when contributing to different projects :-). Thanks for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants