clang-tools: move into llvmPackages

[ShamrockLee]

Description of changes

Format the Nix expression of clang-tools using nixfmt, in accordance with NixOS/rfcs#166.

Move the The clang-tools expression folder under llvm/common and clang-tools package under llvmPackages.

Move clang-tools_<version> into aliases.nix, specifying as llvmPackages_<version>.clang-tools.

Provide clang-tools-python that links and wraps executables from clang-unwrapped.python output. If applied, users will be able to enjoy git clang-format with clang-tools-python installed (or introduced inside Nix shell).

As for "${clang-unwrapped.python}/bin/scan-view", it depends on the relative module inside "../share". The dependent module of scan-view, ScanView.py, is currently not moved into clang-unwrapped.python, which will be fixed by the staging PR #191801.

Cc:
@Patryk27 clang-tools maintainer

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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 (The 24 updated packages will take some time to build.)
• Tested basic functionality of all binary files (usually in ./result/bin/) (Tested git-clang-format, but not scan-view)
• 22.11 Release Notes (or backporting 22.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
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[ShamrockLee]

@ofborg build bat-extras.prettybat cloudcompare entwine grass libpulsar nominatim pdal python310Packages.tiledb python39Packages.tiledb qgis qgis-ltr tiledb vscode-extensions.ms-vscode-cpptools

[ShamrockLee]

Just seperate those Python scripts to a new output python, so that packages depending on clang-tools won't be forced to have Python in their closures.

It will still be installed as meta.outputsToInstall are set to include "python".

Thank you for your time and patience.

[Patryk27]

Looks pretty neat 🙂

[ShamrockLee]

Could it be merged now?

Or should I split the wrapped python script into a new derivation clang-tools-python to reduce the number of rebuild?

[Patryk27]

The changes look fine to me -- I don't personally have merging rights, though 👀

[ShamrockLee]

No wonder. I had thought that all the members have the merging right.

[ShamrockLee]

Just make a new derivation clang-tools-python instead. This way, packages depending on clang-tools won't have to rebuild whenever python gets rebuild.

Wrapping the Python scripts into a new package also guarantees zero rebuild of the current packages, and is thus easier to backport.

[Patryk27]

🚀

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1507

[RossComputerGuy]

Just resolve the conflicts and this should be good to go.

[ShamrockLee]

@RossComputerGuy The release note entry is inside rl-2405.section.md. Is it okay to backport this PR, or how should I handle the release note entry?

[RossComputerGuy]

@ShamrockLee I don't think it's too late to backport. Imo, release note entries are not that important for something like this.

[rrbutani]

Is it okay to backport this PR

I don't think it's too late to backport

I think we should maybe get a second opinion on this; breaking changes are restricted <4 weeks before 24.05's release and changing the clang-tools_* attributes to aliases could be construed as breaking.

Imo, release note entries are not that important for something like this.

I think it's worth having, especially since the intent (as per the OP) is to phase out the aliases that were added.

[ShamrockLee]

@rrbutani Not sure if the change of <pkg>.override interfaces caused by moving clang-tools into llvmPakcages would also be considered backward-incompatible.

[rrbutani]

Looks good, couple of nits about the release note entry:

[rrbutani]

nit: the scope that's used in llvmPackages already has version:

nixpkgs/pkgs/development/compilers/llvm/18/default.nix

Line 61 in 0179f8c

callPackage = newScope (tools // { inherit stdenv cmake ninja libxml2 python3 release_version version monorepoSrc buildLlvmTools; });

You should be able to drop clang-unwrapped from this package's attrs (and all of the llvmPackages_* set's callPackages for clang-tools) and just ask for version in this package's attrs.

[ShamrockLee]

Since clang-tools is a wrapper of (a part of) clang-unwrapped, the build process would be more straightforward to pass clang-unwrapped as a shell variable. It's also okay to string-interpolate it directly.

As for the version, it makes sense to pass it from the llvmPackage an <pkg>.override argument.

You should be able to drop clang-unwrapped from this package's attrs

Are we removing the attribute to prevent people from accessing the clang-unwrapped package used via the attribute (or even try to override it via <pkg>.overrideAttrs) but encourage accessing/overriding via llvmPackages instead?

[rrbutani]

My apologies, I missed that the $out/bin symlinks are produced from clang-unwrapped and not clang/libcxxClang.

Please disregard the above; getting version from clang-unwrapped seems like the sensible thing to do.

[rrbutani]

Not sure if the change of <pkg>.override interfaces caused by moving clang-tools into llvmPakcages would also be considered backward-incompatible.

oh, good point! here, let's ask:

---

@wegank This PR changes some top-level attrs to be aliases and changes some package attributes such that existing clang-tools_<ver>.override { ... } invocations may break (now takes clang, libcxxClang instead of llvmPackages). We're wondering if this is okay to backport to 24.05 or if this is considered a breaking change.

[wegank]

I'd consider the change of argument for clang-tools to be breaking. There's also the move to aliases for clang-tools_1{2-8}, which sounds breaking too.

[rrbutani]

@ShamrockLee thanks for working on this; one last minor change and this will be good to merge: