Skip to content

llvmPackages_17: unbreak on x86_64-darwin #282727

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
Merged
1 commit merged into from
Feb 5, 2024
Merged

llvmPackages_17: unbreak on x86_64-darwin #282727

1 commit merged into from
Feb 5, 2024

Conversation

@wegank
Copy link
Member

@wegank wegank commented Jan 21, 2024 •
edited
Loading

Description of changes

I just borrowed a patch from MacPorts.

Things done

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

Add a 👍 reaction to pull requests you find important.

@wegank wegank marked this pull request as ready for review January 21, 2024 23:37
@wegank wegank requested a review from RaitoBezarius as a code owner January 21, 2024 23:37
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jan 21, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jan 22, 2024
@wegank
Copy link
Member Author

wegank commented Jan 22, 2024

Result of nixpkgs-review pr 282727 run on x86_64-darwin 1

1 package marked as broken and skipped:
  • pcsx2
17 packages built:
  • clang-tools_17
  • clang_17 (llvmPackages_17.clang ,llvmPackages_17.libcxxClang)
  • include-what-you-use
  • llvmPackages_17.clangNoLibc
  • llvmPackages_17.clangNoLibcxx
  • llvmPackages_17.clangUseLLVM
  • llvmPackages_17.compiler-rt (llvmPackages_17.compiler-rt-libc ,llvmPackages_17.compiler-rt-no-libc)
  • llvmPackages_17.compiler-rt.dev (llvmPackages_17.compiler-rt-libc.dev)
  • llvmPackages_17.libcxx
  • llvmPackages_17.libcxx.dev
  • llvmPackages_17.stdenv (llvmPackages_17.libcxxStdenv)
  • llvmPackages_17.libcxxabi
  • llvmPackages_17.libcxxabi.dev
  • llvmPackages_17.libstdcxxClang
  • llvmPackages_17.libunwind
  • llvmPackages_17.libunwind.dev
  • tests.cc-wrapper.llvmTests.llvmPackages_17.clang (tests.cc-wrapper.llvmTests.llvmPackages_17.libcxx)

ghost
ghost reviewed Jan 22, 2024
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.

llvm/llvm-project#64226 is marked as won't fix due to using an EOL and unsupported build system.

other than that -- minor comment to use relative = "libcxx"; in fetchpatch so the changes to the copy / past code in {pre,post}Patch can get reverted and thus match the other copies in llvm{15,16,git,etc).

finally, i think this will cause a merge conflict with #278945 which is in staging-next -- not sure if it is easier to merge this into staging-next or master and deal with the conflict.

@wegank wegank marked this pull request as draft January 22, 2024 03:09
@ofborg ofborg bot added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. and removed 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jan 22, 2024
ghost
ghost reviewed Jan 25, 2024
View reviewed changes
alyssais
alyssais reviewed Jan 25, 2024
View reviewed changes
pkgs/development/compilers/llvm/17/libcxx/default.nix Outdated
Copy link
Member

@alyssais alyssais Jan 25, 2024

Choose a reason for hiding this comment

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

Any reason not to apply this unconditionally? That way people are more likely to notice if some change breaks it.

Copy link
Member Author

@wegank wegank Jan 25, 2024

Choose a reason for hiding this comment

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

I don't think this patch is no-op on macOS >=10.13. As @a-n-n-a-l-e-e mentioned, it would be better to modify the patch to not get the extra include if unneeded.

@Baltoli Baltoli mentioned this pull request Jan 26, 2024
Merged
@ofborg ofborg bot requested a review from alyssais January 29, 2024 13:44
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. and removed 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. labels Jan 29, 2024
ghost
ghost reviewed Jan 29, 2024
View reviewed changes
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. labels Jan 29, 2024
@ghost
Copy link

ghost commented Jan 29, 2024

Result of nixpkgs-review pr 282727 run on x86_64-darwin 1

1 package marked as broken and skipped:
  • pcsx2
17 packages built:
  • clang-tools_17
  • clang_17 (llvmPackages_17.clang ,llvmPackages_17.libcxxClang)
  • include-what-you-use
  • llvmPackages_17.clangNoLibc
  • llvmPackages_17.clangNoLibcxx
  • llvmPackages_17.clangUseLLVM
  • llvmPackages_17.compiler-rt (llvmPackages_17.compiler-rt-libc ,llvmPackages_17.compiler-rt-no-libc)
  • llvmPackages_17.compiler-rt.dev (llvmPackages_17.compiler-rt-libc.dev)
  • llvmPackages_17.libcxx
  • llvmPackages_17.libcxx.dev
  • llvmPackages_17.stdenv (llvmPackages_17.libcxxStdenv)
  • llvmPackages_17.libcxxabi
  • llvmPackages_17.libcxxabi.dev
  • llvmPackages_17.libstdcxxClang
  • llvmPackages_17.libunwind
  • llvmPackages_17.libunwind.dev
  • tests.cc-wrapper.llvmTests.llvmPackages_17.clang (tests.cc-wrapper.llvmTests.llvmPackages_17.libcxx)

@Baltoli
Copy link
Contributor

Baltoli commented Feb 5, 2024

Sorry to keep bothering you all; very grateful for the work you're putting in to maintain this code - @RaitoBezarius @wegank is there any chance of this getting merged soon? It would be a big help to one of the projects I maintain!

@wegank wegank marked this pull request as ready for review February 5, 2024 11:42
@wegank
Copy link
Member Author

wegank commented Feb 5, 2024

The patch doesn't apply on LLVM 18.1.0-rc1, probably due to formatting changes. @a-n-n-a-l-e-e should I make a patch for llvmPackages_git?

@ghost
Copy link

ghost commented Feb 5, 2024

The patch doesn't apply on LLVM 18.1.0-rc1, probably due to formatting changes. @a-n-n-a-l-e-e should I make a patch for llvmPackages_git?

that would be great

@ghost ghost merged commit 1fbf11b into NixOS:master Feb 5, 2024
@wegank wegank deleted the llvm-17-darwin-2 branch February 5, 2024 18:42
@rrbutani rrbutani added the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label May 27, 2024
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RaitoBezarius RaitoBezarius Awaiting requested review from RaitoBezarius

@lovek323 lovek323 Awaiting requested review from lovek323

@sternenseemann sternenseemann Awaiting requested review from sternenseemann

@rrbutani rrbutani Awaiting requested review from rrbutani

@dtzWill dtzWill Awaiting requested review from dtzWill

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@alyssais alyssais Awaiting requested review from alyssais

1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: darwin Running or building packages on Darwin 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: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@wegank @Baltoli @alyssais @rrbutani