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 compilation issues when when using Ubuntu 18.04 + clang12 #2189

Merged
merged 1 commit into from
Apr 4, 2022

Conversation

piponazo
Copy link
Collaborator

@piponazo piponazo commented Apr 4, 2022

I found some issues when using a non very common setup:

  • OS: Ubuntu 18.04 x64
  • Compiler: clang-12, downloaded from the pre-compiler binaries available at LLVM

I was trying to create a build with CLANG and the FUZZ app enabled in order to debug some issues and I found out 2 problems:

  1. The fuzz app could be compiler but it was missing the std::filesystem symbols.
  2. The unit tests target could not be compiled because it was used some deprecated functions
# 1
[2/5] Linking CXX executable bin/fuzz-read-print-write
FAILED: bin/fuzz-read-print-write 
: && ccache /opt/clang+llvm-12.0.1-x86_64-linux-gnu-ubuntu-/bin/clang++  -fsanitize=fuzzer-no-link -fno-omit-frame-pointer -fsanitize=address,undefined -fno-sanitize-recover=all -g3 -gstrict-dwarf -O0  -fsanitize=fuzzer-no-link -fno-omit-frame-pointer -fsanitize=address,undefined -fno-sanitize-recover=all    -fsanitize=fuzzer  -fstack-protector-strong fuzz/CMakeFiles/fuzz-read-print-write.dir/fuzz-read-print-write.cpp.o  -o bin/fuzz-read-print-write  -Wl,-rpath,/home/users/diazmasl/personal/exiv2/buildFuzz/lib  lib/libexiv2.so.1.0.0.9 && :
lib/libexiv2.so.1.0.0.9: undefined reference to `std::filesystem::current_path[abi:cxx11]()'
lib/libexiv2.so.1.0.0.9: undefined reference to `std::filesystem::__cxx11::path::_M_split_cmpts()'
lib/libexiv2.so.1.0.0.9: undefined reference to `std::filesystem::__cxx11::path::has_root_directory() const'
lib/libexiv2.so.1.0.0.9: undefined reference to `std::filesystem::rename(std::filesystem::__cxx11::path const&, std::filesystem::__cxx11::path const&)'
lib/libexiv2.so.1.0.0.9: undefined reference to `std::filesystem::__cxx11::path::has_filename() const'
lib/libexiv2.so.1.0.0.9: undefined reference to `std::filesystem::remove(std::filesystem::__cxx11::path const&)'
lib/libexiv2.so.1.0.0.9: undefined reference to `std::filesystem::status(std::filesystem::__cxx11::path const&)'
clang-12: error: linker command failed with exit code 1 (use -v to see invocation)
#2
[3/7] Building CXX object unitTests/CMakeFiles/unit_tests.dir/test_slice.cpp.o
FAILED: unitTests/CMakeFiles/unit_tests.dir/test_slice.cpp.o 
ccache /opt/clang+llvm-12.0.1-x86_64-linux-gnu-ubuntu-/bin/clang++  -DTESTDATA_PATH=\"/home/users/diazmasl/personal/exiv2/test/data\" -Dexiv2lib_STATIC -I. -I../src -I../include/exiv2 -I../include -isystem /home/users/diazmasl/.conan/data/gtest/1.10.0/_/_/package/579bb3da356b87ffe3f10143f6025bb63a25a8d7/include -fsanitize=fuzzer-no-link -fno-omit-frame-pointer -fsanitize=address,undefined -fno-sanitize-recover=all -g3 -gstrict-dwarf -O0 -fvisibility=hidden -fvisibility-inlines-hidden     -fstack-clash-protection -fcf-protection -fstack-protector-strong -Wp,-D_GLIBCXX_ASSERTIONS -Wall -Wcast-align -Wpointer-arith -Wformat-security -Wmissing-format-attribute -Woverloaded-virtual -W -Wno-error=format-nonliteral -Werror -Wdeprecated-copy -std=gnu++17 -MD -MT unitTests/CMakeFiles/unit_tests.dir/test_slice.cpp.o -MF unitTests/CMakeFiles/unit_tests.dir/test_slice.cpp.o.d -o unitTests/CMakeFiles/unit_tests.dir/test_slice.cpp.o -c ../unitTests/test_slice.cpp
../unitTests/test_slice.cpp:114:1: error: 'TypedTestCase_P_IsDeprecated' is deprecated: TYPED_TEST_CASE_P is deprecated, please use TYPED_TEST_SUITE_P [-Werror,-Wdeprecated-declarations]
TYPED_TEST_CASE_P(slice);

Althought we are not seeing any of these 2 issues at the moment in our CI jobs, this PR should fix those 2 "hiden issues".

@piponazo
Copy link
Collaborator Author

piponazo commented Apr 4, 2022

Of course, this would fail if you install gtest from a system-package containing the old version of gtest (1.8.X) instead of the newer one (1.10.X) which works better with modern C++ standards 🤦 . In the CI jobs we have running on Mac, we do not use conan for brining the dependencies, but we install some system packages:
https://github.com/Exiv2/exiv2/actions/runs/2088236611

At this point we have 2 options:

  1. Adapt CI jobs using Mac to bring dependencies with conan. We would be officially requiring to use gtest >= 1.10.X for being able to develop and run tests in this project.
  2. Drop the commit touching the unit test. We could still support old versions of gtest, but some users might have the kind of problems I had.

@codecov
Copy link

codecov bot commented Apr 4, 2022

Codecov Report

Merging #2189 (c4b3940) into main (3795e53) will not change coverage.
The diff coverage is n/a.

❗ Current head c4b3940 differs from pull request most recent head 9989133. Consider uploading reports for the commit 9989133 to get more accurate results

@@           Coverage Diff           @@
##             main    #2189   +/-   ##
=======================================
  Coverage   63.29%   63.29%           
=======================================
  Files          99       99           
  Lines       19596    19596           
  Branches     9559     9559           
=======================================
  Hits        12404    12404           
  Misses       5118     5118           
  Partials     2074     2074           
Impacted Files Coverage Δ
src/exif.cpp 65.91% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f496bd...9989133. Read the comment docs.

kmilos
kmilos previously approved these changes Apr 4, 2022
Copy link
Collaborator

@kmilos kmilos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

fuzz/CMakeLists.txt Show resolved Hide resolved
@piponazo piponazo merged commit 04e5f28 into main Apr 4, 2022
@piponazo piponazo deleted the mainCompilationIssues branch April 4, 2022 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants