[llvm] add gnu install dirs

[RossComputerGuy]

Based on the patch in https://github.com/NixOS/nixpkgs/blob/aac3118ab56b878f5775b4302c70afc654de75ba/pkgs/development/compilers/llvm/18/llvm/gnu-install-dirs.patch

This was reverted in e941b03 by @Ericson2314.

The goal here is to properly get this patch into LLVM upstream and drop the patch from nixpkgs. This decreases the maintenance burden as we'd no longer need to rely on maintaining this patch.

[nikic]

This change is incorrect. You can't combine CMAKE_INSTALL_LIBDIR and LLVM_LIBDIR_SUFFIX, that is going to result in lib6464 libdirs.

[nikic]

This was reverted in e941b03 by @Ericson2314.

What has changed in your variant of the patch to address the failures that resulted in the previous revert?

The goal here is to properly get this patch into LLVM upstream and drop the patch from nixpkgs. This decreases the maintenance burden as we'd no longer need to rely on maintaining this patch.

It would be more helpful if the PR description explained what issue this patch is actually trying to solve. Presumably nixpkgs carries this patch for a reason :)

[nikic]

This change is incorrect. You can't combine CMAKE_INSTALL_LIBDIR and LLVM_LIBDIR_SUFFIX, that is going to result in lib6464 libdirs.

If you want to use CMAKE_INSTALL_LIBDIR, I think what you need to do is to use only CMAKE_INSTALL_LIBDIR, as we already include LLVM_LIBDIR_SUFFIX inside it. If we do that change, we should do it globally though: there are a lot more lib${LLVM_LIBDIR_SUFFIX} cases that you did not update here. We should also do it independently of other changes to rpath.

(Alternatively we could drop the code that adds LLVM_LIBDIR_SUFFIX to CMAKE_INSTALL_LIBDIR, and then use them together, but that seems less clean to me.)

[RossComputerGuy]

This was reverted in e941b03 by @Ericson2314.

What has changed in your variant of the patch to address the failures that resulted in the previous revert?

I don't think anything yet, I'm trying to figure out what's wrong but I hadn't been able to. I was able to rebase and test failures went away.

The goal here is to properly get this patch into LLVM upstream and drop the patch from nixpkgs. This decreases the maintenance burden as we'd no longer need to rely on maintaining this patch.

It would be more helpful if the PR description explained what issue this patch is actually trying to solve. Presumably nixpkgs carries this patch for a reason :)

Yes, it's so that LLVM can work standalone. We install LLVM itself in a GNU style way. I'm not sure how to explain better but @Ericson2314 could.

This change is incorrect. You can't combine CMAKE_INSTALL_LIBDIR and LLVM_LIBDIR_SUFFIX, that is going to result in lib6464 libdirs.

If you want to use CMAKE_INSTALL_LIBDIR, I think what you need to do is to use only CMAKE_INSTALL_LIBDIR, as we already include LLVM_LIBDIR_SUFFIX inside it. If we do that change, we should do it globally though: there are a lot more lib${LLVM_LIBDIR_SUFFIX} cases that you did not update here. We should also do it independently of other changes to rpath.

(Alternatively we could drop the code that adds LLVM_LIBDIR_SUFFIX to CMAKE_INSTALL_LIBDIR, and then use them together, but that seems less clean to me.)

Ok, is there a good way to verify whether whatever I change is correct?