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

Ignore vendor name in Clang version number. #12725

Closed
wants to merge 1 commit into from
Closed

Conversation

@juikim
Copy link
Contributor

@juikim juikim commented Aug 26, 2020

For example, FreeBSD prepends "FreeBSD" to version string, e.g.,

FreeBSD clang version 11.0.0 (git@github.com:llvm/llvm-project.git llvmorg-11.0.0-rc2-0-g414f32a9e86)
Target: x86_64-unknown-freebsd13.0
Thread model: posix
InstalledDir: /usr/bin

This prevented us from properly detecting AVX support, etc.

Checklist
  • documentation is added or updated
  • tests are added or updated
For example, FreeBSD prepends "FreeBSD" to version string, e.g.,

FreeBSD clang version 11.0.0 (git@github.com:llvm/llvm-project.git llvmorg-11.0.0-rc2-0-g414f32a9e86)
Target: x86_64-unknown-freebsd13.0
Thread model: posix
InstalledDir: /usr/bin

This prevented us from properly detecting AVX support, etc.

CLA: trivial
@levitte
Copy link
Member

@levitte levitte commented Aug 26, 2020

Formality: I agree this is trivial

@emaste
Copy link

@emaste emaste commented Aug 26, 2020

Note that it is not just FreeBSD, we use Clang's built-in vendor ID functionality, as does Apple. Here's a recent Clang/LLVM change that prompted investigation https://reviews.llvm.org/D69925

@kaduk
kaduk approved these changes Aug 26, 2020
Copy link
Contributor

@kaduk kaduk left a comment

I agree that this is trivial

Copy link
Contributor

@paulidale paulidale left a comment

LGTM. Agreed that it is trivial.

@openssl-machine
Copy link

@openssl-machine openssl-machine commented Aug 28, 2020

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request Aug 28, 2020
For example, FreeBSD prepends "FreeBSD" to version string, e.g.,

FreeBSD clang version 11.0.0 (git@github.com:llvm/llvm-project.git llvmorg-11.0.0-rc2-0-g414f32a9e86)
Target: x86_64-unknown-freebsd13.0
Thread model: posix
InstalledDir: /usr/bin

This prevented us from properly detecting AVX support, etc.

CLA: trivial

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from #12725)
openssl-machine pushed a commit that referenced this pull request Aug 28, 2020
For example, FreeBSD prepends "FreeBSD" to version string, e.g.,

FreeBSD clang version 11.0.0 (git@github.com:llvm/llvm-project.git llvmorg-11.0.0-rc2-0-g414f32a9e86)
Target: x86_64-unknown-freebsd13.0
Thread model: posix
InstalledDir: /usr/bin

This prevented us from properly detecting AVX support, etc.

CLA: trivial

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from #12725)

(cherry picked from commit cd84d88)
@kaduk
Copy link
Contributor

@kaduk kaduk commented Aug 28, 2020

Pushed to master and 1.1.1.
Note that util/perl/OpenSSL/config.pm doesn't exist on 1.1.1 (and there is no analogous clang check), so there was a little touch-up in the cherry-pick but it was basically clean.
Thanks for the submission! Closing the PR accordingly.

@kaduk kaduk closed this Aug 28, 2020
swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
For example, FreeBSD prepends "FreeBSD" to version string, e.g.,

FreeBSD clang version 11.0.0 (git@github.com:llvm/llvm-project.git llvmorg-11.0.0-rc2-0-g414f32a9e86)
Target: x86_64-unknown-freebsd13.0
Thread model: posix
InstalledDir: /usr/bin

This prevented us from properly detecting AVX support, etc.

CLA: trivial

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from openssl#12725)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants