ROCm / ROCm-CompilerSupport

The compiler support repository provides various Lightning Compiler related services.
47 stars 31 forks source link

Installation of LICENSE.txt in asan violates the FHS rule on Gentoo #61

Closed littlewu2508 closed 7 months ago

littlewu2508 commented 8 months ago

Hi, I observed that recent Gentoo package dev-libs/rocm-comgr omits QA warning:

 * QA Notice: The ebuild is installing to one or more unexpected paths:                                                                                                                                                                            
 *                                                                                                                                                                                                                                                 
 *   /usr/share/doc/rocm-comgr-5.7.1-asan                                                                                                                                                                                                          
 *                                                                                                                                                                                                                                                 
 * Please fix the ebuild to use correct FHS/Gentoo policy paths.

which is caused by https://github.com/RadeonOpenCompute/ROCm-CompilerSupport/blob/8276083301409001ec7643e68f5ad58b057c21fd/lib/comgr/CMakeLists.txt#L311

Is there a reason that installation must happen? It seems to be duplicated with https://github.com/RadeonOpenCompute/ROCm-CompilerSupport/blob/8276083301409001ec7643e68f5ad58b057c21fd/lib/comgr/CMakeLists.txt#L307

lamb-j commented 8 months ago

So the ASAN library can optionally be included, and right now the license file seems to install at ${CMAKE_INSTALL_DOCDIR}-asan, which breaks the FHS rule for Gentoo:

https://projects.gentoo.org/qa/policy-guide/filesystem.html File system layout — Gentoo Policy Guide documentation "Furthermore, within /usr/share/doc hierarchy only a subdirectory named after full package name and version with revision (PF) is permitted."

I'm looking into this to see if we can instead install the License somewhere valid (for Gentoo)

nunnikri commented 8 months ago

@lamb-j in the current naming scheme used by ROCm, there is no version added in share/doc folder like share/doc/rocm-comgr-5.7.1. Is this 5.7.1 version info mandatory for Gentoo policy?

lamb-j commented 8 months ago

I'm not exactly sure what Gentoo expects. What is it about /usr/share/doc/rocm-comgr-5.7.1-asan that's "breaking the rules"?

One suggestion from @Mystro256 was that it may expect this instead:

/usr/share/doc/rocm-comgr-asan-5.7.1/

But from reading the documentation, I'm wondering if we may need to use this instead:

/usr/share/doc/rocm-comgr-5.7.1/asan/ (asan in a subdirectory)

It seems like the directory under /usr/share/doc/ needs to have the exact name of the package. So we can't create two separate directories under /usr/share/doc from a single package

littlewu2508 commented 8 months ago

But from reading the documentation, I'm wondering if we may need to use this instead:

/usr/share/doc/rocm-comgr-5.7.1/asan/ (asan in a subdirectory)

It seems like the directory under /usr/share/doc/ needs to have the exact name of the package.

Exactly. It would be OK to treat asan as a sub folder

nunnikri commented 8 months ago

@lamb-j @littlewu2508 to give more light on how things are done in https://github.com/RadeonOpenCompute/ROCm-CompilerSupport/blob/amd-stg-open/lib/comgr/CMakeLists.txt. Using the same CMake file 2 packages are created, amd-comgr and amd-comgr-asan. Its handled using cpack components. So license files will be installed in 2 locations. share/doc/amd-comgr and share/doc/amd-comgr-asan. I am not familiar with Gentoo build env or the changes done for Gentoo support. From my basic understanding, looks like only one package is required for Gentoo. If thats the case you can just remove "COMPONENT asan" related code or put that in some condition check like if(ENABLE_ASAN_PACKAGING) . That should take care of the problem described here.

lamb-j commented 8 months ago

If they really are two separate packages, I'm then wondering if @Mystro256 is right in that we should just make the following change:

/usr/share/doc/rocm-comgr-5.7.1-asan -> /usr/share/doc/rocm-comgr-asan-5.7.1

Maybe the "unexpected path" is just not having the version number at the end (which seems to be standard).

@littlewu2508 any chance you could test manually setting "/usr/share/doc/rocm-comgr-asan-5.7.1" in your Gentoo build? If that works, we can update the CMake?

littlewu2508 commented 8 months ago

/usr/share/doc/rocm-comgr-5.7.1-asan -> /usr/share/doc/rocm-comgr-asan-5.7.1

Sorry, that did not help: the package name is rocm-comgr and /usr/share/doc/rocm-comgr-asan-5.7.1 does not match the package_name-package_version rule. And, we currently does not build the asan package.

I'm not sure why the LICENSE file for asan component is installed. I guess Gentoo is not using CPACK -- cmake ends its destiny after install files into a temporary directory, and the package manager "portage" takes over the actual installation.

nunnikri commented 8 months ago

Soft link I dont know is the right approach. I would suggest to remove the the asan license installation as Gentoo doesnt need asan package. Or you can add a condition like this in the cmake. if(ENABLE_ASAN_PACKAGING) install(FILES "LICENSE.txt" COMPONENT asan DESTINATION ${CMAKE_INSTALL_DOCDIR}-asan) endif()

lamb-j commented 8 months ago

Ah, wasn't trying to imply a soft link, just a name change. But I agree, we should only be installing ASAN-related licenses when also building the ASAN package. I've posted an internal code review with these changes. I'll try to get it mirrored out and linked here asap

lamb-j commented 7 months ago

Should be fixed with this update:

https://github.com/RadeonOpenCompute/llvm-project/commit/e782e09f7b113a0f896c6cec7240d411aca1d73f

Let me know if it's still an issue though and we can investigate more!