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.

Sign up for GitHub

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

Bugfix libcxx detection when using CC/CXX #15418

Merged
merged 10 commits into from
Jan 10, 2024
Merged

Bugfix libcxx detection when using CC/CXX #15418

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8f799aa
Bugfix libxx detection when using CC/CXX
AbrilRBS Jan 9, 2024
87e82ad
Fix tests
AbrilRBS Jan 9, 2024
0fa9d80
Fix tests
AbrilRBS Jan 9, 2024
e7c90fe
Fix tests
AbrilRBS Jan 9, 2024
e6a2ea5
Fix tests
AbrilRBS Jan 9, 2024
8344c27
Make compiler_exe in detect_libcxx optional
AbrilRBS Jan 9, 2024
2af1bc9
Create new version not to break existing users
AbrilRBS Jan 10, 2024
7f4eca4
Prettier error when jinja2 rendering fails
AbrilRBS Jan 10, 2024
cd18189
Update conan/internal/api/detect_api.py
AbrilRBS Jan 10, 2024
87e7892
Add exception msg and profile path to output
AbrilRBS Jan 10, 2024
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
54 changes: 27 additions & 27 deletions conan/internal/api/detect_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def _get_e2k_architecture():
}.get(platform.processor())


def detect_libcxx(compiler, version):
def detect_libcxx(compiler, version, compiler_exe):
AbrilRBS marked this conversation as resolved.
Show resolved Hide resolved
assert isinstance(version, Version)

def _detect_gcc_libcxx(version_, executable):
AbrilRBS marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -166,7 +166,7 @@ def _detect_gcc_libcxx(version_, executable):
if compiler == "apple-clang":
return "libc++"
elif compiler == "gcc":
libcxx = _detect_gcc_libcxx(version, "g++")
libcxx = _detect_gcc_libcxx(version, compiler_exe or "g++")
return libcxx
elif compiler == "cc":
if platform.system() == "SunOS":
Expand All @@ -179,7 +179,7 @@ def _detect_gcc_libcxx(version_, executable):
elif platform.system() == "Windows":
return # by default windows will assume LLVM/Clang with VS backend
else: # Linux
libcxx = _detect_gcc_libcxx(version, "clang++")
libcxx = _detect_gcc_libcxx(version, compiler_exe or "clang++")
return libcxx
elif compiler == "sun-cc":
return "libCstd"
Expand Down Expand Up @@ -267,7 +267,7 @@ def detect_compiler():
if platform.system() == "Darwin" and gcc is None:
output.error("%s detected as a frontend using apple-clang. "
"Compiler not supported" % command)
return gcc, gcc_version
return gcc, gcc_version, command
if platform.system() == "SunOS" and command.lower() == "cc":
return _sun_cc_compiler(command)
if (platform.system() == "Windows" and command.rstrip('"').endswith(("cl", "cl.exe"))
Expand All @@ -276,28 +276,28 @@ def detect_compiler():

# I am not able to find its version
output.error("Not able to automatically detect '%s' version" % command)
return None, None
return None, None, None

if platform.system() == "Windows":
version = _detect_vs_ide_version()
version = {"17": "193", "16": "192", "15": "191"}.get(str(version)) # Map to compiler
if version:
return 'msvc', Version(version)
return 'msvc', Version(version), None

if platform.system() == "SunOS":
sun_cc, sun_cc_version = _sun_cc_compiler()
sun_cc, sun_cc_version, compiler_exe = _sun_cc_compiler()
if sun_cc:
return sun_cc, sun_cc_version
return sun_cc, sun_cc_version, compiler_exe

if platform.system() in ["Darwin", "FreeBSD"]:
clang, clang_version = _clang_compiler() # prioritize clang
clang, clang_version, compiler_exe = _clang_compiler() # prioritize clang
if clang:
return clang, clang_version
return clang, clang_version, compiler_exe
return
else:
gcc, gcc_version = _gcc_compiler()
gcc, gcc_version, compiler_exe = _gcc_compiler()
if gcc:
return gcc, gcc_version
return gcc, gcc_version, compiler_exe
return _clang_compiler()


Expand Down Expand Up @@ -326,21 +326,21 @@ def _gcc_compiler(compiler_exe="gcc"):
_, out = detect_runner("%s --version" % compiler_exe)
out = out.lower()
if "clang" in out:
return None, None
AbrilRBS marked this conversation as resolved.
Show resolved Hide resolved
return None, None, None

ret, out = detect_runner('%s -dumpversion' % compiler_exe)
if ret != 0:
return None, None
return None, None, None
compiler = "gcc"
installed_version = re.search(r"([0-9]+(\.[0-9])?)", out).group()
# Since GCC 7.1, -dumpversion return the major version number
# only ("7"). We must use -dumpfullversion to get the full version
# number ("7.1.1").
if installed_version:
ConanOutput(scope="detect_api").info("Found %s %s" % (compiler, installed_version))
return compiler, Version(installed_version)
return compiler, Version(installed_version), compiler_exe
except (Exception,): # to disable broad-except
return None, None
return None, None, None


def _sun_cc_compiler(compiler_exe="cc"):
Expand All @@ -354,28 +354,28 @@ def _sun_cc_compiler(compiler_exe="cc"):
installed_version = re.search(r"([0-9]+\.[0-9]+)", out).group()
if installed_version:
ConanOutput(scope="detect_api").info("Found %s %s" % (compiler, installed_version))
return compiler, Version(installed_version)
return compiler, Version(installed_version), compiler_exe
except (Exception,): # to disable broad-except
return None, None
return None, None, None


def _clang_compiler(compiler_exe="clang"):
try:
ret, out = detect_runner('%s --version' % compiler_exe)
if ret != 0:
return None, None
return None, None, None
if "Apple" in out:
compiler = "apple-clang"
elif "clang version" in out:
compiler = "clang"
else:
return None, None
return None, None, None
installed_version = re.search(r"([0-9]+\.[0-9])", out).group()
if installed_version:
ConanOutput(scope="detect_api").info("Found %s %s" % (compiler, installed_version))
return compiler, Version(installed_version)
return compiler, Version(installed_version), compiler_exe
except (Exception,): # to disable broad-except
return None, None
return None, None, None


def _msvc_cl_compiler(compiler_exe="cl"):
Expand All @@ -386,20 +386,20 @@ def _msvc_cl_compiler(compiler_exe="cl"):
compiler_exe = compiler_exe.strip('"')
ret, out = detect_runner(f'"{compiler_exe}" /?')
if ret != 0:
return None, None
return None, None, None
first_line = out.splitlines()[0]
if "Microsoft" not in first_line:
return None, None
return None, None, None
compiler = "msvc"
version_regex = re.search(r"(?P<major>[0-9]+)\.(?P<minor>[0-9]+)\.([0-9]+)\.?([0-9]+)?",
first_line)
if not version_regex:
return None, None
return None, None, None
# 19.36.32535 -> 193
version = f"{version_regex.group('major')}{version_regex.group('minor')[0]}"
return compiler, Version(version)
return compiler, Version(version), compiler_exe
except (Exception,): # to disable broad-except
return None, None
return None, None, None


def default_compiler_version(compiler, version):
Expand Down
4 changes: 2 additions & 2 deletions conans/client/conf/detect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def detect_defaults_settings():
arch = detect_arch()
if arch:
result.append(("arch", arch))
compiler, version = detect_compiler()
compiler, version, compiler_exe = detect_compiler()
Copy link
Member

Choose a reason for hiding this comment

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

@czoido note this is a breaking change for those that started to use the detect_api feature, but we thought that being it declared as unstable and recently released, we are on time to change it.

The other alteranative was to opt-in via detect_compiler(return_compiler_exe=True) or the like

Copy link
Contributor

@czoido czoido Jan 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I also think that the chances to break are not so high, but my concern is that if anyone is using this, the error is going to be difficult to track, because it will look like this:

ERROR: Traceback (most recent call last):
File "/Users/test/conan/conan/cli/cli.py", line 270, in main
    cli.run(args)
  File "/Users/test/conan/conan/cli/cli.py", line 180, in run
    command.run(self._conan_api, args[0][1:])
  File "/Users/test/conan/conan/cli/command.py", line 126, in run
    info = self._method(conan_api, parser, *args)
  File "/Users/test/conan/conan/cli/commands/install.py", line 55, in install
    profile_host, profile_build = conan_api.profiles.get_profiles_from_args(args)
  File "/Users/test/conan/conan/api/subapi/profiles.py", line 66, in get_profiles_from_args
    profile_host = self._get_profile(host_profiles, args.settings_host, args.options_host, args.conf_host,
  File "/Users/test/conan/conan/api/subapi/profiles.py", line 88, in _get_profile
    profile = loader.from_cli_args(profiles, settings, options, conf, cwd)
  File "/Users/test/conan/conans/client/profile_loader.py", line 95, in from_cli_args
    tmp = self.load_profile(p, cwd)
  File "/Users/test/conan/conans/client/profile_loader.py", line 105, in load_profile
    profile = self._load_profile(profile_name, cwd)
  File "/Users/test/conan/conans/client/profile_loader.py", line 130, in _load_profile
    text = rtemplate.render(context)
  File "/Users/test/conan-virtual-env/lib/python3.9/site-packages/jinja2/environment.py", line 1301, in render
    self.environment.handle_exception()
  File "/Users/test/conan-virtual-env/lib/python3.9/site-packages/jinja2/environment.py", line 936, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File "<template>", line 2, in top-level template code
ValueError: too many values to unpack (expected 2)

ERROR: too many values to unpack (expected 2)

If we don't have a way to raise an error that guides users on how to fix the problem, I personally would prefer a non-breaking change, like adding a new function and deprecate the current one adding a deprecation warning.

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 I agree, we can do it non-breaking and deprecating old behavior.

if not compiler:
result.append(("build_type", "Release"))
ConanOutput().warning("No compiler was detected (one may not be needed)")
Expand All @@ -28,7 +28,7 @@ def detect_defaults_settings():
result.append(("compiler.runtime", runtime))
if runtime_version:
result.append(("compiler.runtime_version", runtime_version))
libcxx = detect_libcxx(compiler, version)
libcxx = detect_libcxx(compiler, version, compiler_exe)
if libcxx:
result.append(("compiler.libcxx", libcxx))
cppstd = detect_cppstd(compiler, version)
Expand Down
3 changes: 2 additions & 1 deletion conans/test/functional/test_profile_detect_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ def test_profile_detect_compiler(self):

client = TestClient()
tpl1 = textwrap.dedent("""
{% set compiler, version = detect_api.detect_compiler() %}
{% set compiler, version, compiler_exe = detect_api.detect_compiler() %}
{% set runtime, _ = detect_api.default_msvc_runtime(compiler) %}
[settings]
compiler={{compiler}}
compiler.version={{detect_api.default_compiler_version(compiler, version)}}
compiler.runtime={{runtime}}
compiler.libcxx={{detect_api.detect_libcxx(compiler, version, compiler_exe)}}
compiler.cppstd={{detect_api.default_cppstd(compiler, version)}}

[conf]
Expand Down