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

improve -ffile-prefix-map detection #2517

Merged

Conversation

timblechmann
Copy link
Contributor

Description

the current implementation has two problems:

  • clang-cl does not know -ffile-prefix-map, but it identifies as "Clang", so the compiler will warn about an unknown compiler option
  • xcode's clang however does not identify as "Clang", but as "AppleClang". so -ffile-prefix-map is not passed although it is supported by the compiler

@codecov
Copy link

codecov bot commented Sep 9, 2022

Codecov Report

Merging #2517 (2848c2c) into devel (32eae0e) will increase coverage by 0.03%.
The diff coverage is n/a.

❗ Current head 2848c2c differs from pull request most recent head 752743b. Consider uploading reports for the commit 752743b to get more accurate results

@@            Coverage Diff             @@
##            devel    #2517      +/-   ##
==========================================
+ Coverage   91.54%   91.56%   +0.03%     
==========================================
  Files         183      183              
  Lines        7561     7560       -1     
==========================================
+ Hits         6921     6922       +1     
+ Misses        640      638       -2     

@jbadwaik
Copy link
Contributor

jbadwaik commented Sep 9, 2022

I think it's a bug with CMake if they identify two different compilers, clang and clang-cl, as the same.

https://gitlab.kitware.com/cmake/cmake/-/issues/22668

@horenmar
Copy link
Member

This will need some fixes to the path normalization code in approval tests.

the current implementation has two problems:
* `clang-cl` does not know `-ffile-prefix-map`, but it identifies as
"Clang", so the compiler will warn about an unknown compiler option
* xcode's clang however does not identify as "Clang", but as
"AppleClang". so `-ffile-prefix-map` is not passed although it is
supported by the compiler
@timblechmann timblechmann force-pushed the feature/silence-clang-cl-warning branch from 5a2e14f to 2848c2c Compare September 27, 2022 00:48
@timblechmann
Copy link
Contributor Author

This will need some fixes to the path normalization code in approval tests.

do you have any pointers for the entry points to do these fixes?

@horenmar
Copy link
Member

@timblechmann Approval tests run the SelfTest binary and compare the output to a known good one. The implementation is in tools/scripts/approvalTests.py, and the problematic part is in normalizePath function, which looks for the absolute path to the binary in the text output, and if it finds it, it removes prefix.

-ffile-prefix-map also removes the prefix, but uses a slightly different prefix than the approval test normalization. This leads me to an idea; we could instead change the mapping used by -ffile-prefix-map to ${CATCH_DIR}/=, so that the resulting path is the same as what the approval tests want.

@horenmar horenmar merged commit 728de35 into catchorg:devel Oct 4, 2022
@horenmar horenmar added the Building and Packaging Issues affecting build/packaging scripts and utilities label Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Building and Packaging Issues affecting build/packaging scripts and utilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants