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

only warn in CMake if build_type is empty and is_multi configuration #14349

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions conan/tools/cmake/cmake.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,10 @@ def _build(self, build_type=None, target=None, cli_args=None, build_tool_args=No
bf = self._conanfile.build_folder
is_multi = is_multi_configuration(self._generator)
if build_type and not is_multi:
self._conanfile.output.error("Don't specify 'build_type' at build time for "
"single-config build systems")

self._conanfile.output.warning("Specifying 'build_type' does not have "
"any effect for single-config build systems")
bt = build_type or self._conanfile.settings.get_safe("build_type")
if not bt:
if not bt and is_multi:
raise ConanException("build_type setting should be defined.")
build_config = "--config {}".format(bt) if bt and is_multi else ""

Expand Down Expand Up @@ -180,9 +179,9 @@ def install(self, build_type=None, component=None, cli_args=None):
mkdir(self._conanfile, self._conanfile.package_folder)

bt = build_type or self._conanfile.settings.get_safe("build_type")
if not bt:
raise ConanException("build_type setting should be defined.")
is_multi = is_multi_configuration(self._generator)
if not bt and is_multi:
raise ConanException("build_type setting should be defined.")
build_config = "--config {}".format(bt) if bt and is_multi else ""

pkg_folder = '"{}"'.format(self._conanfile.package_folder.replace("\\", "/"))
Expand Down
49 changes: 49 additions & 0 deletions conans/test/integration/toolchains/cmake/test_cmake.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,52 @@ def run(self, *args, **kwargs):
assert "-something" in client.out
assert "--testverbose" in client.out
assert "-testok" in client.out

def test_build_type_single_config_warn():
Copy link
Member

Choose a reason for hiding this comment

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

These tests passes in PRs because they run only in Linux in PRs. After merge, they will run in Windows too and fail (these tests fail in Windows due to multi-config VS). It would be better to move them to "functional" folder, so they run in all platforms also in PRs.

Copy link
Member

Choose a reason for hiding this comment

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

I think the best is to do actual builds, with fully functional examples in the "functional" folder, building libraries with full test_package functionality. This will make sure things are working.

Copy link
Contributor Author

@thorsten-klein thorsten-klein Sep 18, 2023

Choose a reason for hiding this comment

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

I have created some tests for my all my use cases under functional. I always set build_type via -s build_type=Debug.

client = TestClient()

conanfile = textwrap.dedent("""
from conan import ConanFile
from conan.tools.cmake import CMake
class Pkg(ConanFile):
name = "pkg"
version = "0.1"
settings = "os", "compiler", "arch"
generators = "CMakeToolchain"
def build(self):
cmake = CMake(self)
cmake.configure()
Copy link
Member

Choose a reason for hiding this comment

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

This build, in single-config build systems is not using build_type at all. What is the purpose of doing a CMake build without build_type, which is explicitly discouraged by CMake?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How can I specify a build type then in this case a profile does not specify any build_type?
Using cmake.build(build_type="Release") does not have any effect (see test above).
Shall I use tc.cache_variables?

Copy link
Member

Choose a reason for hiding this comment

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

Why not defining the build_type in the profile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refering to settings.yml the build_type MAY be empty. So we must cover this case as well.

cmake.build(build_type="Release")
def run(self, *args, **kwargs):
self.output.info("MYRUN: {}".format(*args))
""")
client.save({"conanfile.py": conanfile})
client.run("create . ")
assert "Specifying 'build_type' does not have " + \
"any effect for single-config build systems" in client.out
assert "Created package" in client.out

def test_build_type_empty():
client = TestClient()

conanfile = textwrap.dedent("""
from conan import ConanFile
from conan.tools.cmake import CMake
class Pkg(ConanFile):
name = "pkg"
version = "0.1"
settings = "os", "compiler", "arch"
generators = "CMakeToolchain"
def build(self):
cmake = CMake(self)
cmake.configure()
cmake.build()
cmake.install()
def run(self, *args, **kwargs):
self.output.info("MYRUN: {}".format(*args))
""")
client.save({"conanfile.py": conanfile})
client.run("create . ")
assert "Created package" in client.out