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

Fix Clang detection #4897

Closed
wants to merge 1 commit into from
Closed

Conversation

aykevl
Copy link

@aykevl aykevl commented Apr 15, 2024

The previous way that Clang was detected was not working: first the string is cast to lowercase, and then there's a check for "LLVM" (uppercase) which of course will never work. This change removes that useless check and replaces it with a check for the filename (like is already the case for GCC) so that Clang is detected in at least some cases.

The reason this is necessary is because I'm trying to add a WebAssembly target which uses Clang, and which uses wasm-ld for linking. Sadly, PlatformIO adds -Wl,--start-group and -Wl,--end-group which aren't supported by wasm-ld. But they shouldn't have been added anyway: they were only added because the compiler was (incorrectly) detected as GCC.

Sidenote: the reason Clang is detected as GCC is because $CC -v (which expands to clang -v) outputs the following on my system:

clang version 17.0.6 (Fedora 17.0.6-2.fc39)
Target: aarch64-redhat-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Found candidate GCC installation: /usr/bin/../lib/gcc/aarch64-redhat-linux/13
Selected GCC installation: /usr/bin/../lib/gcc/aarch64-redhat-linux/13
Candidate multilib: .;@m64
Selected multilib: .;@m64

This contains the string 'GCC' even though it is Clang. Perhaps the correct command would be $CC --version? It outputs the following instead:

clang version 17.0.6 (Fedora 17.0.6-2.fc39)
Target: aarch64-redhat-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

There is no 'gcc' string here!
The GCC version of this (gcc --version) also outputs something more reasonable:

gcc (GCC) 13.2.1 20240316 (Red Hat 13.2.1-7)
Copyright (C) 2023 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

The previous way that Clang was detected was not working: first the
string is cast to lowercase, and then there's a check for "LLVM"
(uppercase) which of course will never work. This change removes that
useless check and replaces it with a check for the filename (like is
already the case for GCC) so that Clang is detected in at least some
cases.

The reason this is necessary is because I'm trying to add a WebAssembly
target which uses Clang, and which uses wasm-ld for linking. Sadly,
PlatformIO adds `-Wl,--start-group` and `-Wl,--end-group` which aren't
supported by wasm-ld. But they shouldn't have been added anyway: they
were only added because the compiler was (incorrectly) detected as GCC.

Sidenote: the reason Clang is detected as GCC is because `$CC -v` (which
expands to `clang -v`) outputs the following on my system:

    clang version 17.0.6 (Fedora 17.0.6-2.fc39)
    Target: aarch64-redhat-linux-gnu
    Thread model: posix
    InstalledDir: /usr/bin
    Found candidate GCC installation: /usr/bin/../lib/gcc/aarch64-redhat-linux/13
    Selected GCC installation: /usr/bin/../lib/gcc/aarch64-redhat-linux/13
    Candidate multilib: .;@m64
    Selected multilib: .;@m64

This contains the string 'GCC' even though it is Clang. Perhaps the
correct command would be `$CC --version`? It outputs the following
instead:

    clang version 17.0.6 (Fedora 17.0.6-2.fc39)
    Target: aarch64-redhat-linux-gnu
    Thread model: posix
    InstalledDir: /usr/bin

There is no 'gcc' string here!
The GCC version of this (`gcc --version) also outputs something more
reasonable:

    gcc (GCC) 13.2.1 20240316 (Red Hat 13.2.1-7)
    Copyright (C) 2023 Free Software Foundation, Inc.
    This is free software; see the source for copying conditions.  There is NO
    warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
@ivankravets
Copy link
Member

Thanks for the PR! Please re-test with pio upgrade --dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants