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

mesa: fix cross compilation to x86_64 #233628

Merged
merged 3 commits into from
Nov 9, 2023
Merged

mesa: fix cross compilation to x86_64 #233628

merged 3 commits into from
Nov 9, 2023

Conversation

alyssais
Copy link
Member

Description of changes

In particular, fix cross compilation of rusticl. Previously, Nixpkgs has not been equipped to handle non-Cargo Rust cross compilation. That's getting increasingly important, now that we're building Mesa with Rust, and soon the kernel, and potentially systemd in not too long.

This is a bigger PR than I usually like to make, but it's difficult to explain each of these changes out of context, so I've bundled them into a single PR. I'd definitely recommend reviewing commit by commit though. The commit messages contain the details of each change.

Spiritual successor to #159064.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.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.

@alyssais alyssais added 6.topic: rust 6.topic: cross-compilation Building packages on a different platform than they will be used on labels May 23, 2023
@alyssais alyssais requested review from RaitoBezarius and K900 May 23, 2023 15:02
@github-actions github-actions bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: stdenv Standard environment labels May 23, 2023
@RaitoBezarius

This comment was marked as resolved.

Sign in to view
@ofborg ofborg bot added the ofborg-internal-error Ofborg encountered an error label May 23, 2023
@alyssais
Copy link
Member Author

@ofborg eval

@alyssais alyssais force-pushed the mesa-cross branch 2 times, most recently from c58048e to 5f13c40 Compare May 23, 2023 17:18
@RaitoBezarius

This comment was marked as outdated.

Sign in to view
@alyssais alyssais added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 31, 2023
@alyssais alyssais force-pushed the mesa-cross branch 3 times, most recently from 5762742 to c0d3cbb Compare November 4, 2023 21:31
@alyssais
Copy link
Member Author

alyssais commented Nov 4, 2023

I've rebased to fix the merge conflict, and I've also:

  • Simplified the logic for looking up values from args.rust falling back to args.rustc falling back to a default.
  • Added an assert so that it's not possible to pass args.rust (the new name) and args.rustc (the old name) at the same time.
  • Fixed rustcTargetSpec referring to a non-existent rustcPlatform that was left over from a previous iteration.

All suggested by @Ericson2314.

ghost
ghost previously requested changes Nov 4, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

We need lib.warn backwards-compatibility placeholders on the toFooBar attributes; crate2nix uses them (in their current location).

Otherwise looks good.

Building:

  • pkgs.tests.cross.sanity

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 4, 2023
@ghost ghost dismissed their stale review November 5, 2023 09:40

main issue fixed; will approve once builds complete

ghost
ghost approved these changes Nov 6, 2023
View reviewed changes
Copy link

@ghost ghost left a comment •
edited by ghost
Loading

Choose a reason for hiding this comment

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

Rust changes look great.

Built:

@ghost ghost mentioned this pull request Nov 6, 2023
Closed
13 tasks
Mic92
Mic92 approved these changes Nov 6, 2023
View reviewed changes
Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

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

Looks clean.

Ericson2314
Ericson2314 reviewed Nov 6, 2023
View reviewed changes
Ericson2314
Ericson2314 approved these changes Nov 6, 2023
View reviewed changes
Copy link
Member

@Ericson2314 Ericson2314 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just one little thing, and then it looks good to. This is very important change let's get it in 23.05 23.11!

(I didn't really look at all the usage site, but I think those are very low risk)

@alyssais
lib.systems: elaborate Rust metadata
9bc04e4 
We need this stuff to be available in lib so make-derivation.nix can
access it to construct the Meson cross file.

This has a couple of other advantages:

 - It makes Rust less special.  Now figuring out what Rust calls a
   platform is the same as figuring out what Linux or QEMU call it.

 - We can unify the schema used to define Rust targets, and the schema
   used to access those values later.  Just like you can set "config"
   or "system" in a platform definition, and then access those same
   keys on the elaborated platform, you can now set "rustcTarget" in
   your crossSystem, and then access "stdenv.hostPlatform.rustcTarget"
   in your code.

"rustcTarget", "rustcTargetSpec", "cargoShortTarget", and
"cargoEnvVarTarget" have the "rustc" and "cargo" prefixes because
these are not exposed to code by the compiler, and are not
standardized.  The arch/os/etc. variables are all named to match the
forms in the Rust target spec JSON.

The new rust.target-family only takes a list, since we don't need to
worry about backwards compatibility when that name is used.

The old APIs are all still functional with no warning for now, so that
it's possible for external code to use a single API on both 23.05 and
23.11.  We can introduce the warnings once 23.05 is EOL, and make them
hard errors when 23.11 is EOL.
@alyssais
stdenv: fix meson rust cross compilation
c258730
@alyssais
mesa: fix cross compilation to x86_64
133dcd5 
In particular, fix cross compilation of rusticl.
@ghost
Copy link

ghost commented Nov 7, 2023 •
edited by ghost
Loading

I agree, this should be merged before the branch-off. However,

This is very important change let's get it in 23.05!

I think that ship has sailed 😆 unless nix-shell -p time-machine

@alyssais
Copy link
Member Author

alyssais commented Nov 7, 2023

The sphinx build failure on x86_64-linux seems to be intermittent / load-related. Let's try again:

@ofborg build mesa mesa.passthru.tests stdenv stdenv.passthru.tests

@ghost
Copy link

ghost commented Nov 9, 2023 •
edited by ghost
Loading

The sphinx build failure on x86_64-linux seems to be intermittent / load-related. Let's try again:

I've been getting sphinx test failures on my machines too. It's annoying. I disabled the tests for that package. This has happened before btw.

(and I don't think it is caused by this PR)

We should consider disabling the tests for the sphinx package. It's just a documentation generator.

@alyssais
Copy link
Member Author

alyssais commented Nov 9, 2023

OfBorg hasn't given me the aarch64 builds for days, so I'm just going to merge. I've tested locally on x86_64-linux and aarch64-linux, and IIRC mesa is broken on Darwin anyway. In the unlikely event there are failures, we'll catch them in staging.

@alyssais alyssais merged commit 6247bd0 into NixOS:staging Nov 9, 2023
@alyssais alyssais deleted the mesa-cross branch November 9, 2023 09:02
@andresilva
Copy link
Member

Seems like this PR broke all packages that use rustc-wasm32, namely: lldap, pagefind and polkadot.

@andresilva
Copy link
Member

andresilva commented Nov 17, 2023 •
edited
Loading

Nevermind, I found #267876 which should fix this.

@andresilva andresilva mentioned this pull request Nov 17, 2023
Merged
13 tasks
@alyssais alyssais mentioned this pull request Nov 17, 2023
Merged
13 tasks
@alyssais
Copy link
Member Author

Fix is #268168.

@infinisil infinisil mentioned this pull request Nov 21, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 approved these changes

@RaitoBezarius RaitoBezarius RaitoBezarius approved these changes

@Ericson2314 Ericson2314 Ericson2314 approved these changes

@K900 K900 Awaiting requested review from K900

@jtojnar jtojnar Awaiting requested review from jtojnar

@mmahut mmahut Awaiting requested review from mmahut

@RaghavSood RaghavSood Awaiting requested review from RaghavSood

@zowoq zowoq Awaiting requested review from zowoq

@winterqt winterqt Awaiting requested review from winterqt

@figsoda figsoda Awaiting requested review from figsoda

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@dotlambda dotlambda Awaiting requested review from dotlambda

@primeos primeos Awaiting requested review from primeos

@lilyinstarlight lilyinstarlight Awaiting requested review from lilyinstarlight

@vcunat vcunat Awaiting requested review from vcunat

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

Assignees
No one assigned
Labels
6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: lib The Nixpkgs function library 6.topic: python 6.topic: rust 6.topic: stdenv Standard environment 10.rebuild-darwin: 501-1000 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+ 10.rebuild-linux: 1001-2500 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@alyssais @RaitoBezarius @vcunat @Ericson2314 @nixos-discourse @andresilva @Mic92 @mweinelt