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

[question] GNUInstallDirs not working as expected with conan build and conan export-pkg #14733

Closed
1 task done
maitrey opened this issue Sep 14, 2023 · 14 comments · Fixed by #16214
Closed
1 task done

Comments

@maitrey
Copy link

maitrey commented Sep 14, 2023

What is your question?

Dear Conan Folks,

Conan Version: 2.0.5
I was using all this while conan create and with that all the GNUInstallDirs were somehow defined.
However, when I use the command conan build + conan export-pkg like this:

conan build . -of outputBuild --user autosar --channel featuresamefolder -pr:b=default -pr:h=kdp -c tools.build:skip_test=True

I see some definitions are not set : ${CMAKE_INSTALL_LIBDIR} , ${CMAKE_INSTALL_INCLUDEDIR} .
My recipe uses : cmake.build and cmake.install()
The documentation of CMake looks different from 1.x to 2.x:
https://docs.conan.io/2.0/reference/tools/cmake/cmake.html
https://docs.conan.io/1/reference/build_helpers/cmake.html?highlight=cmake_install_libdir

Recipe looks like this:

from conan import ConanFile
from conan.tools.files import copy
from conan.tools.scm import Git
from conan.tools.cmake import CMake, CMakeToolchain, CMakeDeps


class mytest(ConanFile):
    name = 'mytest'
    url = 'https://github.com/Rad6XX/mytest.git'
    exports_sources = "src/*", "archi/*", "doc/*", "inc/*", "test/*", "CMakeLists.txt", "report/*"
    settings = "os", "compiler", "arch", "build_type"

    def build(self):
        cmake = CMake(self)
        cmake.configure()
        cmake.build()
        if not self.conf.get("tools.build:skip_test", default=False):
            print("Tests are set to:", self.conf.get("tools.build:skip_test"))
            self.run("ctest --verbose --output-on-failure")
        cmake.install()


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

    def package_info(self):
        self.cpp_info.includedirs = ['include']
        self.cpp_info.libdirs = ['lib']
        self.cpp_info.objects = [os.path.join("lib", "mytest.c.obj")]
        self.cpp_info.srcdirs = ['src']
# EOF

And cmakelists looks like this:

cmake_minimum_required(VERSION 3.21.1)


project(mytest LANGUAGES C)
set(CMAKE_SYSTEM_NAME Generic)

add_library(mytest OBJECT ${CMAKE_CURRENT_SOURCE_DIR}/src/mytest.c)
target_include_directories(mytest PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/inc)
set(OBJS $<TARGET_OBJECTS:mytest>)
set_target_properties(mytest PROPERTIES PUBLIC_HEADER ${CMAKE_CURRENT_SOURCE_DIR}/inc/mytest.h)
install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/mytest.dir/src/  DESTINATION ${CMAKE_INSTALL_LIBDIR}/lib FILES_MATCHING PATTERN "*.obj")
install(FILES ${CMAKE_SOURCE_DIR}/inc/mytest.h DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/)
install(FILES ${CMAKE_SOURCE_DIR}/src/mytest.c DESTINATION src/)

Do I need to specify somewhere in some method explicitly?
Could someone please help me?

Thanks for your help!
Br
Maitrey

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@maitrey
Copy link
Author

maitrey commented Sep 14, 2023

It works if I add:

include(GNUInstallDirs)

to CmakeLists.txt

@memsharded
Copy link
Member

Hi @maitrey

Thanks for your question.
Not sure if I understood, your last comment about adding include(GNUInstallDirs), is that the solution for your problem? Or is there still some issue?

Some other side observation: You typically don't want to set set(CMAKE_SYSTEM_NAME Generic) in the CMakeLists.txt. This should be the responsibility of a toolchain file, not a CMakeLists.txt. The conan_toolchain.cmake might set a value for it if necessary.

@maitrey
Copy link
Author

maitrey commented Sep 18, 2023

Thats the solution to my problem. When I use conan build + conan export-pkg , and I use CMAKE_INSTALL_INCLUDEDIR it points to the output folder instead of outputfolder/include. If I include include(GNUInstallDirs) in CMakeLists.txt , only then it works.

Another point that I noted is, I need to check the dependencies in the recipes in order to exclude the include header of dependencies in the test reports(static code analysis):

compa = self.dependencies["compa"]
print("compa include dirs are  ", compa.cpp_info.includedirs)

This points to conan cache which is in my user home. I expected that it points to the folder specified in -of of the conan build command.

@maitrey
Copy link
Author

maitrey commented Sep 29, 2023

When I check the code of conan I only find one mention of

include(GNUInstallDirs)

in the tests:
https://github.com/conan-io/conan/blob/release/2.0/conans/test/functional/toolchains/cmake/cmakedeps/test_apple_frameworks.py#L440

@memsharded
Copy link
Member

Hi @maitrey

I am still not fully sure what would be the question here.

The cmake.install() does nothing but call to cmake ... --install ... --prefix .... It expects the CMakeLists.txt to be correct and define the installation folders correctly. If that means that include(GNUInstallDirs) has to be added, it is a responsibility of the CMakeLists.txt (it might not be always necessary, folders can be directly hardcoded in the CMakeLists.txt, and we have tests that check cmake.install() that don't necessary require the GNUInstallDirs).

Please let me know if this clarifies the question, thanks!

@maitrey
Copy link
Author

maitrey commented Nov 17, 2023

I use conan build -of along with conan export-pkg. If I use conan create I donot need to use include(GNUInstallDirs) .
I found that when I use conan create then the toolchain.cmake has:

build/Release/generators/conan_toolchain.cmake:set(CMAKE_INSTALL_LIBDIR "lib")

While when I used conan build -of without include(GNUInstallDirs) it fails with:

  install DIRECTORY given no DESTINATION!

The toolchain.cmake is not having the CMAKE_INSTALL_LIBDIR set.
When I use include(GNUInstallDirs) together with conan build -of + conan export-pkg , then i see that the CMakeCache.txt has the following variables set:

//User executables (bin)
CMAKE_INSTALL_BINDIR:PATH=bin

//Read-only architecture-independent data (DATAROOTDIR)
CMAKE_INSTALL_DATADIR:PATH=

//Read-only architecture-independent data root (share)
CMAKE_INSTALL_DATAROOTDIR:PATH=share

//Documentation root (DATAROOTDIR/doc/PROJECT_NAME)
CMAKE_INSTALL_DOCDIR:PATH=

//C header files (include)
CMAKE_INSTALL_INCLUDEDIR:PATH=include

//Info documentation (DATAROOTDIR/info)
CMAKE_INSTALL_INFODIR:PATH=

//Object code libraries (lib)
CMAKE_INSTALL_LIBDIR:PATH=lib

//Program executables (libexec)
CMAKE_INSTALL_LIBEXECDIR:PATH=libexec

//Locale-dependent data (DATAROOTDIR/locale)
CMAKE_INSTALL_LOCALEDIR:PATH=

//Modifiable single-machine data (var)
CMAKE_INSTALL_LOCALSTATEDIR:PATH=var

//Man documentation (DATAROOTDIR/man)
CMAKE_INSTALL_MANDIR:PATH=

//C header files for non-gcc (/usr/include)
CMAKE_INSTALL_OLDINCLUDEDIR:PATH=/usr/include

@Tails86
Copy link

Tails86 commented May 1, 2024

I have run into a similar issue. If I build with conan create ., everything works fine. If I build with conan build ., the generator file looks a little different, causing issues with conan export-pkg. For the below, the left file is the toolchain for a conan build . and the right is the toolchain file for a conan create ..

~/Project$ diff ./build/Release/generators/conan_toolchain.cmake /home/user/.conan2/p/b/proj22900c70d7aa1/b/build/Release/generators/conan_toolchain.cmake
108a109,117
> set(CMAKE_INSTALL_PREFIX "/home/user/.conan2/p/b/proj22900c70d7aa1/p")
> set(CMAKE_INSTALL_BINDIR "bin")
> set(CMAKE_INSTALL_SBINDIR "bin")
> set(CMAKE_INSTALL_LIBEXECDIR "bin")
> set(CMAKE_INSTALL_LIBDIR "lib")
> set(CMAKE_INSTALL_INCLUDEDIR "include")
> set(CMAKE_INSTALL_OLDINCLUDEDIR "include")
> 
> 

If we don't specify the CONAN_INSTALL_* variables in the CMakeLists.txt, then install and package fails when artifacts are created using conan build .. That is what include(GNUInstallDirs) is doing which is the root of what OP was getting at.

@memsharded - can you comment on this, please? Is it a bug or is there something that I need to be doing in the conanfile to get the two generator files to match? Obviously, I wouldn't want the prefix to be defined for conan build .. That is properly being set by conan export-pkg using the CMake --prefix option. I'd like all of the other CMAKE_INSTALL_ variables to be defined by the generator though.

@memsharded
Copy link
Member

Well, if I understand this correctly, this sounds something intrinsic to the CMake flow:

  • the conan build command is completely "local", at the moment of conan build there is no concept at all of a package_folder, this doesn't exist at all. If it had to be defined, at most it would be an invented local "package" folder, but even this would make the later install flow to fail.
  • The CMakeToolchain block OutputDirsBlock that defines all the CMAKE_INSTALL_XXXDIR is conditioned to the existing of package_folder, and it can only be defined when the package is being created in the local cache, when the package_folder is defined.
  • The normal conan new cmake_lib template works in the local flow because the cmake.install() defines the "--prefix=package_folder argument, and that is enough if not specifically depending on the CMAKE_INSTALL_XXXDIR variables.

I don't think that it is possible to do anything regarding this, it is an intrinsic issue of the CMake configuration + build + install flow, and unless a new re-build would be launched, it would be impossible for Conan to define those variables correctly, and in that case a conan create already works. Another alternative is making the CMake install functionality independent of those specific variables, or use copy() calls in the package() method instead of the CMake functionality.

I think this can be closed as "won't fix", because it looks like an unsolvable issue.

@Tails86
Copy link

Tails86 commented May 1, 2024

The CMakeToolchain block OutputDirsBlock that defines all the CMAKE_INSTALL_XXXDIR is conditioned to the existing of package_folder, and it can only be defined when the package is being created in the local cache, when the package_folder is defined.

I understand why CMAKE_INSTALL_PREFIX cannot be defined in this case, but is there a reason for preventing the others? They seem to normally be relative directory names which shouldn't rely on knowing if/where the package will be installed down the line.

@memsharded
Copy link
Member

I understand why CMAKE_INSTALL_PREFIX cannot be defined in this case, but is there a reason for preventing the others? They seem to normally be relative directory names which shouldn't rely on knowing if/where the package will be installed down the line.

Oh, I see what you mean now. It is true that it might be possible to define them with relative paths (it seems that in the cache they are defined with absolute paths, which is allowed by CMake, but defining relative paths is recommended apparently).

Let's have a look and try to make these variables relative paths and independent of the package_folder.

@memsharded
Copy link
Member

I am creating PR #16214, for next Conan release that defines such CMAKE_INSTALL_XXXDIRS, also for the local flow. If you want to give it a try from source code, that would also be good feedback, otherwise, it will be included in next Conan 2.4. Thanks for the feedback!

@memsharded
Copy link
Member

Closed by #16214, it will be in next Conan 2.4

@Tails86
Copy link

Tails86 commented May 22, 2024

Sorry for the delay in my response. I confirmed that #16214 corrects this issue on my end. Only set(CMAKE_INSTALL_PREFIX) differs between the two toolchain files now as expected.

~/Project$ diff ./build/Release/generators/conan_toolchain.cmake /home/user/.conan2/p/b/proj55613b9ec6cca/b/build/Release/generators/conan_toolchain.cmake
108a109
> set(CMAKE_INSTALL_PREFIX "/home/user/.conan2/p/b/proj55613b9ec6cca/p")

Thanks for making that correction!

@memsharded
Copy link
Member

Thanks for the feedback @Tails86!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants