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

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

Merged
merged 4 commits into from Dec 20, 2019

Conversation

jgsogo
Copy link
Member

@jgsogo jgsogo commented Dec 13, 2019

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
Copy link
Member Author

@jgsogo jgsogo commented Dec 13, 2019

@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 Dec 13, 2019
@jgsogo jgsogo requested a review from lasote Dec 13, 2019
@jgsogo jgsogo added this to the 1.22 milestone Dec 16, 2019
Copy link
Member

@memsharded memsharded left a comment

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
Copy link
Contributor

@lasote lasote commented Dec 20, 2019

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
2 checks passed
@jgsogo jgsogo deleted the fix/multi_generators branch Dec 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants