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

jgsogo · 2020-10-09T07:33:21Z

Specify library name and version: cmake/all

Packages that act only as tools (do not contain libraries or they are not currently used) can remove compiler and build_type settings and generate less packages. C3i generates the profiles in order taking into account some rules: Release builds first (minimize problem with VS redistributables), older compilers first (avoid issue with docker images and glibc).

Let's double-check if the promise fulfills.

SpaceIm · 2020-10-09T09:10:03Z

Could it be worth opening an issue listing all "tools only" packages and their current settings? It's not really consistent currently.

conan-center-bot · 2020-10-09T09:39:58Z

All green in build 1 (78a0187371c828fc4d093d703b434c0e4274fe03)! 😊

cmake/3.16.2: Generated 3 packages. All logs here
cmake/3.16.3: Generated 3 packages. All logs here
cmake/3.16.4: Generated 3 packages. All logs here
cmake/3.16.5: Generated 3 packages. All logs here
cmake/3.16.6: Generated 3 packages. All logs here
cmake/3.16.7: Generated 3 packages. All logs here
cmake/3.16.8: Generated 3 packages. All logs here
cmake/3.17.0: Generated 3 packages. All logs here
cmake/3.17.1: Generated 3 packages. All logs here
cmake/3.17.2: Generated 3 packages. All logs here
cmake/3.17.3: Generated 3 packages. All logs here
cmake/3.17.4: Generated 3 packages. All logs here
cmake/3.18.0: Generated 3 packages. All logs here
cmake/3.18.1: Generated 3 packages. All logs here
cmake/3.18.2: Generated 3 packages. All logs here
cmake/3.18.4: Generated 3 packages. All logs here

jgsogo · 2020-10-09T09:51:12Z

Having a look to profiles:

Linux (package-ID: 3be0335ecbcae34c1eca15767d4672128da755cf)

[settings]
arch=x86_64
arch_build=x86_64
build_type=Release
compiler=gcc
compiler.libcxx=libstdc++
compiler.version=4.9
os=Linux
os_build=Linux

MacOS (package-ID: 7472a0907e34e17166c741496858d22be213d903)

[settings]
arch=x86_64
arch_build=x86_64
build_type=Release
compiler=apple-clang
compiler.libcxx=libc++
compiler.version=9.1
os=Macos
os_build=Macos

Windows (package-ID: ca33edce272a279b24f87dc0d4cf5bbdcffbc187)

[settings]
arch=x86_64
arch_build=x86_64
build_type=Release
compiler=Visual Studio
compiler.runtime=MT
compiler.version=14
os=Windows
os_build=Windows

These packages, when used as build-requires, should have max-compatibility. They should work on any docker image we are using in the CI.

... and it only took 2 hours to build 🙄

ping @uilianries @SSE4 @danimtb

uilianries

LGTM

Croydon · 2020-10-09T14:57:01Z

Interesting situation: Does the bot merge PRs which passed all checks but are still marked as drafts or does it check the status before?

LGTM too

jgsogo · 2020-10-09T15:02:55Z

Interesting situation: Does the bot merge PRs which passed all checks but are still marked as drafts or does it check the status before?

It only considers those not in draft mode.

madebr · 2020-10-09T16:52:19Z

This is not correct.
Compare the following logs:

Compare the calls to cmake.build().

cmake --build C:\J\w\cci_PR-3153\.conan\data\cmake\3.16.2\_\_\build\ca33edce272a279b24f87dc0d4cf5bbdcffbc187

versus

C:\J\w\cci_PR-3109@5\.conan\data\cmake\3.18.2\_\_\build\6cc50b139b9c3d27b3e9042d5f5372d327b3a9f7 --config Release -- /m:1 /verbosity:minimal

We need to keep compiler/build_type in settings, because otherwise we depend on the default of compilers/cmake script.

Imho we should tag tools-only recipes and only build Release+MT configurations for them instead of butchering the recipe.

jgsogo · 2020-10-09T17:14:49Z

Yes, we need full settings: os, arch, compiler and build_type, and then remove compiler and build_type from the package_id. Why? We want to generate fewer packages in C3i and it also demonstrates how Conan works with it. Keep all the settings is needed too so the consumer can force a build for other configurations if desired.

madebr · 2020-10-09T17:45:29Z

@jgsogo
I would combine #2813 with this.
It is sometimes useful to have both a release and debug version of a package available.

Using #2813, you can tell c3i to only build certain configurations.

conan-center-bot · 2020-10-10T13:21:30Z

All green in build 2 (e77551bac3de4edf8ffcdd12f5d491bde9032759)! 😊

cmake/3.16.2: Generated 3 packages. All logs here
cmake/3.16.3: Generated 3 packages. All logs here
cmake/3.16.4: Generated 3 packages. All logs here
cmake/3.16.5: Generated 3 packages. All logs here
cmake/3.16.6: Generated 3 packages. All logs here
cmake/3.16.7: Generated 3 packages. All logs here
cmake/3.16.8: Generated 3 packages. All logs here
cmake/3.17.0: Generated 3 packages. All logs here
cmake/3.17.1: Generated 3 packages. All logs here
cmake/3.17.2: Generated 3 packages. All logs here
cmake/3.17.3: Generated 3 packages. All logs here
cmake/3.17.4: Generated 3 packages. All logs here
cmake/3.18.0: Generated 3 packages. All logs here
cmake/3.18.1: Generated 3 packages. All logs here
cmake/3.18.2: Generated 3 packages. All logs here
cmake/3.18.4: Generated 3 packages. All logs here

jgsogo · 2020-10-13T11:13:56Z

After #2941, to consider build_type in package ID

conan-center-bot · 2020-10-13T12:21:41Z

All green in build 3 (e77551bac3de4edf8ffcdd12f5d491bde9032759)! 😊

cmake/3.16.2: Generated 3 packages. All logs here
cmake/3.16.3: Generated 3 packages. All logs here
cmake/3.16.4: Generated 3 packages. All logs here
cmake/3.16.5: Generated 3 packages. All logs here
cmake/3.16.6: Generated 3 packages. All logs here
cmake/3.16.7: Generated 3 packages. All logs here
cmake/3.16.8: Generated 3 packages. All logs here
cmake/3.16.9: Generated 3 packages. All logs here
cmake/3.17.0: Generated 3 packages. All logs here
cmake/3.17.1: Generated 3 packages. All logs here
cmake/3.17.2: Generated 3 packages. All logs here
cmake/3.17.3: Generated 3 packages. All logs here
cmake/3.17.4: Generated 3 packages. All logs here
cmake/3.17.5: Generated 3 packages. All logs here
cmake/3.18.0: Generated 3 packages. All logs here
cmake/3.18.1: Generated 3 packages. All logs here
cmake/3.18.2: Generated 3 packages. All logs here
cmake/3.18.3: Generated 3 packages. All logs here
cmake/3.18.4: Generated 3 packages. All logs here

Croydon · 2020-10-13T16:30:10Z

Just to explain my change in the other PR: I was thinking that excluding the compiler is rather uncontroversial for an installer package like CMake

As @madebr mentioned there might be cases where it would be annoying to exclude the build_type, like switching often between debug and release

When I sometimes used Clion it was often configured to compile both build_types in this case Conan would compile/overwrite the package all the time I guess

jgsogo · 2020-10-13T16:48:26Z

We agree (inside the team) that installers shouldn't include compiler or build_type in the packageID, but for sure we didn't take into account every possible scenario. I've opened this PR and it is in draft mode exactly for that purpose, to know what you think about these changes.

Removing settings from the package_id() won't affect the generation/build workflow, Conan will generate the proper binary taking into account all the configuration provided. It only affects the consuming flow of packages: information about compiler and build_type is removed, you cannot know which exact binary you are using (you can use properties to get that information).

@madebr I would like to know the scenario where you need (and cannot generate it yourself) the Debug build of CMake or any other tool. Thanks!

madebr · 2020-10-17T14:48:03Z

My resistance comes from these old cmake issues:

this comment: cmake: add compiler + remove compiler.version to reduce number of builds #1783 (comment)
and this comment: [package] cmake/3.16.3: not working for Windows #1705 (comment)

By only removing compiler and build_type in package_id, we leave the possibility of building + publishing a cmake package with debug runtime, DLL runtime and with a version that is possibly not installed.

So I propose the following:

settings = "os", "arch", "compiler", "build_type"
def configure(self):
    if self.settings.compiler == "Visual Studio" and self.setting.compiler.runtime != "MT":
        raise ConanInvalidConfiguration("CMake only supports MT to limit configurations")
    if self.settings.build_type != "Release":
        raise ConanInvalidConfiguration("CMake only supports Release build_type to limit configurations")
def package_id(self):
    del self.info.settings.compiler
    del self.info.settings.build_type

jgsogo · 2020-10-20T14:48:52Z

My resistance comes from these old cmake issues:

this comment: #1783 (comment)

and this comment: #1705 (comment)

By only removing compiler and build_type in package_id, we leave the possibility of building + publishing a cmake package with debug runtime, DLL runtime and with a version that is possibly not installed.

I understand your concerns, but this is something that won't happen in CCI. We ensure a fixed order for the profiles and we know which ones are going to be uploaded. I totally understand this is something that can accidentally happen somewhere else if users do not pay enough attention, but in any production environment this should be controlled: order of profiles and jobs that can upload packages to the remotes.

So I propose the following:

settings = "os", "arch", "compiler", "build_type"
def configure(self):
    if self.settings.compiler == "Visual Studio" and self.setting.compiler.runtime != "MT":
        raise ConanInvalidConfiguration("CMake only supports MT to limit configurations")
    if self.settings.build_type != "Release":
        raise ConanInvalidConfiguration("CMake only supports Release build_type to limit configurations")
def package_id(self):
    del self.info.settings.compiler
    del self.info.settings.build_type

I see a problem here, we can't raise ConanInvalidConfiguration exceptions if we know the recipe works and the package builds for that configuration. Some user might want to compile using those settings.

madebr · 2020-10-21T09:26:37Z

I understand your concerns, but this is something that won't happen in CCI. We ensure a fixed order for the profiles and we know which ones are going to be uploaded. I totally understand this is something that can accidentally happen somewhere else if users do not pay enough attention, but in any production environment this should be controlled: order of profiles and jobs that can upload packages to the remotes.

This mixing of c3i build policy/operational management and recipe properties is what I'm feeling uneasy about.
This will work fine most of the cases but not when doing e.g.

conan create .... -s build_type=Debug -s compiler="Visual Studio" -s compiler.runtime=MDd --build all
conan upload ...

Such an upload can happen accidentally. I'm just forecasting an issues some conan users will face.

So I propose the following:

settings = "os", "arch", "compiler", "build_type"
def configure(self):
    if self.settings.compiler == "Visual Studio" and self.setting.compiler.runtime != "MT":
        raise ConanInvalidConfiguration("CMake only supports MT to limit configurations")
    if self.settings.build_type != "Release":
        raise ConanInvalidConfiguration("CMake only supports Release build_type to limit configurations")
def package_id(self):
    del self.info.settings.compiler
    del self.info.settings.build_type

I see a problem here, we can't raise ConanInvalidConfiguration exceptions if we know the recipe works and the package builds for that configuration. Some user might want to compile using those settings.

Indeed, this is not a good approach.
It shows that there is no solution with the current set of tools.
See my (crazy) comment at #2813 (comment).

jgsogo · 2020-10-22T14:28:10Z

:D

conan-center-bot · 2020-10-22T14:28:17Z

config.yml syntax error in build 4:

No changes detected in this pull request: []|

CMake as installer needs only os and arch

78a0187

jgsogo mentioned this pull request Oct 9, 2020

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

Merged

4 tasks

danimtb previously approved these changes Oct 9, 2020

View reviewed changes

uilianries previously approved these changes Oct 9, 2020

View reviewed changes

Croydon mentioned this pull request Oct 9, 2020

[question] Consistency of settings in tools only packages #3157

Open

Croydon previously approved these changes Oct 9, 2020

View reviewed changes

use full settings, and remove from package_id

e77551b

jgsogo dismissed stale reviews from Croydon, uilianries, and danimtb via e77551b October 10, 2020 11:19

jgsogo requested review from danimtb and uilianries October 10, 2020 14:11

madebr mentioned this pull request Oct 10, 2020

[service] Ability to purposefully NOT build all versions for a recipe #2813

Closed

Merge branch 'master' into cmake/installer

82c9598

jgsogo added 2 commits October 22, 2020 16:27

Merge branch 'master' into cmake/installer

c764601

do not remove build_type

cbcd9d9

jgsogo closed this Oct 22, 2020

jgsogo deleted the cmake/installer branch October 22, 2020 14:28

conan-center-bot added the Failed label Oct 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jgsogo commented Oct 9, 2020

SpaceIm commented Oct 9, 2020

conan-center-bot commented Oct 9, 2020

jgsogo commented Oct 9, 2020

uilianries left a comment

Croydon commented Oct 9, 2020

jgsogo commented Oct 9, 2020 •

edited

Loading

madebr commented Oct 9, 2020 •

edited

Loading

jgsogo commented Oct 9, 2020

madebr commented Oct 9, 2020

conan-center-bot commented Oct 10, 2020

jgsogo commented Oct 13, 2020

conan-center-bot commented Oct 13, 2020

Croydon commented Oct 13, 2020 •

edited

Loading

jgsogo commented Oct 13, 2020

madebr commented Oct 17, 2020 •

edited

Loading

jgsogo commented Oct 20, 2020

madebr commented Oct 21, 2020

jgsogo commented Oct 22, 2020

conan-center-bot commented Oct 22, 2020

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

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

Conversation

jgsogo commented Oct 9, 2020

SpaceIm commented Oct 9, 2020

conan-center-bot commented Oct 9, 2020

jgsogo commented Oct 9, 2020

uilianries left a comment

Choose a reason for hiding this comment

Croydon commented Oct 9, 2020

jgsogo commented Oct 9, 2020 • edited Loading

madebr commented Oct 9, 2020 • edited Loading

jgsogo commented Oct 9, 2020

madebr commented Oct 9, 2020

conan-center-bot commented Oct 10, 2020

jgsogo commented Oct 13, 2020

conan-center-bot commented Oct 13, 2020

Croydon commented Oct 13, 2020 • edited Loading

jgsogo commented Oct 13, 2020

madebr commented Oct 17, 2020 • edited Loading

jgsogo commented Oct 20, 2020

madebr commented Oct 21, 2020

jgsogo commented Oct 22, 2020

conan-center-bot commented Oct 22, 2020

jgsogo commented Oct 9, 2020 •

edited

Loading

madebr commented Oct 9, 2020 •

edited

Loading

Croydon commented Oct 13, 2020 •

edited

Loading

madebr commented Oct 17, 2020 •

edited

Loading