[bug] resdirs not working how user expects

[paulharris]

Environment Details (include every applicable attribute)

• Operating System+version: Linux
• Compiler+version: gcc
• Conan version: 1.50.0
• Python version: 3.good.one

See #11471 copying the comment stream in here, as it isn't a simple obvious fix for me.

@lasote I think you need to look at this again. This change broke my vtk recipe (expectedly) as it was not setting resdirs, and I had assumed the files would end up in "res" eg "res/licences"

However, the files now end up packagedir/None/licences ! What is the "None" doing there? What is an empty resdirs supposed to mean exactly? Should the files end up in packagedir/licences going forward?

---

Also, side note, there doesn't seem to be any docs about adjusting dirs (in the layout(self) method) AND using the cmake_layout(self) call.

eg: https://docs.conan.io/en/latest/migrating_to_2.0/recipes.html?highlight=resdirs does not include any mention of cmake_layout(self)

Nor this page: https://docs.conan.io/en/latest/reference/conanfile/tools/cmake/cmaketoolchain.html?highlight=resdirs

It is not clear if further adjustments should be done before, or after cmake_layout(self) ... ie it is not clear if cmake_layout will overwrite any previous settings, or will incorporate them...

---

Last problem, the migration docs say You can adjust the cpp_info in the layout method too, not only for a package in the cache, that was typically done in the package_info() method using the self.cpp_info,

but I have found setting self.cpp_info.components[comp].resdirs = ["res"] has no effect, yet that pattern is symmetrical with the old setting for libdirs.

---

I changed my recipe to:

def layout(self):
   cmake_layout(self)
   self.cpp.package.resdirs = ["res"]

And I removed these calls in package(self):

self.cpp_info.components[comp].resdirs = ["res"]

ie "res" is only defined in one place.

This resulted in an empty folder packagedir/res/licences - right location, wrong content. The licence files were put in packagedir/licences

ie, no more "None" in the folder path.

I put back the cpp_info...resdirs calls, that didn't help either.

I can't figure out how to do this ...

The recipe (with resdirs lines commented out) is here for reference: https://github.com/conan-io/conan-center-index/pull/10776 Still doesn't build in the CI as the cgns recipe has unexpected errors in the CI.

[paulharris]

I think it is because cmake_flags.py and blocks.py are setting CMAKE_INSTALL_DATAROOTDIR but actually the correct setting is CMAKE_INSTALL_DATADIR

However, if I adjust both locations to the correct variable name, the licence files now get written to share/licences, and not res/licences as I asked for...

[lasote]

What is the "None" doing there?

There is a bug in the CMakeToolchain, if the "res" is not set it shouldn't adjust the CMAKE_INSTALL_DATAROOTDIR.

It is not clear if further adjustments should be done before, or after cmake_layout(self) ... ie it is not clear if cmake_layout will overwrite any previous settings, or will incorporate them...

The generate() method, where all the generator files are created, including the toolchain file from CMakeToolchain, is run after calling layout(). Calling the cmake_layout before or after adjusting the self.cpp.package should be the same.

I have to discuss with the team the convenience of restoring the "res" default. Honestly, the cmake.install using the cpp_info stuff to know where to package stuff is a bit weird.

This resulted in an empty folder packagedir/res/licences - right location, wrong content. The licence files were put in packagedir/licences

self.cpp.package.resdirs = ["res"] should work, we will investigate it.

[lasote]

Hi, I made a small tests to try to reproduce the issue installing the license but I cannot reproduce it, here is the test:

def test_resdirs_cmake_install():
    client = TestClient(path_with_spaces=False)

    conanfile = textwrap.dedent("""
            from conan import ConanFile
            from conan.tools.cmake import CMakeToolchain, CMake, cmake_layout

            class AppConan(ConanFile):
                settings = "os", "compiler", "build_type", "arch"
                exports_sources = "CMakeLists.txt", "my_license"
                name = "foo"
                version = "1.0"

                def generate(self):
                    tc = CMakeToolchain(self)
                    tc.generate()

                def layout(self):
                    self.cpp.package.resdirs = ["res"]

                def build(self):
                    cmake = CMake(self)
                    cmake.configure()
                    cmake.build()

                def package(self):
                    cmake = CMake(self)
                    cmake.install()
            """)

    cmake = """
    cmake_minimum_required(VERSION 3.15)
    set(CMAKE_CXX_COMPILER_WORKS 1)
    project(foo)
    message("DATAROOTDIR: ${CMAKE_INSTALL_DATAROOTDIR}")
    install(FILES my_license DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/licenses)
    """

    client.save({"conanfile.py": conanfile, "CMakeLists.txt": cmake, "my_license": "MIT"})
    client.run("create .")
    assert "/res/licenses/my_license" in client.out
    assert "Packaged 1 file: my_license" in client.out

It is kind of doing the same thing to install the license than that the original vtk CMakeLists.txt.

The my_license gets copied in the right place. Any feedback?

[paulharris]

@lasote so sorry, I had this on my TODO list but I couldn't find the PR/Issue until now.

Feedback: what happens if you change layout to:

def layout(self):
   cmake_layout(self)

I'm not sure if my conan is still patched/hacked, but now I get errors about file cannot create directory: /licenses

I see there has been changes since my report, just wanted to touch base before I dive deeper in here again.

[lasote]

@paulharris Sorry for the delay. I'm able to reproduce it. The issue comes from not declaring resdirs, causing the CMakeToolchain to not set the CMAKE_INSTALL_DATAROOTDIR. If the CMakeLists.txt contains something like install(FILES my_license DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/licenses) means that CMake is supposed to write at /licenses which is forbidden in the operating system. You can call cmake_layout(self) and then adjust the resdirs:

                def layout(self):
                    cmake_layout(self)
                    self.cpp.package.resdirs = ["res"]

So every time you need to install something with CMake you need to provide a resdirs.

[paulharris]

Wouldn't it be better to provide a default, even a bad default, that would help recipe writers to figure out what needs to happen?

Perhaps set the default resdirs to ["you_should_set_self_cpp_package_resdirs_in_layout"] so that the folders created will include a very strangely worded folder that points the user in the right direction.

or even set the resdirs to ["refer_to_conan_docnote_12345"]

Any kind of bread crumb for a developer to follow, rather than just effectively undefined behaviour...

[lasote]

We have been reviewing this. As I said, we only adjust CMAKE_INSTALL_DATAROOTDIR when there is a resdir defined, otherwise the variable is not filled. That variable should/might have a default value when calling GNUInstallDirs in the CMakeLists.txt so the result directory is never empty. We consider that Conan is not here to improve how difficult is to debug an unset variable in CMake, we understand that it is indeed hard, but CMake it is like it is. We won't pollute the cppinfo model to improve the traceability of a CMake error.

[jcar87]

Hi @paulharris - thanks for bringing this very interesting issue to our attention.

The CMAKE_INSTALL* variables typically used in install() directives are not initialised by default, the project is expected to set them, with a lot of projects choosing to rely on the values set by the GNUInstallDirs module (docs here). The values are expected to be relative to the install_prefix.

From what I can see, the top-level CMakeLists.txt inside the VTK project does something like this:

cmake_minimum_required(VERSION 3.12...3.16 FATAL_ERROR)

project(VTK)

include(GNUInstallDirs)

set(CMAKE_INSTALL_LICENSEDIR ""
  CACHE STRING "License files (DATAROOTDIR/licenses/${CMAKE_PROJECT_NAME}")
mark_as_advanced(CMAKE_INSTALL_LICENSEDIR)
if (NOT CMAKE_INSTALL_LICENSEDIR)
  set(CMAKE_INSTALL_LICENSEDIR
    "${CMAKE_INSTALL_DATAROOTDIR}/licenses/${CMAKE_PROJECT_NAME}")
endif ()

The inclusion of GNUInstallDirs is causing the CMAKE_INSTALL_DATAROOTDIR to be initialised to share. CMake will interpretet this to be relative to the install prefix when the install rules are run - so in absence of an explicit override, the license files would be copied to {INSTALL_PREFIX}/share/licenses/VTK/. Is this not what is happening? - At package/install time, this prefix should be the package root.

If res is specified in the layout like this:

self.cpp.package.resdirs = ["res"]

I'd expect CMAKE_INSTALL_DATAROOTDIR to be res and the files to end up copied in {INSTALL_PREFIX}/res/licenses/.

What I fail to see is a scenario where the install target would attempt to write to /licenses relative to the root of the filesystem - if you have steps to reproduce this that would be great!

[paulharris]

Appears to not be a problem anymore :)