conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
7.95k stars 951 forks source link

[question] How to explicitly pass a build directory to `CMake.build()`? #16484

Closed PauloCarvalhoRJ closed 2 weeks ago

PauloCarvalhoRJ commented 2 weeks ago

Hello,

I have a recipe with code like this:

            cmake.configure(build_script_folder=os.path.join(self.build_folder, self._source_subfolder, 'contrib', 'minizip'),
                            cli_args=["-B\""+os.path.join(self.build_folder, self._source_subfolder, 'contrib', 'minizip')+"\"",
                                      "-DCMAKE_TOOLCHAIN_FILE=\""+os.path.join(self.build_folder, 'conan_toolchain.cmake')+"\"",
                                      "-DCMAKE_BUILD_TYPE="+str(self.settings.build_type)])

This is necessary because I have, in this case, to build a library called minizip in a subdirectory different from the main project's build directory (in this case, the main project is zlib). Configure works just fine. However, doing a cmake.build() after that I get this output:

zlib/1.2.11: RUN: cmake --build "C:\conan_packages\b\zlibe25c1ce7aa130\b" --config Release

Hence, it just redoes the build of the main projet (zlib) and the minizip library is not being built. The command above is supposed to be cmake --build "C:\conan_packages\b\zlibe25c1ce7aa130\b\source_subfolder\contrib\minizip" --config Release to match that of the -B parameter passed to the cmake.configure(...) call. So, how do I instruct build() to use a specific directory like I can to configure()?

thanks in advance,

PC

Have you read the CONTRIBUTING guide?

memsharded commented 2 weeks ago

Hi @PauloCarvalhoRJ

Thanks for your question. It is not very clear what you are trying to achieve. If you want to use minizip, there is already a recipe for it in ConanCenter, have you tried it?

Otherwise, it is not clear why you would pass "-DCMAKE_TOOLCHAIN_FILE=\""+os.path.join(self.build_folder, these arguments are already added by the CMake helper automatically.

Maybe if you provide the whole conanfile.py, it would be easier to understand the full picture. Thanks!

PauloCarvalhoRJ commented 2 weeks ago

Hi, @memsharded ,

I just need to be able to explicitly do a build in a subdirectory like described in my OP. It used to work in Conan 1 like charm, but for some reason it's being more difficult to work in Conan 2. Yes, I know ConanCenter, but its zlib and minizip recipes do not do what I need. I need, for example, to apply patches to certain source files which are specific to the source code kept in a local zip file.

Regarding the -DCMAKE_TOOLCHAIN_FILE=..., if I omit it, I get a file-not-found error in CMake. Again, this was an addition due to Conan 2. The original Conan 1 script didn't have it.

Here's my current zlib and minizip recipe:

# -*- coding: utf-8 -*-
import os
from conan import ConanFile
from conan.tools.cmake import CMake
from conan.tools.files import unzip, patch, chdir, replace_in_file, mkdir, load, save, copy
import shutil

class ZlibConan(ConanFile):
    name = "zlib"
    version = "1.2.11"
    url = "https://github.com/conan-io/conan-center-index"
    homepage = "https://zlib.net"
    license = "Zlib"
    description = ("A Massively Spiffy Yet Delicately Unobtrusive Compression Library "
                   "(Also Free, Not to Mention Unencumbered by Patents)")
    settings = "os", "arch", "compiler", "build_type"
    options = {"shared": [True, False],
               "fPIC": [True, False],
               "minizip": [True, False],
               "os_version": ["linux", "windows", "other"],
               }
    default_options = {"shared": False, "fPIC": True, "minizip": True}
    exports_sources = ["zlib-{}.tar.gz".format(version),
                       "CMakeLists.txt",
                       "CMakeLists_minizip.txt",
                       "patches/**"]
    generators = "CMakeDeps", "CMakeToolchain"
    source_url = "https://zlib.net/zlib-1.2.11.tar.gz"
    _source_subfolder = "source_subfolder"
    topics = ("conan", "zlib", "compression")
    short_paths = True
    build_requires = "cmake_installer/3.29.3"

    #---------------------------------------ZlibConan methods (not Conan API)----------------------------------------

    # Called by build(). Returns the name of the archive containing the zlib source code.
    @property
    def _source_zip_filename(self):
        return "zlib-{}.tar.gz".format(self.version)

    # Called by _build_zlib().
    def _build_zlib_cmake(self):
        cmake = CMake(self)
        #cmake.configure(build_dir=".")
        cmake.configure()
        # we need to build only libraries without test example/example64 and minigzip/minigzip64
        make_target = "zlib" if self.options.shared else "zlibstatic"
        #cmake.build(build_dir=".", target=make_target)
        cmake.build(target=make_target)

    # Called by build().
    def _build_zlib(self):

        # Apply patches.  These are changes to the source code applied in the build diretory 
        # (the original source files remain unchanged in the source repository).
        # See conandata.yml file that comes along this recipe in the source repository for info
        # on the patches to be applied.
        patches = self.conan_data["patches"][self.version]
        for p in patches:
            patch(self, base_path=p["base_path"], patch_file=p["patch_file"])

        with chdir(self, self._source_subfolder):
            # https://github.com/madler/zlib/issues/268
            replace_in_file(self, 'gzguts.h',
                                  '#if defined(_WIN32) || defined(__CYGWIN__)',
                                  '#if defined(_WIN32) || defined(__MINGW32__)')

            is_apple_clang12 = self.settings.compiler == "apple-clang" and tools.Version(self.settings.compiler.version) >= "12.0"
            if not is_apple_clang12:
                for filename in ['zconf.h', 'zconf.h.cmakein', 'zconf.h.in']:
                    replace_in_file(self, filename,
                                          '#ifdef HAVE_UNISTD_H    '
                                          '/* may be set to #if 1 by ./configure */',
                                          '#if defined(HAVE_UNISTD_H) && (1-HAVE_UNISTD_H-1 != 0)')
                    replace_in_file(self, filename,
                                          '#ifdef HAVE_STDARG_H    '
                                          '/* may be set to #if 1 by ./configure */',
                                          '#if defined(HAVE_STDARG_H) && (1-HAVE_STDARG_H-1 != 0)')
            #mkdir(self, "_build")
            #with chdir(self, "_build"): # this _build directory stays empty...
                #self._build_zlib_cmake()
            self._build_zlib_cmake()

    # Called by build().
    def _build_minizip(self):
        minizip_dir = os.path.join(self._source_subfolder, 'contrib', 'minizip')
        os.rename(os.path.join(self.source_folder, "CMakeLists_minizip.txt"), os.path.join(minizip_dir, 'CMakeLists.txt'))
        with chdir(self, minizip_dir):
            cmake = CMake(self)
            # It is necessary to pass minizip's build directory as cmake.configure() tries to build in zlib's own build directory
            # resulting in a CMake error. 
            # https://stackoverflow.com/questions/18826789/cmake-how-to-set-the-build-directory-to-be-different-than-source-directory
            # Is is also necessary to explicitly pass where conan_toochain.cmake used to bild main zlib is.
            # Somehow CMAKE_BUILD_TYPE is also not being set for the minizip library (Conan is supposed to do that but it doesn't).
            # This is somewhat messy in Conan 2.  In Conan 1 it was simpler.
            cmake.configure(build_script_folder=os.path.join(self.build_folder, self._source_subfolder, 'contrib', 'minizip'),
                            cli_args=["-B\""+os.path.join(self.build_folder, self._source_subfolder, 'contrib', 'minizip')+"\"",
                                      "-DCMAKE_TOOLCHAIN_FILE=\""+os.path.join(self.build_folder, 'conan_toolchain.cmake')+"\"",
                                      "-DCMAKE_BUILD_TYPE="+str(self.settings.build_type)])
            with chdir(self, os.path.join(self.build_folder, self._source_subfolder, 'contrib', 'minizip')):
                #cmake.build(cli_args=["--build \""+os.path.join(self.build_folder, self._source_subfolder, 'contrib', 'minizip')+"\""])
                cmake.build()
            cmake.install()

    # Called by package().
    # The different toolsets create different library names, so this method standardizes them.
    def _rename_libraries(self):
        if self.settings.os == "Windows":
            lib_path = os.path.join(self.package_folder, "lib")
            suffix = "d" if self.settings.build_type == "Debug" else ""

            if self.options.shared:
                #if self.settings.compiler == "Visual Studio":
                if self.settings.compiler == "msvc":
                    current_lib = os.path.join(lib_path, "zlib%s.lib" % suffix)
                    os.rename(current_lib, os.path.join(lib_path, "zlib.lib"))
            else:  # static libaries enabled (self.options.shared==False)
                #if self.settings.compiler == "Visual Studio":
                if self.settings.compiler == "msvc":
                    current_lib = os.path.join(lib_path, "zlibstatic%s.lib" % suffix)
                    os.rename(current_lib, os.path.join(lib_path, "zlib.lib"))
                elif self.settings.compiler == "gcc":
                    if self.settings.os != "Windows" or not self.settings.os.subsystem:
                        current_lib = os.path.join(lib_path, "libzlibstatic.a")
                        os.rename(current_lib, os.path.join(lib_path, "libzlib.a"))
                elif self.settings.compiler == "clang":
                    current_lib = os.path.join(lib_path, "zlibstatic.lib")
                    os.rename(current_lib, os.path.join(lib_path, "zlib.lib"))

    #---------------------------------------Methods of ConanFile's interface----------------------------------------
    def config_options(self):
        if self.settings.os == "Windows":
            del self.options.fPIC

    def configure(self):
        del self.settings.compiler.cppstd
        if self.settings.os == "Windows":
            self.options.os_version = "windows"
        elif self.settings.os == "Linux":
            del self.settings.compiler.libcxx
            self.options.os_version = "linux"
        else:
            del self.settings.compiler.libcxx
            self.options.os_version = "other"

#    def source(self):
#        self.output.info("Source code was exported (copied) with the recipe.")
#        self.output.info("From here: {}".format(self.source_url))

    def build(self):
        unzip(self, filename=os.path.join(self.source_folder, self._source_zip_filename), destination=self.build_folder, keep_permissions=True)
        shutil.move("zlib-{}".format(self.version), self._source_subfolder)
        self._build_zlib()
        if self.options.minizip:
            self._build_minizip()

    def package(self):
        # Extract the License/s from the header to a file
        with chdir(self, os.path.join(self.source_folder, self._source_subfolder)):
            tmp = load(self, "zlib.h")
            license_contents = tmp[2:tmp.find("*/", 1)]
            save(self, "LICENSE", license_contents)

        # Copy the license files
        copy(self, pattern="LICENSE", src=self._source_subfolder, dst="licenses")

        # Copy headers
        for header in ["*zlib.h", "*zconf.h"]:
            copy(self, pattern=header, dst="include", src=self._source_subfolder, keep_path=False)
            copy(self, pattern=header, dst="include", src="_build", keep_path=False)

        # Copying static and dynamic libs
        build_dir = os.path.join(self._source_subfolder, "_build")
        if self.options.shared:
            copy(self, pattern="*.dylib*", dst="lib", src=build_dir, keep_path=False, symlinks=True)
            copy(self, pattern="*.so*", dst="lib", src=build_dir, keep_path=False, symlinks=True)
            copy(self, pattern="*.dll", dst="bin", src=build_dir, keep_path=False)
            copy(self, pattern="*.dll.a", dst="lib", src=build_dir, keep_path=False)
        else:
            copy(self, pattern="*.a", dst="lib", src=build_dir, keep_path=False)
        copy(self, pattern="*.lib", dst="lib", src=build_dir, keep_path=False)

        self._rename_libraries()

    def package_info(self):
        self.cpp_info.libs.append("zlib" if self.settings.os == "Windows" and not self.settings.os.subsystem else "z")
        if self.options.minizip:
            self.cpp_info.libs.append('minizip')
            if self.options.shared:
                self.cpp_info.defines.append('MINIZIP_DLL')
        self.cpp_info.name = "zlib"

thank you for your time,

PC

PauloCarvalhoRJ commented 2 weeks ago

I can share my archive with the source so you can try it yourself. Just comment out the build_requires = "cmake_installer/3.29.3" line so the package becomes stand-alone (you, however, need to have Ninja and CMake binaries in your PATH environment variable).

PauloCarvalhoRJ commented 2 weeks ago

Here's the entire zlib/minizip package source: zlib_package.zip complete with patches and CMakeList.txts.

memsharded commented 2 weeks ago

Thanks for the reproducible code, that helps.

I have had some partial success by just adding:

add_subdirectory("source_subfolder/contrib/minizip")

to the main CMakeLists.txt when building minizip, plus some other minor changes. This helps simplifying the recipe too. Is this approach something that you would like? If yes, I can try finishing the proof of concept.

PauloCarvalhoRJ commented 2 weeks ago

Hello,

You're quite welcome. Well, if the recipe's package() and package_info() produce the same results (that is, the other dependencies' recipes will work fine), I believe you can do whatever you find more appropriate with Conan 2. That approach makes more sense now that you mentioned it.

Well, I thank you a lot for helping me sorting this out and to understand how Conan 2 works.

all the best,

PC

memsharded commented 2 weeks ago
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 82043d8..7bac748 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -3,3 +3,7 @@ project(conanzlib)

 include_directories(${CMAKE_SOURCE_DIR}/source_subfolder)
 add_subdirectory("source_subfolder")
+message(STATUS "BUILDING WITH MINIZIP ${BUILD_MINIZIP}")
+if(${BUILD_MINIZIP})
+    add_subdirectory("source_subfolder/contrib/minizip")
+endif()
diff --git a/CMakeLists_minizip.txt b/CMakeLists_minizip.txt
index 7ef5124..947e3b7 100644
--- a/CMakeLists_minizip.txt
+++ b/CMakeLists_minizip.txt
@@ -22,12 +22,11 @@ target_include_directories(minizip PRIVATE
   ${CMAKE_CURRENT_SOURCE_DIR}/../../_build
   ${CMAKE_CURRENT_SOURCE_DIR}/../../_build/source_subfolder)

-find_library(ZLIB_LIBRARY NAMES z zlib zlibd zlibstatic zlibstaticd PATHS
-  ${CMAKE_CURRENT_SOURCE_DIR}/../../_build
-  ${CMAKE_CURRENT_SOURCE_DIR}/../../_build/lib
-  ${CMAKE_CURRENT_SOURCE_DIR}/../..
-  ${CMAKE_CURRENT_SOURCE_DIR}/../../${CMAKE_BUILD_TYPE})
-
+if(BUILD_SHARED_LIBS)
+  set(ZLIB_LIBRARY zlib)
+else()
+  set(ZLIB_LIBRARY zlibstatic)
+endif()
 target_link_libraries(minizip PRIVATE ${ZLIB_LIBRARY})
 target_compile_definitions(minizip PRIVATE MINIZIP_BUILDING)
 if(BUILD_SHARED_LIBS)
diff --git a/conanfile.py b/conanfile.py
index 250bd50..cf1cb5a 100644
--- a/conanfile.py
+++ b/conanfile.py
@@ -1,7 +1,6 @@
-# -*- coding: utf-8 -*-
 import os
 from conan import ConanFile
-from conan.tools.cmake import CMake
+from conan.tools.cmake import CMake, CMakeToolchain
 from conan.tools.files import unzip, patch, chdir, replace_in_file, mkdir, load, save, copy
 import shutil

@@ -18,6 +17,7 @@ class ZlibConan(ConanFile):
     options = {"shared": [True, False],
                "fPIC": [True, False],
                "minizip": [True, False],
+               # FIXME: THe os_version seems redundant and unused, should be removed
                "os_version": ["linux", "windows", "other"],
                }
     default_options = {"shared": False, "fPIC": True, "minizip": True}
@@ -25,12 +25,12 @@ class ZlibConan(ConanFile):
                        "CMakeLists.txt",
                        "CMakeLists_minizip.txt",
                        "patches/**"]
-    generators = "CMakeDeps", "CMakeToolchain"
+    generators = "CMakeDeps"
     source_url = "https://zlib.net/zlib-1.2.11.tar.gz"
-    _source_subfolder = "source_subfolder"
+    _source_subfolder = "source_subfolder"  # TODO :This can be improved with "layout()" and "cmake_layout()"
     topics = ("conan", "zlib", "compression")
     short_paths = True
-    build_requires = "cmake_installer/3.29.3"
+    # build_requires = "cmake_installer/3.29.3"

     #---------------------------------------ZlibConan methods (not Conan API)----------------------------------------

@@ -39,6 +39,11 @@ class ZlibConan(ConanFile):
     def _source_zip_filename(self):
         return "zlib-{}.tar.gz".format(self.version)

+    def generate(self):
+        tc = CMakeToolchain(self)
+        tc.variables["BUILD_MINIZIP"] = bool(self.options.minizip)
+        tc.generate()
+
     # Called by _build_zlib().
     def _build_zlib_cmake(self):
         cmake = CMake(self)
@@ -48,10 +53,14 @@ class ZlibConan(ConanFile):
         make_target = "zlib" if self.options.shared else "zlibstatic"
         #cmake.build(build_dir=".", target=make_target)
         cmake.build(target=make_target)
+        if self.options.minizip:
+            cmake.build(target="minizip")

     # Called by build().
     def _build_zlib(self):

+        minizip_dir = os.path.join(self._source_subfolder, 'contrib', 'minizip')
+        os.rename(os.path.join(self.source_folder, "CMakeLists_minizip.txt"), os.path.join(minizip_dir, 'CMakeLists.txt'))
         # Apply patches.  These are changes to the source code applied in the build diretory 
         # (the original source files remain unchanged in the source repository).
         # See conandata.yml file that comes along this recipe in the source repository for info
@@ -82,27 +91,6 @@ class ZlibConan(ConanFile):
                 #self._build_zlib_cmake()
             self._build_zlib_cmake()

-    # Called by build().
-    def _build_minizip(self):
-        minizip_dir = os.path.join(self._source_subfolder, 'contrib', 'minizip')
-        os.rename(os.path.join(self.source_folder, "CMakeLists_minizip.txt"), os.path.join(minizip_dir, 'CMakeLists.txt'))
-        with chdir(self, minizip_dir):
-            cmake = CMake(self)
-            # It is necessary to pass minizip's build directory as cmake.configure() tries to build in zlib's own build directory
-            # resulting in a CMake error. 
-            # https://stackoverflow.com/questions/18826789/cmake-how-to-set-the-build-directory-to-be-different-than-source-directory
-            # Is is also necessary to explicitly pass where conan_toochain.cmake used to bild main zlib is.
-            # Somehow CMAKE_BUILD_TYPE is also not being set for the minizip library (Conan is supposed to do that but it doesn't).
-            # This is somewhat messy in Conan 2.  In Conan 1 it was simpler.
-            cmake.configure(build_script_folder=os.path.join(self.build_folder, self._source_subfolder, 'contrib', 'minizip'),
-                            cli_args=["-B\""+os.path.join(self.build_folder, self._source_subfolder, 'contrib', 'minizip')+"\"",
-                                      "-DCMAKE_TOOLCHAIN_FILE=\""+os.path.join(self.build_folder, 'conan_toolchain.cmake')+"\"",
-                                      "-DCMAKE_BUILD_TYPE="+str(self.settings.build_type)])
-            with chdir(self, os.path.join(self.build_folder, self._source_subfolder, 'contrib', 'minizip')):
-                #cmake.build(cli_args=["--build \""+os.path.join(self.build_folder, self._source_subfolder, 'contrib', 'minizip')+"\""])
-                cmake.build()
-            cmake.install()
-
     # Called by package().
     # The different toolsets create different library names, so this method standardizes them.
     def _rename_libraries(self):
@@ -153,34 +141,36 @@ class ZlibConan(ConanFile):
         unzip(self, filename=os.path.join(self.source_folder, self._source_zip_filename), destination=self.build_folder, keep_permissions=True)
         shutil.move("zlib-{}".format(self.version), self._source_subfolder)
         self._build_zlib()
-        if self.options.minizip:
-            self._build_minizip()

     def package(self):
         # Extract the License/s from the header to a file
+        build_dir = self.build_folder
+        lib = os.path.join(self.package_folder, "lib")
+        bin = os.path.join(self.package_folder, "bin")
+        include = os.path.join(self.package_folder, "include")
         with chdir(self, os.path.join(self.source_folder, self._source_subfolder)):
             tmp = load(self, "zlib.h")
             license_contents = tmp[2:tmp.find("*/", 1)]
             save(self, "LICENSE", license_contents)

         # Copy the license files
-        copy(self, pattern="LICENSE", src=self._source_subfolder, dst="licenses")
+        copy(self, pattern="LICENSE", src=self._source_subfolder, dst=os.path.join(self.package_folder, "licenses"))

         # Copy headers
         for header in ["*zlib.h", "*zconf.h"]:
-            copy(self, pattern=header, dst="include", src=self._source_subfolder, keep_path=False)
-            copy(self, pattern=header, dst="include", src="_build", keep_path=False)
+            copy(self, pattern=header, dst=include, src=self._source_subfolder, keep_path=False)
+            copy(self, pattern=header, dst=include, src="_build", keep_path=False)

         # Copying static and dynamic libs
-        build_dir = os.path.join(self._source_subfolder, "_build")
+        
         if self.options.shared:
-            copy(self, pattern="*.dylib*", dst="lib", src=build_dir, keep_path=False, symlinks=True)
-            copy(self, pattern="*.so*", dst="lib", src=build_dir, keep_path=False, symlinks=True)
-            copy(self, pattern="*.dll", dst="bin", src=build_dir, keep_path=False)
-            copy(self, pattern="*.dll.a", dst="lib", src=build_dir, keep_path=False)
+            copy(self, pattern="*.dylib*", dst=lib, src=build_dir, keep_path=False, symlinks=True)
+            copy(self, pattern="*.so*", dst=lib, src=build_dir, keep_path=False, symlinks=True)
+            copy(self, pattern="*.dll", dst=bin, src=build_dir, keep_path=False)
+            copy(self, pattern="*.dll.a", dst=lib, src=build_dir, keep_path=False)
         else:
-            copy(self, pattern="*.a", dst="lib", src=build_dir, keep_path=False)
-        copy(self, pattern="*.lib", dst="lib", src=build_dir, keep_path=False)
+            copy(self, pattern="*.a", dst=lib, src=build_dir, keep_path=False)
+        copy(self, pattern="*.lib", dst=lib, src=build_dir, keep_path=False)

         self._rename_libraries()

This is some preliminary proof of concept with the idea above. There are still some missing things, I started to update the package() method for example, that requires absolute paths now with the new copy() method, not relative. The test_package is also missing the upgrade. But the package seems to be being created including the minizip library.

PauloCarvalhoRJ commented 2 weeks ago

Hello,

Thank you very much, but when I do a git apply <diff_file> (diff_file contains the diffs posted by you), it gives a corrupt patch error. What am I missing? Or should I manually make the changes?

best,

PC

memsharded commented 2 weeks ago

I am not sure why git apply won't work, it should, I captured the patch with git diff > file.diff, and I always use those diffs with git apply. Github won't allow to upload diff files, I can try to rename it with the txt extension:

changes.txt

PauloCarvalhoRJ commented 2 weeks ago

So, I did a git apply changes.txt and there was no error but nothing happened... figures...

I just manually edited the diffs. It worked for such small diffs, but it would be a real trouble if I had a larger changeset.

The build with the updated files went on and stopped at the errors in the test_package sub-package like you said. I'll work on it myself based on what I learned so far. Thanks again for kindly helping me.

PauloCarvalhoRJ commented 2 weeks ago

Getting rid of the messy _build_minizip() was a major improvement. The code is now more elegant than before.

memsharded commented 2 weeks ago

Thanks again for kindly helping me.

Happy to help :)

Note there will be still some other pending things to upgrade:

Getting rid of the messy _build_minizip() was a major improvement. The code is now more elegant than before.

I agree. There might be other simplification opportunities in the recipe: