Skip to content

scripts: fix clang version detection on certain Linux distributions#651

Merged
ojeda merged 1 commit intoRust-for-Linux:rustfrom
macano953:fix-for-libclang-clang-versions-mismatch-warning
Jan 27, 2022
Merged

scripts: fix clang version detection on certain Linux distributions#651
ojeda merged 1 commit intoRust-for-Linux:rustfrom
macano953:fix-for-libclang-clang-versions-mismatch-warning

Conversation

@macano953
Copy link
Copy Markdown

@macano953 macano953 commented Jan 26, 2022

Prevents the libclang vs clang version mismatch warning message from showing up when the clang version reported by 'clang --version' contains a trailing Linux distro version.

Example of non-working scenario:

$ LC_ALL=C clang --version 2>/dev/null
Ubuntu clang version 11.0.0-2~ubuntu20.04.1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Signed-off-by: Miguel Cano macanroj@gmail.com

@ojeda
Copy link
Copy Markdown
Member

ojeda commented Jan 26, 2022

Oh, I used that regex precisely to allow for any trailing part while keeping the command as simple as possible, but I didn't expect another 3-part full version in the trailing part... Thanks a lot for catching this!

I think we can be a bit more strict on the matching (to still match a single thing), something like:

sed -nE 's:.*version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'

By the way, please keep the first head -n 1 or use 1s -- the idea is that the version should really be on the first line, so we should not look elsewhere.

@macano953
Copy link
Copy Markdown
Author

macano953 commented Jan 26, 2022 via email

Prevents the libclang vs clang version mismatch warning message from showing up when the clang version reported by 'clang --version' contains the Linux distro version

Signed-off-by: macano953 <macanroj@gmail.com>
@macano953 macano953 force-pushed the fix-for-libclang-clang-versions-mismatch-warning branch from 50be0ed to 47889d7 Compare January 27, 2022 08:36
@ojeda
Copy link
Copy Markdown
Member

ojeda commented Jan 27, 2022

Looks good. We will probably want to update the other places in the file with a similar approach to be consistent, but this can go in already. Thanks!

@ojeda ojeda merged commit 3fe71f8 into Rust-for-Linux:rust Jan 27, 2022
@ojeda
Copy link
Copy Markdown
Member

ojeda commented Jan 27, 2022

By the way, in the PR message above you correctly had your full name in the Signed-off-by line, but the Git commit author field and the Signed-off-by line in the commit do not have it -- please fix it for the next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants