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

llvm_15: reenable libclang_rt.profile-....a build on musl #217042

Merged
merged 1 commit into from
Feb 19, 2023
Merged

llvm_15: reenable libclang_rt.profile-....a build on musl #217042

merged 1 commit into from
Feb 19, 2023

Conversation

yu-re-ka
Copy link
Contributor

@yu-re-ka yu-re-ka commented Feb 19, 2023

This was most likely lost because of a bad merge

Note how a few lines below it is already disabled for useLLVM || bareMetal, so they get the COMPILER_RT_BUILD_PROFILE=OFF flag twice.

Change for LLVM 14 was in #191372

Description of changes
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.

This was most likely lost because of a bad merge
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Feb 19, 2023
Copy link
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.

ahh, sorry, I bungled this in rrbutani@8c4b8b5; thanks for the fix!


Definitely doesn't need to be part of this PR or anything but it'd be great to have a regression test for this!

I'm not sure the best way to go about adding one though; @yu-re-ka do you generally notice errors like this because those compiler-rt files are missing or does this manifest as link errors when you try to use -fprofile-instr-generate or something similar?

Edit: nvm, just saw in your original PR that this gets tested as part of the pkgsMusl.firefox build.

@rrbutani rrbutani added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Feb 19, 2023
@yu-re-ka
Copy link
Contributor Author

I notice this when I build pkgsMusl.firefox with PGO enabled

@yu-re-ka yu-re-ka merged commit e27be43 into NixOS:master Feb 19, 2023
@yu-re-ka yu-re-ka deleted the musl-llvm-15-profile branch February 19, 2023 00:38
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: 0 This PR does not cause any packages to rebuild on Linux 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.

2 participants