-
Notifications
You must be signed in to change notification settings - Fork 1k
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
2231 conan compiler detection #2487
Changes from 13 commits
16b0d02
a01a613
c13d249
3db2bf9
ed9fffd
39b4845
9704aef
93264c3
3c90274
97ecdee
85a181f
5b7b94d
8c8ce2c
40cb556
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,52 @@ | ||
import platform | ||
import subprocess | ||
import unittest | ||
from conans.test.utils.tools import TestBufferConanOutput | ||
|
||
from conans import tools | ||
from conans.client.conf.detect import detect_defaults_settings | ||
import platform | ||
from conans.test.utils.tools import TestBufferConanOutput | ||
|
||
|
||
class DetectTest(unittest.TestCase): | ||
|
||
def detect_test(self): | ||
def detect_default_compilers_test(self): | ||
platform_default_compilers = { | ||
"Linux": "gcc", | ||
"Darwin": "apple-clang", | ||
"Windows": "Visual Studio" | ||
} | ||
output = TestBufferConanOutput() | ||
result = detect_defaults_settings(output) | ||
if platform.system() == "Linux": | ||
for (name, value) in result: | ||
if name == "compiler": | ||
self.assertEquals(value, "gcc") | ||
# result is a list of tuples (name, value) so converting it to dict | ||
result = dict(result) | ||
platform_compiler = platform_default_compilers.get(platform.system(), None) | ||
if platform_compiler is not None: | ||
self.assertEquals(result.get("compiler", None), platform_compiler) | ||
|
||
def detect_default_in_mac_os_using_gcc_as_default_test(self): | ||
""" | ||
Test if gcc in Mac OS X is using apple-clang as frontend | ||
""" | ||
# See: https://github.com/conan-io/conan/issues/2231 | ||
if platform.system() != "Darwin": | ||
return | ||
|
||
try: | ||
output = subprocess.check_output(["gcc", "--version"], stderr=subprocess.STDOUT) | ||
except subprocess.CalledProcessError: | ||
# gcc is not installed or there is any error (no test scenario) | ||
return | ||
|
||
if b"clang" not in output: | ||
# Not test scenario gcc should display clang in output | ||
# see: https://stackoverflow.com/questions/19535422/os-x-10-9-gcc-links-to-clang | ||
raise Exception("Apple gcc doesn't point to clang with gcc frontend anymore! please check") | ||
|
||
output = TestBufferConanOutput() | ||
with tools.environment_append({"CC": "gcc"}): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Actually it might be testing it, if gcc is not installed in the testing environment as real gcc but as the default clang compiler. But it should check "clang" in the test, shouldn't it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @memsharded Indeed, this test is only executed in MacOS x (Darwin) where gcc command is clang which was the bug report for this issue. In my Mac OS X it's returning
I've updated it to make more evident that it should detect Let me know if it makes sense to you. It looks like in CI we have the same environment gcc reports it's apple-clang. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not to make it more evident. It could happen that the CI machine could have gcc really installed in it, and the test would pass without checking for clang output, so it wouldn't test at all the reported bug fix. It is necessary to explicitly check for "apple-clang" in the output There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @memsharded I don't understand what's the problem? @lasote do you know what are we talking about? This test is only executed in Mac OS X and the CI machine for Mac OS X has fake There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just check the output to confirm that the assumption is true. Because if the assumption change, it is better to know it with a fail test than being happy with a green build and a useless test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I got it, thanks for the explanation. I wasn't getting it, sorry guys!!! |
||
result = detect_defaults_settings(output) | ||
# result is a list of tuples (name, value) so converting it to dict | ||
result = dict(result) | ||
self.assertEquals(result.get("compiler", None), "apple-clang") | ||
self.assertIn("gcc detected as a frontend using apple-clang", output) | ||
self.assertIsNotNone(output.warn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think this is a weird outcome. So you defined that your CC=gcc, still conan is detecting and defaulting your profile to clang compiler? I think it is safer to just leave the compiler None for this case, so it will fail fast, instead of lots of potential mismatches between the default profile and the declared default compiler CC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@memsharded this is your comment 5 days ago (Ok, makes sense) so I've just left it to use as default clang in case the problem is detected (gcc as a fronted of apple-clang)
@lasote Anything that I'm missing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I talked with @memsharded and I changed my mind, it is better to no set a compiler and make more visible that we are not managing this situation with Conan. Not a big deal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lasote @memsharded Ok now conan should display: