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

llvmPackages_git: remove #269994

Closed
wants to merge 2 commits into from
Closed

llvmPackages_git: remove #269994

wants to merge 2 commits into from

Conversation

Artturin
Copy link
Member

@Artturin Artturin commented Nov 25, 2023

There are no maintainers for this as shown by it still being at llvm 15 and it's unnecessary work to apply all changes to it.

It has not been a git version for a long time and often lags behind the latest stable llvm

Here are all the differences between the current base(15) of _git and 15 https://gist.github.com/Artturin/c39265ecaeea523cd7224fc7b7632371

We should do this because it eases (my) work on deduplicating llvmPackages, after more of it is deduplicated then we could reconsider adding this back but for now it's just a burden.

Any user who wants to use a git release can use the gitRelease argument in 13+

Description of changes

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/)
  • 23.11 Release Notes (or backporting 23.05 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.

Priorities

Add a 👍 reaction to pull requests you find important.

Comment on lines -83 to -101
preInstall = lib.optionalString stdenv.isDarwin ''
for file in lib/*.dylib; do
if [ -L "$file" ]; then continue; fi

# Fix up the install name. Preserve the basename, just replace the path.
installName="$out/lib/$(basename $(${stdenv.cc.targetPrefix}otool -D $file | tail -n 1))"

# this should be done in CMake, but having trouble figuring out
# the magic combination of necessary CMake variables
# if you fancy a try, take a look at
# https://gitlab.kitware.com/cmake/community/-/wikis/doc/cmake/RPATH-handling
${stdenv.cc.targetPrefix}install_name_tool -id $installName $file

# cc-wrapper passes '-lc++abi' to all c++ link steps, but that causes
# libcxxabi to sometimes link against a different version of itself.
# Here we simply make that second reference point to ourselves.
for other in $(${stdenv.cc.targetPrefix}otool -L $file | awk '$1 ~ "/libc\\+\\+abi" { print $1 }'); do
${stdenv.cc.targetPrefix}install_name_tool -change $other $installName $file
done
Copy link
Member Author

Choose a reason for hiding this comment

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

This change is missing from 15 but in #194634

#185766
verify still needed; add a regression passthru test?

appears to be no longer needed; libc++abi.so didn't seem to be linked against older libc++abi on non-darwin anyways (where the default stdenv wasn't LLVM based) and now we unconditionally use the clang from the same package set which has libcxx set to null and thus doesn't try to link in any libc++abi

, ninja
, python3
, libffi
, enableGoldPlugin ? (!stdenv.isDarwin && !stdenv.targetPlatform.isWasi)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is enableGoldPlugin ? libbfd.hasPluginAPI in 15 because b9a9dfc wasn't ported to git

@Artturin
Copy link
Member Author

I went through all the diffs and there are no changes missing for 15, only changes missing from _git

Works after changing `git/default.nix` to use `gitRelease` instead of `officialRelease`

Wont update the version attr

```
, gitRelease ? {
    version = "15.0.0";
    rev = "a5640968f2f7485b2aa4919f5fa68fd8f23e2d1f";
    rev-version = "unstable-2022-26-07";
    sha256 = "1sh5xihdfdn2hp7ds3lkaq1bfrl4alj36gl1aidmhlw65p5rdvl7";
  }

```
There are no maintainers for this as shown by it still being at llvm 15
and it's unnecessary work to apply all changes to it.

It has not been a git version for a long time and often lags behind the latest stable llvm
@Artturin
Copy link
Member Author

Artturin commented Nov 25, 2023

llvmPackages_git was requested in #114828

@dotlambda dotlambda removed their request for review November 26, 2023 01:25
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Nov 26, 2023
@rrbutani rrbutani self-requested a review December 8, 2023 03:39
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Mar 8, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 20, 2024
@RossComputerGuy
Copy link
Member

Probably shouldn't kill llvmPackages_git. Since LLVM 17, I've been using it as a template for the next LLVM version. Once 18 is merged, I plan on trying to keep llvmPackages_git more up to date and I even am aiming to make everything common.

@Artturin Artturin closed this Mar 26, 2024
@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
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild 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.

5 participants