Skip to content

Comments

llvmPackages_14.openmp: unbreak#163868

Merged
siraben merged 6 commits intoNixOS:masterfrom
dtzWill:fix/openmp-14-patch
Mar 25, 2022
Merged

llvmPackages_14.openmp: unbreak#163868
siraben merged 6 commits intoNixOS:masterfrom
dtzWill:fix/openmp-14-patch

Conversation

@dtzWill
Copy link
Member

@dtzWill dtzWill commented Mar 12, 2022

Description of changes

Fixup old patches causing OpenMP to be marked broken,
one of which can be removed in favor of pointing the build
at the required tools directly.

Mostly get tests working, but few failures I haven't tracked down
caused me to ultimately disable them for now.

Help testing or pointing to something that should work on Linux with
this is appreciated :).

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/)
  • 22.05 Release Notes (or backporting 21.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@dtzWill dtzWill requested a review from matthewbauer as a code owner March 12, 2022 17:20
@ofborg ofborg bot requested review from 7c6f434c, lovek323 and primeos March 12, 2022 17:33
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Mar 12, 2022
@dtzWill
Copy link
Member Author

dtzWill commented Mar 12, 2022

Looks like others need fixing too, see #162423 . Especially libunwind and libcxxabi. libunwind is trickier re:patch.
@Ericson2314 IIRC you upstreamed some patches to LLVM re:GNUInstallDirs, what's the status there? And don't suppose you have patches for these already or would be willing to take a look? Thanks!

@dtzWill dtzWill force-pushed the fix/openmp-14-patch branch from a82f397 to a122d38 Compare March 14, 2022 14:18
@dtzWill dtzWill force-pushed the fix/openmp-14-patch branch from a122d38 to 2efcc3e Compare March 24, 2022 23:59
@dtzWill
Copy link
Member Author

dtzWill commented Mar 24, 2022

(force-push'ing after rebasing onto master, and -rc updates before that)

@Ericson2314
Copy link
Member

@dtzWill #154465 updated the patches somewhat, a tiny bit more was perhaps upstreamed since. I am not sure at what point the branch-offs occured upstream.

@Ericson2314
Copy link
Member

github.com/Ericson2314/llvm-project has the commits I made by patches from, but I would rebase them so it is unclear whether we have enough to stuff for prior releases from there :(.

@Ericson2314
Copy link
Member

@dtzWill Did you just remove parts of the patch that already were applied, or what?

@dtzWill
Copy link
Member Author

dtzWill commented Mar 25, 2022

@dtzWill Did you just remove parts of the patch that already were applied, or what?

That's what it ended up being, yes, I think. Not how I went about it but yes, applied and seemed safe to drop bits that were already applied (since seemed you were upstreaming things). The rest still seem useful, and seem to be needed (and doing what they seem intended to).

Bummer about the patches rebase'd and maybe lost from around the branch-off. These things happen, but ugh sorry. How do you want to handle this, then? (don't suppose you have a super high git reflog limit and they're still sticking around in there? hopeful face)

What we have here (openmp) doesn't seem super broken (bits all end up in expected places, compared to openmp13 -- includes are almost entirely the same, etc.), but I haven't checked the produced cmake bits all work (esp across multiple outputs). Haven't sorted what a good test might be, since our in-tree users are Darwin or a clangStdenv (so can't just point nixpkgs-review at it or something :)).

The other bits (the other subprojects) I/we might need (or really appreciate at the least) some help with. Can you take a look? Or is there a way to check things work as expected?


BTW I've done a hopefully-not-too-rough attempt at packaging up MLIR and flang over here: #163878 . I noticed your github has a gnu installdirs for MLIR (5ad96992ef909e6b99d8c693a285f06c9fc645fe), is any of that needed? To fix libraries being installed in the wrong location, I addressed it same as in another patch (but I don't see that in your commit). Well anyway, this can be discussed over in that PR if you have time/are interested 😄.

@siraben
Copy link
Member

siraben commented Mar 25, 2022

Result of nixpkgs-review pr 163868 run on aarch64-darwin 1

1 package built:
  • llvmPackages_14.openmp

@siraben siraben merged commit c65d061 into NixOS:master Mar 25, 2022
@Ericson2314
Copy link
Member

@dtzWill is the issue that the GNU install dirs LLVM patches only contain too much stuff, or do they sometimes not contain enough stuff? If just tthe former I might have reflog, but honestly I would just start with what's in Nixpkgs as more of a known-good state.

@Ericson2314
Copy link
Member

Ericson2314 commented Mar 25, 2022

Also note I did save the original backports in split-prefix-*. I should probably put 13's in there for safe keeping, and 14's once it is fixed up.

@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: llvm/clang Issues related to llvmPackages, clangStdenv and related 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants