cmake: add compiler + remove compiler.version to reduce number of builds #1783

madebr · 2020-05-29T22:16:12Z

Specify library name and version: cmake/all

For #1705 : by adding compiler to the package_id, the ci will/should build a package for Visual Studio where compiler.runtime is not a debug runtime

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

conan-center-bot · 2020-05-29T22:18:24Z

Failure in build 1 (88872e5df4df3fecafcb3f94a1c5596a4bd59d92):
cmake/3.16.2:

Error processing recipe: Linux x86_64, Release, gcc 5, libstdc++
Unexpected error happened:

ERROR: cmake/3.16.2: Error in package_id() method, line 59
	del self.info.compiler.version
	AttributeError: 'ConanInfo' object has no attribute 'compiler'

prince-chrismc · 2020-05-30T02:13:47Z

recipes/cmake/3.x.x/conanfile.py

@@ -54,6 +54,10 @@ def package(self):
        cmake.install()
        tools.rmdir(os.path.join(self.package_folder, "doc"))

+    def package_id(self):
+        # CMake is a executable-only package, so the compiler version does not matter


It matters since a system might be missing the runtimes. It won't fix the related issue. CCI will build one MSVC package using the 2015 runtimes, but the target system is a 2017 so it won't be able to execute

Building with 2015 is fine, it’ll work with the 2015 - 2019 runtimes.

https://docs.microsoft.com/en-us/cpp/porting/binary-compat-2015-2017?view=vs-2019

Removing the compiler ver may not be a best practice, because if CCI is upgraded to build with 2017+ and the system only had the 2015 runtime, that wouldn’t work.

That's a very good point, however these recipes can be used to generate a package for 2013 locally, and the same scenario would cause trouble.

To what runtime do the official cmake windows builds link?
The only problem I see with removing version in package_id, is the loss of reproduceability as the ci can build cmake using whatever version of msvc it pleases.

Would it make sense to only allow static runtime? Or should we just build all variants of cmake and don't bother with limiting anything?

https://gitlab.kitware.com/cmake/cmake/-/tree/master/.gitlab/ci looks like vs2019

Looks like the official build uses the static runtime. This is the latest release:

I'd prefer to build static runtime for CMake (and built tools in general), this will avoid a dependency on Visual Studio CRT redist, which is not necessarily available (especially, if I am using alternate compiler).

conan-center-bot · 2020-05-30T06:55:27Z

All green in build 2 (4a209d5aa0f8c9b8db67e8f1da2c3885502662b8)! 😊

cmake/3.16.2: Generated 14 packages. All logs here
cmake/3.16.3: Generated 14 packages. All logs here
cmake/3.16.4: Generated 14 packages. All logs here
cmake/3.16.5: Generated 14 packages. All logs here
cmake/3.16.6: Generated 14 packages. All logs here
cmake/3.16.7: Generated 14 packages. All logs here
cmake/3.17.0: Generated 14 packages. All logs here
cmake/3.17.1: Generated 14 packages. All logs here
cmake/3.17.2: Generated 14 packages. All logs here
cmake/3.17.3: Generated 14 packages. All logs here

@robert-shade

Reported by @robert-shade

cmake does not build without openssl-devel

recipes/cmake/3.x.x/conanfile.py

robert-shade · 2020-05-30T20:48:57Z

recipes/cmake/3.x.x/conanfile.py

@@ -14,6 +14,8 @@ class CMakeConan(ConanFile):
    generators = "cmake"
    settings = "os", "arch", "compiler", "build_type"

+    requires = "openssl/1.1.1g"


openssl isn't enabled by default on Windows, I think this requires needs to be os specific

Thanks for the link.
I'll have to add the CMAKE_USE_OPENSSL option.

conan-center-bot · 2020-05-30T21:20:09Z

Some configurations of 'cmake/3.16.2' failed in build 3 (92b1b37f046c3da05792f594a29b7d8a651146a0):

Linux x86_64, Release, gcc 5, libstdc++
Access to all the logs

conan-center-bot · 2020-05-31T01:25:30Z

Some configurations of 'cmake/3.16.2' failed in build 4 (08c4f3c234db6591994fd780e28fc8952e7da26d):

Windows x86_64, Debug, Visual Studio 14, MTd
Access to all the logs

rweickelt · 2020-05-31T10:51:09Z

Would it be possible to make this package entirely self-contained (only build dependencies, only os and host architecture settings)? Would it be possible to link all dependencies statically, incl. runtime libs?

The same applies to many binary tool packages. The reason I ask this is that I may want to use binary tools without even having a suitable compiler profile set up on my host or when using an exotic toolchain that the package has not been compiled for. Otherwise I would have to force certain settings just for these packages which is rather inconvenient.

That being said, the CMake package should only come in 3 variants: Macos, Windows, Linux and architecture x86_64. It might make sense to build it for Linux arm as well.

… runtime

conan-center-bot · 2020-05-31T19:45:53Z

All green in build 6 (ecbb8476021fc3cc4d64c01165cfd04f6c863f33)! 😊

cmake/3.16.2: Generated 6 packages. All logs here
cmake/3.16.3: Generated 6 packages. All logs here
cmake/3.16.4: Generated 6 packages. All logs here
cmake/3.16.5: Generated 6 packages. All logs here
cmake/3.16.6: Generated 6 packages. All logs here
cmake/3.16.7: Generated 6 packages. All logs here
cmake/3.17.0: Generated 6 packages. All logs here
cmake/3.17.1: Generated 6 packages. All logs here
cmake/3.17.2: Generated 6 packages. All logs here
cmake/3.17.3: Generated 6 packages. All logs here

SSE4 · 2020-06-01T03:50:56Z

@rweickelt I agree, valid points

madebr · 2020-06-02T14:02:56Z

I think I addressed @rweickelt 's concerns.
cmake is now buit in the following configurations:

Windows Release MSVC MT
Linux Release gcc libstdc++
Linux Release gcc libstdc++11
Linux Release clang libc++
Linux Release clang libstdc++
Macos Release apple-clang libc++

I think this covers all spectrum and still allows each user to build cmake with his compiler of choice.

robert-shade · 2020-06-02T14:57:30Z

Isn't keeping build_type as a setting and making debug an invalid setting going to cause issues for people building with a debug build_type?

robert-shade · 2020-06-02T17:47:29Z

Also, if we're removing the compiler version from the package_info, do we not also need to use the static runtime for the other platforms? They're going to run into the same issues if we, for example, try to run a gcc9 compiled binary on a gcc5 system.

Edit: Seems like the same conversation going on in #213

rweickelt · 2020-06-02T18:26:12Z

I think I addressed @rweickelt 's concerns.
cmake is now buit in the following configurations:

Windows Release MSVC MT

Linux Release gcc libstdc++

Linux Release gcc libstdc++11

Linux Release clang libc++

Linux Release clang libstdc++

Macos Release apple-clang libc++

I think this covers all spectrum and still allows each user to build cmake with his compiler of choice.

@madebr, thanks, but what if my (consuming) recipe is built with the SDCC or IAR or an Intel toolchain and has build_type=Debug? There are a ton of toolchains out there and for using the CMake package, this should not matter. Neither should the build type of my consuming recipe. They should simply pull a CMake binary package from bintray that works on my host os and arch.

So I am wondering: The recipe contains settings = "os", "arch", "compiler", "build_type" and in packaged_id() you remove only compiler_version.

That forms still a hard connection to the compiler type in the consuming recipe. Is that assumption correct? Can we get rid of that entirely and make it optional? If consumer recipes need CMake to be built with a specific toolchain, they may specify that as an option, but in 99.9% of the cases (assuming that there are no hidden run-time dependencies), they won't care.
Does build_type affect the package_id? If yes, can make it optional?

Note that I assume that this package will always and only be used as a build-only dependency in other recipes. In that case, the toolchain used for creating this (CMake) package does absolutely not matter. It is sufficient that a working binary package for the host os and architecture is available on bintray.

Thanks.

madebr · 2020-06-02T19:24:15Z

I think I addressed @rweickelt 's concerns.
cmake is now buit in the following configurations:

Windows Release MSVC MT

Linux Release gcc libstdc++

Linux Release gcc libstdc++11

Linux Release clang libc++

Linux Release clang libstdc++

Macos Release apple-clang libc++

I think this covers all spectrum and still allows each user to build cmake with his compiler of choice.

@madebr, thanks, but what if my (consuming) recipe is built with the SDCC or IAR or an Intel toolchain and has build_type=Debug? There are a ton of toolchains out there and for using the CMake package, this should not matter. Neither should the build type of my consuming recipe. They should simply pull a CMake binary package from bintray that works on my host os and arch.

This will introduce a limitation if you use cmake as a requirement (host os/arch).
But cmake should really only be used as a build requirement (build os/arch).

So I am wondering: The recipe contains settings = "os", "arch", "compiler", "build_type" and in packaged_id() you remove only compiler_version.

This is a way to make this recipe build a specific os/arch/build_type but don't let the the compiler.version matter.
You're probably right that also compiler.runtime and build_type should be removed in package_id.

1. That forms still a hard connection to the `compiler` type in the consuming recipe. Is that assumption correct? Can we get rid of that entirely and make it optional? If consumer recipes need CMake to be built with a specific toolchain, they may specify that as an option, but in 99.9% of the cases (assuming that there are no hidden run-time dependencies), they won't care.

If this recipe is used as a build requirement, then there is no connection between the settings of the consuming recipe and that of cmake.

2. Does `build_type` affect the `package_id`? If yes, can make it optional?

It does affect, but the current patches only allow Debug. So it is a constant. So it can be removed.

Note that I assume that this package will always and only be used as a build-only dependency in other recipes. In that case, the toolchain used for creating this (CMake) package does absolutely not matter. It is sufficient that a working binary package for the host os and architecture is available on bintray.

Indeed.

@jgsogo @uilianries @SSE4 @Croydon
Can I have your opinions/views?

My view is that the recipe should include os, arch, compiler, build_type but the CI should anyhow only build compiler.runtime=MT, build_Type=Release and only for Linux/Macos/Windows.
Imho, this recipe should store some extra meta data such that the CI will only build 3 recipes.

e.g. the CI should only build one combination if the property ConanFile.is_installer == True

rweickelt · 2020-06-02T19:37:56Z

@madebr, thanks, but what if my (consuming) recipe is built with the SDCC or IAR or an Intel toolchain and has build_type=Debug? There are a ton of toolchains out there and for using the CMake package, this should not matter. Neither should the build type of my consuming recipe. They should simply pull a CMake binary package from bintray that works on my host os and arch.

This will introduce a limitation if you use cmake as a requirement (host os/arch).
But cmake should really only be used as a build requirement (build os/arch).

I think we agree that the CMake package should only be a build requirement.

If this recipe is used as a build requirement, then there is no connection between the settings of the consuming recipe and that of cmake.

But if compiler information is still part of the package_id, how would my consuming recipe (which may use completely different compiler settings) be able to deduce the package_id of the CMake package? That confuses me.

jgsogo

Hi! Sorry for the delay. Too much work piled up.

We currently don't have a way to identify "installer packages", so they are exactly the same as any other package... and, given it is a Conan limitation, and we don't have (yet) time/build time constraints, I won't limit the number of binaries generated. We are thinking about a way to flag these and other types of packages, but it would be something for Conan 2.0, not before.

With the new cross-building approach, where the build context requires a full profile, I wouldn't avoid (by raising ConanInvalidConfiguration) any configuration that works to build the library. The problem here is that this build_requires can belong to a big graph with other build_requirements and some configuration (debug, MD,...) can be needed for them.... and Conan applies the same build-profile for all the packages in the build context. We cannot block that big graph just because one of the packages cannot compile in debug mode.

If it takes too much time to generate all the packages, this is something we can think about and see how can we improve it in Conan Center infrastructure and, if needed, we might add something to Conan itself.

recipes/cmake/3.x.x/conanfile.py

madebr · 2020-06-15T18:39:46Z

recipes/cmake/3.x.x/conanfile.py

+
+        del self.info.settings.compiler
+        del self.info.settings.build_type


@jgsogo @SSE4
By not removing compiler and build_type, at least 50 packages of cmake will be built for each version.

Suggested change

del self.info.settings.compiler

del self.info.settings.build_type

Yes. I understand that the desired scenario would be to remove these settings but to be able to tell C3i that we want to build the Release build.... that way, when someone retrieves the package from CCI it will be the good one.

This is not possible right now, the only alternative is to build them all 😞

I'll apply both suggestions then.

Is there already an issue open on github about this?

No, none of the two issues here, but both are really hard to tackle.

From the CCI perspective we don't want to run a different pipeline based on custom attributes and Conan right now doesn't have a feature to classify packages (and IMO we don't need it yet). The other issue: being able to assign different build profiles to different branches of the graph, requires a new graph model, something we are working for Conan 2.0, but it will require time to implement.

So, for the time being, we are building ~100 packages for each CMake version even though we will probably use only a dozen of them

Can't you just add an entry in the top-level yml file for telling cci which permutations to build? If 'compiler' and 'build_type' are removed from the exported 'package_id', no consumer should ever need to rebuild this package, no matter what profile they use. Or am I missing something?

We can think about different alternatives to tell CCI which configurations to build and probably it is about something in the config.yml file as you have suggested. OTH, if we remove build_type from the package ID we cannot guarantee (at this moment) that CCI will choose Release build (this was the issue originating this PR, I think).

Even though these things might look almost trivial, they are not as straightforward as they look like, and there are other things in the backlog with higher priority. We would really want to address all these issues, but we need to go step by step.

conan-center-bot · 2020-06-16T00:41:49Z

Some configurations of 'cmake/3.17.2' failed in build 11 (d8e61fcc243a71e828dba21be90cdd42ab5ad280):

Windows x86_64, Debug, Visual Studio 14, MTd
Access to all the logs

madebr · 2020-06-16T15:50:19Z

@uilianries @danimtb @jgsogo
The error is the same as #1783 (comment)

It looks like the CI/conan is ignoring some errors while extracting the sources.

conan-center-bot · 2020-06-16T17:45:09Z

An unexpected error happened and has been reported. Help is on its way! 🏇

conan-center-bot · 2020-06-17T14:57:29Z

All green in build 13 (d8e61fcc243a71e828dba21be90cdd42ab5ad280)! 😊

cmake/3.16.2: Generated 68 packages. All logs here
cmake/3.16.3: Generated 68 packages. All logs here
cmake/3.16.4: Generated 68 packages. All logs here
cmake/3.16.5: Generated 68 packages. All logs here
cmake/3.16.6: Generated 68 packages. All logs here
cmake/3.16.7: Generated 68 packages. All logs here
cmake/3.17.0: Generated 68 packages. All logs here
cmake/3.17.1: Generated 68 packages. All logs here
cmake/3.17.2: Generated 68 packages. All logs here
cmake/3.17.3: Generated 68 packages. All logs here

Croydon · 2020-06-21T11:36:02Z

Currently I don't have much time for Conan related issues, but I'm kinda surprised by this issue to begin with

Shouldn't static linking handling this?

Adding now all combinations for all installer packages feels fundamental wrong

madebr · 2020-06-21T11:46:34Z

Cci build systems are missing functionality to not build combinations, even though the recipe allows it to.

@Croydon

Adding now all combinations for all installer packages feels fundamental wrong

It feels not.
This allows one to debug an installer package.
CMake, as all programs, contain bugs so this allows a consumer to debug it.
Whether Bintray provides a debug package by default is another problem.

We should separate what we want the recipe to build and what we want to provide on bintray.

jgsogo · 2020-06-22T10:25:22Z

Hi! Back here, first of all I want to apologize for introducing too much noise in my last comments (and more on this one). We've been talking about this issue because we had different opinions about these installers and we need to agree on how we want to handle them in Conan Center.

We think that the best approach is as follows:

Note.- Here I'm referring to packages that only contain executables, if they contain libraries that are consumed by other packages then they should be managed as regular packages. So, let's talk about pure installers, packages with executables only.

We want these packages to show that Conan is able to retrieve the same binary for different configurations, that you don't need to build from sources for every change in the configuration (if all you want is an executable).
These packages will list all the settings (os, arch, compiler, build_type) because all of them generate different artifacts, the package must compile and take into account all the combinations for these settings. We cannot raise a ConanInvalidConfiguration because it can block graphs with other packages in the build context. User building from sources must generate the binary that corresponds to given configuration.
These packages will remove compiler and build_type from the package_id.
Conan Center CI will iterate the profiles in a given order and will build the binaries for the first configuration that returns every different package-ID. This will ensure that these packages will be compiled with Release, static and a known compiler version. glibc shouldn't be a problem after Use base image for all compiler version conan-docker-tools#204 (probably before that we will use the lower compiler version to minimize this error)
If these packages are required by others (because besides de executable it is generating other libraries) we will stop removing compiler and build_type in order to generate all the binaries again. Of course, we can consider other use-cases once they are known.

The main issue with this approach right now is that we cannot guarantee the order CCI uses to iterate the profiles, it means that we cannot be sure that the chosen configuration will be the one we expect. We are working on this, but it will take time (two weeks?). Meanwhile, we can stop this PR, or we can generate all these settings combinations and remove them afterward when the functionality required is implemented in CCI.

I hope this makes sense to everybody.

Thanks for pushing so hard!

Croydon · 2020-06-23T06:35:08Z

This will ensure that these packages will be compiled with Release, static and a known compiler version.

Finally, thanks! ❤️

recipes/cmake/3.x.x/conanfile.py

jgsogo

I think we need this PR while we ensure that profiles in CCI are iterated in order.

madebr mentioned this pull request May 29, 2020

[package] cmake/3.16.3: not working for Windows #1705

Closed

conan-center-bot added the Failed label May 29, 2020

cmake: add compiler + remove compiler.version to reduce number of builds

4a209d5

madebr force-pushed the cmake_compiler branch from 88872e5 to 4a209d5 Compare May 29, 2020 22:19

prince-chrismc reviewed May 30, 2020

View reviewed changes

conan-center-bot removed the Failed label May 30, 2020

madebr added 3 commits May 30, 2020 22:25

cmake: don't import Version

570f312

cmake: build cmake from the root directory

213a27b

Reported by @robert-shade

cmake: add openssl to requires

92b1b37

cmake does not build without openssl-devel

robert-shade reviewed May 30, 2020

View reviewed changes

conan-center-bot added the Failed label May 30, 2020

cmake: add openssl dependency optionally

2931d4d

cmake: patch cmakelists to include conanbuildinfo.cmake

aaf55f8

madebr force-pushed the cmake_compiler branch from 08c4f3c to aaf55f8 Compare May 31, 2020 01:45

cmake: limit to Release build_type + only allow static non-debug MSVC…

ecbb847

… runtime

conan-center-bot removed the Failed label May 31, 2020

madebr mentioned this pull request Jun 1, 2020

Add swig/4.0.1 #1717

Merged

4 tasks

jgsogo reviewed Jun 15, 2020

View reviewed changes

madebr commented Jun 15, 2020

View reviewed changes

recipes/cmake/3.x.x/conanfile.py Outdated Show resolved Hide resolved

madebr commented Jun 15, 2020

View reviewed changes

cmake: allow all os/build_type/compiler

d8e61fc

conan-center-bot added the Failed label Jun 16, 2020

conan-center-bot added the Unexpected Error label Jun 16, 2020

conan-center-bot removed Unexpected Error Failed labels Jun 17, 2020

This was referenced Jun 21, 2020

[package] Many packages missing pdbs...strategy? #1982

Open

ninja: allow any MSVC runtime when consuming ninja #1985

Merged

uilianries reviewed Jun 23, 2020

View reviewed changes

recipes/cmake/3.x.x/conanfile.py Show resolved Hide resolved

uilianries approved these changes Jun 23, 2020

View reviewed changes

jgsogo mentioned this pull request Jun 26, 2020

[service] ninja windows binary outdated #2049

Closed

jgsogo approved these changes Jul 2, 2020

View reviewed changes

danimtb approved these changes Jul 2, 2020

View reviewed changes

danimtb merged commit 13adb10 into conan-io:master Jul 2, 2020

madebr mentioned this pull request Jul 14, 2020

Add lemon/3.32.3 recipe #2201

Merged

4 tasks

madebr deleted the cmake_compiler branch July 20, 2020 13:41

madebr mentioned this pull request Sep 1, 2020

[question] cmake packages are dependent on compiler? #2752

Open

Croydon mentioned this pull request Oct 8, 2020

cmake: add 3.16.9, 3.17.5, 3.18.3 + update OpenSSL + exclude unsupported compilers #2941

Merged

4 tasks

madebr mentioned this pull request Oct 17, 2020

[check] CMake, as a tool-only package, doesn't need compiler or build_type settings #3153

Closed

SSE4 mentioned this pull request Nov 11, 2020

Removing some unecessary settings #3486

Closed

4 tasks


		del self.info.settings.compiler
		del self.info.settings.build_type

cmake: add compiler + remove compiler.version to reduce number of builds #1783

cmake: add compiler + remove compiler.version to reduce number of builds #1783

Conversation

madebr commented May 29, 2020 • edited Loading

conan-center-bot commented May 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conan-center-bot commented May 30, 2020

robert-shade May 30, 2020 • edited Loading

Choose a reason for hiding this comment

madebr May 30, 2020 • edited Loading

Choose a reason for hiding this comment

conan-center-bot commented May 30, 2020

conan-center-bot commented May 31, 2020

rweickelt commented May 31, 2020 • edited Loading

conan-center-bot commented May 31, 2020

SSE4 commented Jun 1, 2020

madebr commented Jun 2, 2020 • edited Loading

robert-shade commented Jun 2, 2020

robert-shade commented Jun 2, 2020 • edited Loading

rweickelt commented Jun 2, 2020 • edited Loading

madebr commented Jun 2, 2020

rweickelt commented Jun 2, 2020

jgsogo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conan-center-bot commented Jun 16, 2020

madebr commented Jun 16, 2020

conan-center-bot commented Jun 16, 2020

conan-center-bot commented Jun 17, 2020

Croydon commented Jun 21, 2020

madebr commented Jun 21, 2020 • edited Loading

jgsogo commented Jun 22, 2020

Croydon commented Jun 23, 2020

jgsogo left a comment

Choose a reason for hiding this comment

madebr commented May 29, 2020 •

edited

Loading

robert-shade May 30, 2020 •

edited

Loading

madebr May 30, 2020 •

edited

Loading

rweickelt commented May 31, 2020 •

edited

Loading

madebr commented Jun 2, 2020 •

edited

Loading

robert-shade commented Jun 2, 2020 •

edited

Loading

rweickelt commented Jun 2, 2020 •

edited

Loading

madebr commented Jun 21, 2020 •

edited

Loading