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.

Sign up for GitHub

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

Jump to bottom

Synchronize cross-stdenv LLVM input with pkgs.llvmPackages #107594

Merged
merged 1 commit into from Jan 10, 2021
Merged

Synchronize cross-stdenv LLVM input with pkgs.llvmPackages #107594

merged 1 commit into from Jan 10, 2021

Conversation

ehmry
Copy link
Contributor

@ehmry ehmry commented Dec 25, 2020

Motivation for this change

Refer to the unversioned llvmPackages in pkgs/stdenv/cross/default.nix so that it can overriden via an overlay. As a prerequisite llvmPackages must be updated for consistency with the current cross LLVM.

@ehmry ehmry added the 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on label Dec 25, 2020
@ofborg ofborg bot added the 6.topic: stdenv Standard environment label Dec 25, 2020
@Ericson2314
Copy link
Member

I imagine this is a big change for Darwin? And you might want to bump the llvmPackages_7 in the darwin stdenv to llvmPackages_8 too. (Unfortunately it must keep the hard-coded number.)

CC @NixOS/darwin-maintainers

@ehmry ehmry closed this Dec 25, 2020
@ehmry ehmry reopened this Dec 25, 2020
ehmry
ehmry commented Dec 25, 2020
View reviewed changes
pkgs/stdenv/cross/default.nix
@@ -66,7 +66,7 @@ in lib.init bootStages ++ [
else if crossSystem.isDarwin
then buildPackages.llvmPackages.clang
else if crossSystem.useLLVM or false
then buildPackages.llvmPackages_8.lldClang
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who is actually using this cross useLLVM?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WASM. I often use it when testing because its much faster to bootstrap.

@ehmry
Copy link
Contributor Author

ehmry commented Dec 25, 2020

I probably should have opened an issue rather than a PR.

@ehmry
Copy link
Contributor Author

ehmry commented Dec 25, 2020

I imagine this is a big change for Darwin? And you might want to bump the llvmPackages_7 in the darwin stdenv to llvmPackages_8 too. (Unfortunately it must keep the hard-coded number.)

Would it be better to downgrade the cross LLVM to llvmPackages_7 using the current llvmPackages alias, and whomever needs llvmPackages_8 can make an override somewhere?

@Ericson2314
Copy link
Member

WASM won't work with LLVM 7 IIRC.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/cross-compiling-with-clang-for-risc-v/6683/7

blitz
blitz reviewed Dec 26, 2020
View reviewed changes
pkgs/top-level/all-packages.nix Outdated
@@ -9865,7 +9865,7 @@ in
llvm_6 = llvmPackages_6.llvm;
llvm_5 = llvmPackages_5.llvm;

llvmPackages = recurseIntoAttrs llvmPackages_7;
llvmPackages = recurseIntoAttrs llvmPackages_8;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that LLVM 11 is out, why are we so conservative here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ask @NixOS/darwin-maintainers I would assume people try to bump this all the time and there are issues.

@ehmry
Copy link
Contributor Author

ehmry commented Jan 4, 2021

I've rewritten the PR to make the llvmPackages version explicit for some platforms in all-packages.nix, seems like the most transparent thing to do.

@ehmry
Copy link
Contributor Author

ehmry commented Jan 10, 2021

ping @Ericson2314

@ehmry ehmry changed the title Synchronize llvmPackages with the cross-stdenv LLVM input Synchronize cross-stdenv LLVM input with pkgs.llvmPackages Jan 10, 2021
@Ericson2314 Ericson2314 merged commit 5114061 into NixOS:staging Jan 10, 2021
@Ericson2314
Copy link
Member

Ericson2314 commented Jan 10, 2021 •
edited

@ehmry hehe nice escape hatch! Also I suppose we could send to master too? I meant to not merge and post that instead haha.

@ehmry
Copy link
Contributor Author

ehmry commented Jan 11, 2021

@Ericson2314 yes, that would be great, this gets me closer to building Genode using an overlay against nixpkgs master.

@ehmry
Copy link
Contributor Author

ehmry commented Jan 11, 2021

Done.

@ehmry ehmry deleted the stdenv-cross-llvm branch January 11, 2021 10:11
@DieracDelta
Copy link
Member

Do you know how I would override this attribute? I'd like to bump the version of llvmPackages for riscv64 (since llvm 7 doesn't exist for that platform) but I'm not sure where to bump it.

@ehmry
Copy link
Contributor Author

ehmry commented Apr 23, 2021

If you can use a overlay I would do this:

final: prev:
with prev; {
  llvmPackages =
    if targetPlatform.isRiscV then llvmPackages_latest else llvmPackages;
}

@ehmry
Copy link
Contributor Author

ehmry commented Apr 24, 2021

Actually I think hostPlatform.isRiscV is what you want.

@Ericson2314
Copy link
Member

you probably want if hostPlatform.isRiscV || targetPlatform.isRiscV: it's target for the tools but host for the libraries, but the versions of each ought to be in sync too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blitz blitz blitz left review comments

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@dtzWill dtzWill Awaiting requested review from dtzWill

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314 Ericson2314 is a code owner

Assignees
No one assigned
Labels
6.topic: cross-compilation Building packages on a different sort platform than than they will be run on 6.topic: stdenv Standard environment 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ehmry @Ericson2314 @nixos-discourse @DieracDelta @blitz