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

Bugfix libcxx detection when using CC/CXX #15418

Merged
merged 10 commits into from
Jan 10, 2024

Conversation

AbrilRBS
Copy link
Member

@AbrilRBS AbrilRBS commented Jan 9, 2024

Changelog: Bugfix: Fix libcxx detection when using CC/CXX env vars.
Docs: conan-io/docs#3509

Closes: #15399

@AbrilRBS AbrilRBS added this to the 2.0.17 milestone Jan 9, 2024
@AbrilRBS AbrilRBS requested a review from memsharded January 9, 2024 09:27
@AbrilRBS AbrilRBS linked an issue Jan 9, 2024 that may be closed by this pull request
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Please also check broken tests

conan/internal/api/detect_api.py Outdated Show resolved Hide resolved
conan/internal/api/detect_api.py Show resolved Hide resolved
@AbrilRBS AbrilRBS requested a review from memsharded January 9, 2024 16:18
@@ -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

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.

@czoido czoido merged commit 56e981c into conan-io:release/2.0 Jan 10, 2024
2 checks passed
@AbrilRBS AbrilRBS deleted the rr/detect-api-compiler-exe branch January 29, 2024 17:51
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.

[bug] profile detect does not try to use the compiler provided
4 participants