2231 conan compiler detection #2487

pvicente · 2018-02-16T19:52:30Z

Refer to the issue that supports this Pull Request.
I've read the Contributing guide.
I've followed the PEP8 style guides for Python code.

Previously failing compiler detection in mac os x when gcc was forced:

09:28 $ CC=gcc CXX=g++ conan install boost/1.66.0@conan/stable
Auto detecting your dev setup to initialize the default profile (/Users/pvicente/.conan/profiles/default)
CC and CXX: gcc, g++
Found gcc 4.2
Default settings
	os=Macos
	os_build=Macos
	arch=x86_64
	arch_build=x86_64
	compiler=gcc
	compiler.version=4.2
	compiler.libcxx=libstdc++
	build_type=Release
*** You can change them in /Users/pvicente/.conan/profiles/default ***
*** Or override with -s compiler='other' -s ...s***


ERROR: Invalid setting '4.2' is not a valid 'settings.compiler.version' value.
Possible values are ['4.1', '4.4', '4.5', '4.6', '4.7', '4.8', '4.9', '5', '5.1', '5.2', '5.3', '5.4', '5.5', '6', '6.1', '6.2', '6.3', '6.4', '7', '7.1', '7.2', '7.3']
Read "http://docs.conan.io/en/latest/faq/troubleshooting.html#error-invalid-setting"

Now:

09:26 $ CC=gcc CXX=g++ conan install boost/1.66.0@conan/stable
Auto detecting your dev setup to initialize the default profile (/Users/pvicente/.conan/profiles/default)
CC and CXX: gcc, g++
Found apple-clang 9.0
Default settings
	os=Macos
	os_build=Macos
	arch=x86_64
	arch_build=x86_64
	compiler=apple-clang
	compiler.version=9.0
	compiler.libcxx=libc++
	build_type=Release
*** You can change them in /Users/pvicente/.conan/profiles/default ***
*** Or override with -s compiler='other' -s ...s***
...

lasote

Good job. Take a look to my comment.

lasote · 2018-02-17T17:39:30Z

conans/client/conf/detect.py

@@ -146,9 +159,9 @@ def _get_default_compiler(output):
    if cc or cxx:  # Env defined, use them
        output.info("CC and CXX: %s, %s " % (cc or "None", cxx or "None"))
        command = cc or cxx
-        if "gcc" in command:
+        if _is_real_compiler("gcc", command):


I think it is clearer to get here the output of the "command --help", then "if "gcc" in output:" and "if "clang" in output". The function is real compiler feels a little confusing. Maybe also check the output in lowercase in case GCC or Clang in output.

Sorry for my comments formatting. I'm with the mobile.

Yes, agree, I'd think it would be more understandable. I think that @lasote meant command --version, not --help.

@lasote @memsharded do you mean something like this?

if "gcc" in command: _, out = _execute("%s --version" % command) if "gcc" in out.lower(): return _gcc_compiler(output, command) if "clang" in command.lower(): _, out = _execute("%s --version" % command.lower()) if "clang" in out.lower(): return _clang_compiler(output, command)

or:

if "gcc" in command or "clang" in command.lower(): _, out = _execute("%s --version" % command) out = out.lower() if "gcc" in out: return _gcc_compiler(output, command) if "clang" in out: return _clang_compiler(output, command)

I would go for 2nd option only one call to _execute

I've fixed it with 2nd option

memsharded

As a reference: https://stackoverflow.com/questions/19535422/os-x-10-9-gcc-links-to-clang

memsharded · 2018-02-18T22:40:33Z

conans/client/conf/detect.py

@@ -146,9 +159,9 @@ def _get_default_compiler(output):
    if cc or cxx:  # Env defined, use them
        output.info("CC and CXX: %s, %s " % (cc or "None", cxx or "None"))
        command = cc or cxx
-        if "gcc" in command:
+        if _is_real_compiler("gcc", command):


Yes, agree, I'd think it would be more understandable. I think that @lasote meant command --version, not --help.

memsharded · 2018-02-18T22:42:24Z

conans/test/util/detect_test.py

+    def detect_default_in_mac_os_using_gcc_as_default_test(self):
+        # See: https://github.com/conan-io/conan/issues/2231
+        output = TestBufferConanOutput()
+        with tools.environment_append({"CC": "gcc"}):


~~This test is nice, but is not testing the reported issue. Which is gcc being actually a symlink to clang. It might be difficult to fully automate, so I am fine if tested manually.~~

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?

@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

17:06 $ gcc --version Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1 Apple LLVM version 9.0.0 (clang-900.0.38) Target: x86_64-apple-darwin17.4.0 Thread model: posix InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

I've updated it to make more evident that it should detect apple-clang.

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.

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

@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 gcc as apple-clang, but if you don't want to rely on this assumption maybe it's better to remove this end2end test.

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.

Ok, I got it, thanks for the explanation. I wasn't getting it, sorry guys!!!

SSE4 · 2018-02-19T15:20:45Z

may be it's better and more reliable to avoid parsing --version output at all, and use instead macro definitions reported by compiler as shown at #1127?

lasote · 2018-02-20T08:58:57Z

conans/client/conf/detect.py

@@ -49,7 +49,7 @@ def _clang_compiler(output, compiler_exe="clang"):
            compiler = "apple-clang"
        elif "clang version" in out:
            compiler = "clang"
-        installed_version = re.search("([0-9]\.[0-9])", out).group()
+        installed_version = re.search("version ([0-9]\.[0-9])", out).group(1)


This change in the regular expression worries me a little. If some specific clang is not printing the "version" word we could be breaking. Why did you need it? If there is a good reason I'm ok with it, if don't, better keep it like it was.

If it is only for apple-clang I would prefer to limit it to the Apple if to mitigate possible issues

But apple-clang was already working, when the compiler is "clang --version". So parsing didn't seem an issue

That's a good question, but looking at the output it could be:

Apple LLVM version 9.0.0 (clang-900.0.39.2) Target: x86_64-apple-darwin17.4.0

Please @pvicente confirm then it is failing and why didn't fail so far.

The problem is when using fake gcc in mac os x.

This is the output for gcc:

10:31 $ gcc --version Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1 Apple LLVM version 9.0.0 (clang-900.0.38) Target: x86_64-apple-darwin17.4.0 Thread model: posix InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

The previous regular expression catches 4.2 as compiler version, see regex problem

When I added version in regex I checked out that clang in linux, clang in apple are returning version in the output.

MacOSX:

23:21 $ clang --version Apple LLVM version 9.0.0 (clang-900.0.38) Target: x86_64-apple-darwin17.4.0 Thread model: posix InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

Linux:

conan@8bb2abebb6c8:/Users/pvicente/git/conan/conan$ clang --version clang version 5.0.0-3 (tags/RELEASE_500/final) Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin

Hope it makes sense to you.

lasote · 2018-02-20T09:03:19Z

@SSE4 could be more accurate, but I think we are good by now with the --version until we detect that it is not enough. Anyway thanks for your suggestion!

pvicente · 2018-02-20T13:39:30Z

@lasote @memsharded fixed test case checking output

lasote

Take a look to: #2231
I think in case of see gcc in CC in osx, if the returned trace is about clang, we should discard this detection (warn something maybe) and try with the regular one (not looking defaults CC).
You could think that the result is the same, but I think it is cleaner and safer.

lasote · 2018-02-21T08:49:07Z

conans/test/util/detect_test.py

+
+    def detect_default_in_mac_os_using_gcc_as_default_test(self):
+        """
+        In some Mac OS X gcc is in reality clang.


We investigate it and we changed our mind, the clang "gcc" is not a pure gcc. Read the last comment of @memsharded here: #2231

@lasote @memsharded I've updated the code to skip gcc detection this case arises as commented in #2231

pvicente · 2018-02-21T16:28:27Z

@lasote @memsharded it's ready for review with suggested changes #2231

But I think the logic should probably be: Skip the gcc (as clang frontend) in OSX in the default conan detection of compiler. So in OSX, conan will be able to detect normal "apple-clang" clang compiler, and maybe "gcc", but as complete gcc, not that clang frontend.

lasote

Just a little simplification in the test and the rest I think it is ok.

lasote · 2018-02-21T16:30:56Z

conans/test/util/detect_test.py

+        if platform.system() != "Darwin":
+            return
+
+        def _execute(command):


You can simplify all this with output_str = subprocess.check_output(command) and try, except in case of subprocess failure.

Yes, totally right!!! fixing it!!!

lasote · 2018-02-21T17:21:46Z

conans/test/util/detect_test.py

-        if proc_return != 0 or "clang" not in output:
-            # Not test scenario (gcc should display clang in output
+        if "clang" not in output:
+            # Not test scenario gcc should display clang in output
            return


Mhh, may we raise here to realize that maybe the default OSX policy for "gcc" has changed?

I know that it is not the scope of the test, but useful to know if something changes

Yes It's a good idea, @memsharded WDYT?

like: raise Exception("Apple gcc doesn't point to clang with gcc frontend anymore! please check")

memsharded · 2018-02-21T23:26:54Z

conans/client/conf/detect.py

+            if "clang" not in out:
+                return _gcc_compiler(output, command)
+            output.warn("%s detected as a frontend using apple-clang, skipping it to use native apple-clang" % command)
+            command = "clang"  # Force use of clang when gcc is detected as a fronted of apple-clang


I am not sure we want to fallback to clang. Because if the env-var CC and CXX are set to gcc, then, typically CMake will use them when running, and the user might not notice, because it detected "clang" when CC=gcc. What do you think @lasote?

I'm ok with this. If the user is messing with CC/CXX is his problem. And the default profile is just a default.

Ok, makes sense

memsharded · 2018-02-21T23:32:14Z

conans/client/conf/detect.py

+            _, out = _execute("%s --version" % command)
+            out = out.lower()
+            if "clang" not in out:
+                return _gcc_compiler(output, command)


The overall logic of this code seems a bit convolute IMHO, with 2 calls to _gcc_compiler() plus one manual _execute("gcc.... Maybe it would make more sense to put this logic inside _gcc_compiler(), so the compiler is called exactly once, and the rest is processing the output?

@memsharded @lasote in that case what about the fallback to clang when gcc is a frontend of apple-clang.

Shall we call _clang_compiler(output, "clang") inside _gcc_compiler(output, command) as the logic will be inside _gcc_compiler?

I've seen that we are using _gcc_compiler also in the case that we are not forcing the usage of gcc with CC variable so I'm not sure about putting the logic inside _gcc_compiler

…ned. Checked if compiler name is matches with compiler exe output

… Mac OS X

…e _gcc_compiler method

memsharded · 2018-02-26T15:28:34Z

conans/client/conf/detect.py

+                    "%s detected as a frontend using apple-clang, skipping it to use native apple-clang" % command
+                )
+                # Fallback to use clang instead
+                command = "clang"


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.

@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)

memsharded 5 days ago Owner
I am not sure we want to fallback to clang. Because if the env-var CC and CXX are set to gcc, >then, typically CMake will use them when running, and the user might not notice, because it >detected "clang" when CC=gcc. What do you think @lasote?

@lasote
lasote 4 days ago Owner
I'm ok with this. If the user is messing with CC/CXX is his problem. And the default profile is >just a default.

@memsharded
memsharded 4 days ago Owner
Ok, makes sense

@lasote Anything that I'm missing here?

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.

@lasote @memsharded Ok now conan should display:

16:13 $ CC=gcc conan install libiconv/1.15@bincrafters/stable Auto detecting your dev setup to initialize the default profile (/Users/pvicente/.conan/profiles/default) CC and CXX: gcc, None ERROR: gcc detected as a frontend using apple-clang. Compiler not supported ERROR: Unable to find a working compiler Default settings os=Macos os_build=Macos arch=x86_64 arch_build=x86_64 build_type=Release *** You can change them in /Users/pvicente/.conan/profiles/default *** *** Or override with -s compiler='other' -s ...s*** libiconv/1.15@bincrafters/stable: Not found in local cache, looking in remotes... libiconv/1.15@bincrafters/stable: Trying with 'conan-center'... libiconv/1.15@bincrafters/stable: Trying with 'conan-transit'... ....

…is not supported

* Fixing default compiler detection when environment variables are defined. Checked if compiler name is matches with compiler exe output * Added end2end tests for compiler detection * Addressed comment from PR, removing _is_real_compiler method * Updated test for mac os x reported bug * Checking output of "gcc --version" and execute in right test scenario. * Updated code to skip gcc when it's using apple-clang as a frontend in Mac OS X * Updated test to run only in Mac OS X * Simplified test using subprocess.check_output * Raising exception if gcc doesn't point to clang in Mac OS X * Fixed problem python 2/3 * As sugested in PR putting logic to detect GCC as clang frontend inside _gcc_compiler method * Updated to simplify logic * Moving check inside try/except * As commented in PR now compiler GCC if it's a fronted of apple-clang is not supported

pvicente assigned memsharded Feb 16, 2018

ghost assigned pvicente Feb 16, 2018

ghost added the stage: review label Feb 16, 2018

pvicente assigned lasote and unassigned lasote Feb 17, 2018

pvicente requested a review from lasote February 17, 2018 09:35

lasote suggested changes Feb 17, 2018

View reviewed changes

memsharded reviewed Feb 18, 2018

View reviewed changes

lasote reviewed Feb 20, 2018

View reviewed changes

memsharded added this to the 1.1 milestone Feb 20, 2018

memsharded mentioned this pull request Feb 20, 2018

Conan on Mac can't recognize compiler correctly #2231

Closed

pvicente force-pushed the 2231_conan_compiler_detection branch from f5fd32a to 22b941a Compare February 20, 2018 13:37

lasote suggested changes Feb 21, 2018

View reviewed changes

pvicente force-pushed the 2231_conan_compiler_detection branch from 16b1e85 to a8c1e6d Compare February 21, 2018 13:59

lasote approved these changes Feb 21, 2018

View reviewed changes

lasote reviewed Feb 21, 2018

View reviewed changes

memsharded reviewed Feb 21, 2018

View reviewed changes

pvicente added 9 commits February 24, 2018 13:58

Fixing default compiler detection when environment variables are defi…

16b0d02

…ned. Checked if compiler name is matches with compiler exe output

Added end2end tests for compiler detection

a01a613

Addressed comment from PR, removing _is_real_compiler method

c13d249

Updated test for mac os x reported bug

3db2bf9

Checking output of "gcc --version" and execute in right test scenario.

ed9fffd

Updated code to skip gcc when it's using apple-clang as a frontend in…

39b4845

… Mac OS X

Updated test to run only in Mac OS X

9704aef

Simplified test using subprocess.check_output

93264c3

Raising exception if gcc doesn't point to clang in Mac OS X

3c90274

pvicente added 3 commits February 24, 2018 13:58

Fixed problem python 2/3

97ecdee

As sugested in PR putting logic to detect GCC as clang frontend insid…

85a181f

…e _gcc_compiler method

Updated to simplify logic

5b7b94d

pvicente force-pushed the 2231_conan_compiler_detection branch from 0a467d1 to 5b7b94d Compare February 24, 2018 14:11

Moving check inside try/except

8c8ce2c

memsharded reviewed Feb 26, 2018

View reviewed changes

As commented in PR now compiler GCC if it's a fronted of apple-clang …

40cb556

…is not supported

memsharded merged commit 7c15a59 into conan-io:develop Feb 26, 2018

ghost removed the stage: review label Feb 26, 2018

pvicente deleted the 2231_conan_compiler_detection branch February 26, 2018 17:08

2231 conan compiler detection #2487

2231 conan compiler detection #2487

Conversation

pvicente commented Feb 16, 2018 • edited Loading

lasote left a comment

Choose a reason for hiding this comment

lasote Feb 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvicente Feb 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

memsharded left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

memsharded Feb 18, 2018 • edited Loading

Choose a reason for hiding this comment

pvicente Feb 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SSE4 commented Feb 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvicente Feb 20, 2018 • edited Loading

Choose a reason for hiding this comment

lasote commented Feb 20, 2018

pvicente commented Feb 20, 2018

lasote left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvicente commented Feb 21, 2018

lasote left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lasote Feb 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvicente Feb 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvicente Feb 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvicente Feb 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvicente commented Feb 16, 2018 •

edited

Loading

lasote Feb 17, 2018 •

edited

Loading

pvicente Feb 19, 2018 •

edited

Loading

memsharded Feb 18, 2018 •

edited

Loading

pvicente Feb 19, 2018 •

edited

Loading

pvicente Feb 20, 2018 •

edited

Loading

lasote Feb 21, 2018 •

edited

Loading

pvicente Feb 21, 2018 •

edited

Loading

pvicente Feb 22, 2018 •

edited

Loading

pvicente Feb 26, 2018 •

edited

Loading