Skip to content

Multi-generators cannot be used without 'build_type' setting - #6228

Merged
memsharded merged 4 commits into
conan-io:developfrom
jgsogo:fix/multi_generators
Dec 20, 2019
Merged

Multi-generators cannot be used without 'build_type' setting#6228
memsharded merged 4 commits into
conan-io:developfrom
jgsogo:fix/multi_generators

Conversation

@jgsogo

@jgsogo jgsogo commented Dec 13, 2019

Copy link
Copy Markdown
Contributor

Changelog: Fix: Multi-generators cannot be used without build_type setting. A failure is forced to cmake_find_package_multi and visual_studio_multi as it was in cmake_multi.
Docs: omit

closes #6135

This PR forces in all the multi-generators the same failure that we were triggering for the cmake_multi one.

Currently, the full error shown to the user that runs the conan install is:

[...]

conanfile.py: ERROR: Generator cmake_multi(file:None) failed
'settings.build_type' doesn't exist
'settings' possible configurations are ['arch', 'compiler', 'os']
ERROR: 'settings.build_type' doesn't exist
'settings' possible configurations are ['arch', 'compiler', 'os']

Let me know if we want to improve it to add a more meaningful message.

#tags: slow

@jgsogo

jgsogo commented Dec 13, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@lasote I feel like it makes no sense to use a multi generator in a recipe without build_type. Existing generator cmake_multi fails in this scenario, with this PR generators cmake_find_package_multi and visual_studio_multi will fail too (we may write a better error).

The advantage of the multi generators is to have the files for all the build_types in the same directory, but if the recipe doesn't have this setting, the build type information won't reach the generators and we can't know which file to create.... also, we can't just create a XXXTarget-AllBuildType.cmake file because it will contain only the information corresponding to one build type. So, IMO the best alternative is to fail with an error in the recipe.

This fixes #6135 in the sense that the error now says something about the build_type and the generator.

@jgsogo
jgsogo marked this pull request as ready for review December 13, 2019 15:23
@jgsogo
jgsogo requested a review from lasote December 13, 2019 15:23
@jgsogo jgsogo added this to the 1.22 milestone Dec 16, 2019
memsharded
memsharded approved these changes Dec 17, 2019
View reviewed changes

@memsharded memsharded left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works both for:

  • conanfile.py with build_type setting.
  • conanfile.txt (because this one doesn't remove/blank the settings, it keeps the ones defined in the profile)

If we wanted to build & test a header-only library it is necessary to define the build_type setting if you want to use this generator, but I am fine with that.

I was about to merge it, but I saw comment from @lasote here #6228 (comment), so I might be missing some previous discussion. Leaving it open and assigning to him.

@memsharded memsharded assigned lasote and unassigned memsharded Dec 17, 2019
@lasote

lasote commented Dec 20, 2019

Copy link
Copy Markdown
Contributor

I haven't commented anything about this. Go ahead.

@lasote lasote assigned memsharded and unassigned lasote Dec 20, 2019
@memsharded
memsharded merged commit 15651a6 into conan-io:develop Dec 20, 2019
@jgsogo
jgsogo deleted the fix/multi_generators branch December 20, 2019 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@memsharded memsharded memsharded approved these changes
@lasote lasote Awaiting requested review from lasote

Assignees

@memsharded memsharded

Labels

None yet

Projects

None yet

Milestone

1.22

Development

Successfully merging this pull request may close these issues.

[bug] Generator 'cmake_find_package_multi' without build_type in settings fails

3 participants

@jgsogo @lasote @memsharded