Skip to content

clang_15: fix build!=(host==target) cross compilation#226551

Merged
alyssais merged 1 commit intostagingfrom
unknown repository
Apr 19, 2023
Merged

clang_15: fix build!=(host==target) cross compilation#226551
alyssais merged 1 commit intostagingfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Apr 17, 2023

Description of changes

clang_15 appears to not cross compile in the build!=(host==target) case due to two problems, which this commit fixes:

  • It trips -Wmaybe-uninitialized on recent gcc, but only in the build!=host case (likely due to #ifdefs)

  • Two more buildPlatform tools have been added: clang-tidy-confusable-chars-gen and clang-pseudo-gen

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.

@ghost ghost requested a review from matthewbauer as a code owner April 17, 2023 00:10
@ofborg ofborg bot added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Apr 17, 2023
@ghost
Copy link
Copy Markdown
Author

ghost commented Apr 17, 2023

ofborg build pkgsCross.aarch64-multiplatform.clang_15

@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. labels Apr 17, 2023
@rrbutani
Copy link
Copy Markdown
Contributor

(I think we have to actually @ ofborg; trying again)

@ofborg build pkgsCross.aarch64-multiplatform.clang_15

Copy link
Copy Markdown
Contributor

@rrbutani rrbutani left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! I definitely dropped the ball on making sure we didn't break cross in #194634.


This looks good; I left some notes with minor nits.

Could you also apply these changes to llvmPackages_git? I believe the llvmPackages_git version on master is just slightly too old to pick up llvm/llvm-project@18b4a8b so clang-tidy-confusable-chars-gen won't exist and the build will fail but it's fine; we're bumping llvmPackages_git to 15.0.7 shortly: #222894.

@ghost
Copy link
Copy Markdown
Author

ghost commented Apr 17, 2023

This looks good; I left some notes with minor nits.

I believe I've taken care of them; if not let me know.

Could you also apply these changes to llvmPackages_git?

Done: b789673fb9d62d1fe86d9ea49dc492ff24c65256

Next force-push will be a squash.

I definitely dropped the ball on making sure we didn't break cross in #194634.

Not at all! We have almost no ofborg/hydra coverage for pkgsCross -- until we do I just sort of accept that things will break a lot. Fortunately I am very, very close to having my daily-driver laptop be completely cross-compiled (by a much faster machine), so any future breakage should be noticed quickly.

@ofborg ofborg bot requested a review from rrbutani April 17, 2023 06:55
@rrbutani
Copy link
Copy Markdown
Contributor

Looks good, thank you!

I think this may have to target staging though; not sure.

@ofborg ofborg bot requested a review from rrbutani April 17, 2023 07:14
@RaitoBezarius
Copy link
Copy Markdown
Member

I believe indeed it should target staging.

Copy link
Copy Markdown
Member

@alyssais alyssais left a comment

Choose a reason for hiding this comment

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

Thanks! I partially had the same fix (without the -Wno-maybe-uninitialized) in a local branch, but was waiting for LLVM 15 and git to be sufficiently in sync before I PRed it. (It's actually what spawned that whole effort.)

Should go to staging though, as others have pointed out.

@alyssais
Copy link
Copy Markdown
Member

@RaitoBezarius maybe we should merge #222894 first, so we don't forget to add the missing program once it's bumped? Is it waiting for anything?

@rrbutani
Copy link
Copy Markdown
Contributor

@RaitoBezarius maybe we should merge #222894 first, so we don't forget to add the missing program once it's bumped? Is it waiting for anything?

I believe #222894 is ready; I think we just needed to confirm that the ofborg failures on that PR are spurious.

Not sure I follow re: the missing program; IIUC this PR updates llvmPackages_git to copy all the host tools that are in LLVM 15. If we merge this PR before #222894 llvmPackages_git.llvm will be broken for a bit (and not just under cross) but if this PR is targeting staging I think it's fine? (so long as we merge #222894 before the next staging cycle finishes)

@RaitoBezarius
Copy link
Copy Markdown
Member

AFAIK we should proceed with the merge of the PR you are mentioning, I was waiting for your final validations.

clang_15 appears to not cross compile in the build!=(host==target)
case due to two problems, which this commit fixes:

- It trips -Wmaybe-uninitialized on recent gcc, but only in the
  build!=host case (likely due to #ifdefs)

- Two more buildPlatform tools have been added:
  clang-tidy-confusable-chars-gen and clang-pseudo-gen

Co-authored-by: Rahul Butani <rrbutani@users.noreply.github.com>
@ghost ghost marked this pull request as draft April 18, 2023 02:42
@ghost ghost changed the base branch from master to staging April 18, 2023 02:42
@ghost
Copy link
Copy Markdown
Author

ghost commented Apr 18, 2023

I believe indeed it should target staging.

Done.

@ghost ghost marked this pull request as ready for review April 18, 2023 02:43
@ofborg ofborg bot requested review from alyssais and rrbutani April 18, 2023 03:02
@rrbutani rrbutani added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Apr 18, 2023
@alyssais
Copy link
Copy Markdown
Member

@ofborg build pkgsCross.powernv.clang

@alyssais alyssais merged commit 2e77eb8 into NixOS:staging Apr 19, 2023
@ghost ghost deleted the pr/clang_15/fixcross branch April 20, 2023 19:29
"-DSPHINX_OUTPUT_HTML=OFF"
"-DSPHINX_WARNINGS_AS_ERRORS=OFF"
] ++ lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [
] ++ lib.optionals (!stdenv.buildPlatform.canExecute stdenv.hostPlatform) [
Copy link
Copy Markdown
Member

@Artturin Artturin Aug 21, 2023

Choose a reason for hiding this comment

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

Broke pkgsCross.musl64.llvmPackages_16.clang.cc and (presumably, not built) pkgsStatic.llvmPackages_16.clang.cc (15 too)

       > cd /build/clang-src-16.0.6/clang/build && clang-tblgen -gen-clang-diags-defs -clang-component=Serialization -I /build/clang-src-16.0.6/clang/include/clang/Basic -I/build/clang-src-16.0.6/clang/include -I/build/clang-src-16.0.6/clang/build/include -I/nix/store/0jp04rmkq2b25qqar9y6nm0z3dzmziib-llvm-x86_64-unknown-linux-musl-16.0.6-dev/include /build/clang-src-16.0.6/clang/include/clang/Basic/Diagnostic.td --write-if-changed -o include/clang/Basic/DiagnosticSerializationKinds.inc -d include/clang/Basic/DiagnosticSerializationKinds.inc.d
       > /bin/sh: clang-tblgen: not found

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@rrbutani rrbutani added the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants