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.

Sign up for GitHub

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

Implement Use CMake Header File Sets #30

Merged
merged 8 commits into from
Sep 28, 2024
Merged

Implement Use CMake Header File Sets #30

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
26367d7
include folder is not working
wusatosi Sep 24, 2024
735a7e6
fixed include header
wusatosi Sep 24, 2024
fc85080
removed unnecessary directives
wusatosi Sep 24, 2024
e2a8352
update dockerfile to reflect cmake change
wusatosi Sep 24, 2024
6edc629
switch to CMAKE_INSTALL_LIBDIR
wusatosi Sep 25, 2024
f4ff6ac
remove INSTALL directive
wusatosi Sep 25, 2024
62f37d9
Revert "remove INSTALL directive"
wusatosi Sep 25, 2024
b9b56e0
update DESTINATION location
wusatosi Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .ci/docker/ubuntu.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

# Using a non-LTS Ubuntu, just until CMake 3.23 is available on Ubuntu 24.04.
# Using a non-LTS Ubuntu, just until CMake 3.24 is available on Ubuntu 24.04.
FROM ubuntu:23.10

# Install dependencies,
Expand Down Expand Up @@ -30,5 +30,6 @@ ENV CC="$cc" CXX="$cxx" CMAKE_GENERATOR="Ninja" CMAKE_EXPORT_COMPILE_COMMANDS=on
RUN ls -lR src
RUN cmake -B build -S . "$cmake_args"
RUN cmake --build build --verbose
RUN cmake --build build --target all_verify_interface_header_sets
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big deal, but is the --verbose flag worth adding for the all_verify_interface_header_sets command? If we see a compiler error in a CI log, do we see enough detail to identify the relevant compilation commands?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure I can add the verbose flag. Logs are free.

Copy link
Member

Choose a reason for hiding this comment

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

#6 tries to remove the Dockerfiles. Maybe we should decide if we'll do that or not, as it may influence current PR.

Copy link
Member Author

@wusatosi wusatosi Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I have not checked failure case but I don't think verbose provide any useful information, should we still add the verbose flag?

Copy link
Member Author

@wusatosi wusatosi Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I can work on this later too but can you help me produce a case where the all_verify_interface_header_sets fails? I can go check the log and see if verbose provide more information.

Copy link
Contributor

@bretbrownjr bretbrownjr Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

For instance, I would expect the following to work in a normally-formed CMake library but fail an all_verify_interface_header_sets check.

// @file something.hpp
struct something::Widget {
  // BUG: Uses std::vector but doesn't include <vector>
  std::vector vec;
};

// @file something.cpp
#include <vector>
// something.hpp as expanded here gets vector defined for "free"
// by the preceeding #include, so something.cpp compiles fine
#include <something.hpp>

namespace {

size_t something::foobar(something::Widget const& widget) {
    return widget.vec.size();
}

} // unnamed namespace

I just want to make sure the person looking at a failure log can tell:

  • That something.hpp is being checked with a specific compile command
    • (Something about building a temporary like $some_build_dir/something.hpp.cpp probably)
  • What the compile command is, because flags like header search paths can be relevant
  • Exactly what the problem is

If we get that with --verbose off, I'm happy. We can land a shorter log and everyone wins.

If we don't get it regardless of whether --verbose is on or off, let's make an upstream CMake issue. I wouldn't expect tweaking this would be tricky or expensive for them.

Copy link
Contributor

Choose a reason for hiding this comment

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

#6 tries to remove the Dockerfiles. Maybe we should decide if we'll do that or not, as it may influence current PR.

Since #6 is closed, no worries on this PR, right?

Copy link
Member Author

@wusatosi wusatosi Sep 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

#6 tries to remove the Dockerfiles. Maybe we should decide if we'll do that or not, as it may influence current PR.

Since #6 is closed, no worries on this PR, right?

We shouldn't need to worry about that.

Copy link
Member Author

@wusatosi wusatosi Sep 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Without verbose Flag 
⋊> ~/D/test-cmake cmake --build build --target verify_interface_header_sets
[ 40%] Built target something_lib
[ 60%] Generating verify_something.hpp.cpp

[ 60%] Generating verify_something.hpp.cpp

[ 80%] Building CXX object CMakeFiles/verify_something.dir/verify_something.hpp.cpp.o
cc1plus: fatal error: /home/bradwu/Desktop/test-cmake/build/verify_something.hpp.cpp: No such file or directory
compilation terminated.
gmake[3]: *** [CMakeFiles/verify_something.dir/build.make:80: CMakeFiles/verify_something.dir/verify_something.hpp.cpp.o] Error 1
gmake[2]: *** [CMakeFiles/Makefile2:113: CMakeFiles/verify_something.dir/all] Error 2
gmake[1]: *** [CMakeFiles/Makefile2:146: CMakeFiles/verify_interface_header_sets.dir/rule] Error 2
gmake: *** [Makefile:150: verify_interface_header_sets] Error 2
With Verbose Flag 
⋊> ~/D/test-cmake cmake --build build --target verify_interface_header_sets --verbose
/home/linuxbrew/.linuxbrew/Cellar/cmake/3.26.3/bin/cmake -S/home/bradwu/Desktop/test-cmake -B/home/bradwu/Desktop/test-cmake/build --check-build-system CMakeFiles/Makefile.cmake 0
/usr/bin/gmake  -f CMakeFiles/Makefile2 verify_interface_header_sets
gmake[1]: Entering directory '/home/bradwu/Desktop/test-cmake/build'
/home/linuxbrew/.linuxbrew/Cellar/cmake/3.26.3/bin/cmake -S/home/bradwu/Desktop/test-cmake -B/home/bradwu/Desktop/test-cmake/build --check-build-system CMakeFiles/Makefile.cmake 0
/home/linuxbrew/.linuxbrew/Cellar/cmake/3.26.3/bin/cmake -E cmake_progress_start /home/bradwu/Desktop/test-cmake/build/CMakeFiles 5
/usr/bin/gmake  -f CMakeFiles/Makefile2 CMakeFiles/verify_interface_header_sets.dir/all
gmake[2]: Entering directory '/home/bradwu/Desktop/test-cmake/build'
/usr/bin/gmake  -f CMakeFiles/something_lib.dir/build.make CMakeFiles/something_lib.dir/depend
gmake[3]: Entering directory '/home/bradwu/Desktop/test-cmake/build'
cd /home/bradwu/Desktop/test-cmake/build && /home/linuxbrew/.linuxbrew/Cellar/cmake/3.26.3/bin/cmake -E cmake_depends "Unix Makefiles" /home/bradwu/Desktop/test-cmake /home/bradwu/Desktop/test-cmake /home/bradwu/Desktop/test-cmake/build /home/bradwu/Desktop/test-cmake/build /home/bradwu/Desktop/test-cmake/build/CMakeFiles/something_lib.dir/DependInfo.cmake --color=
gmake[3]: Leaving directory '/home/bradwu/Desktop/test-cmake/build'
/usr/bin/gmake  -f CMakeFiles/something_lib.dir/build.make CMakeFiles/something_lib.dir/build
gmake[3]: Entering directory '/home/bradwu/Desktop/test-cmake/build'
gmake[3]: Nothing to be done for 'CMakeFiles/something_lib.dir/build'.
gmake[3]: Leaving directory '/home/bradwu/Desktop/test-cmake/build'
[ 40%] Built target something_lib
/usr/bin/gmake  -f CMakeFiles/verify_something.dir/build.make CMakeFiles/verify_something.dir/depend
gmake[3]: Entering directory '/home/bradwu/Desktop/test-cmake/build'
[ 60%] Generating verify_something.hpp.cpp
/home/linuxbrew/.linuxbrew/Cellar/cmake/3.26.3/bin/cmake -E echo #include\ <something.hpp> > /home/bradwu/Desktop/test-cmake/build/verify_something.hpp.cpp

cd /home/bradwu/Desktop/test-cmake/build && /home/linuxbrew/.linuxbrew/Cellar/cmake/3.26.3/bin/cmake -E cmake_depends "Unix Makefiles" /home/bradwu/Desktop/test-cmake /home/bradwu/Desktop/test-cmake /home/bradwu/Desktop/test-cmake/build /home/bradwu/Desktop/test-cmake/build /home/bradwu/Desktop/test-cmake/build/CMakeFiles/verify_something.dir/DependInfo.cmake --color=
gmake[3]: Leaving directory '/home/bradwu/Desktop/test-cmake/build'
/usr/bin/gmake  -f CMakeFiles/verify_something.dir/build.make CMakeFiles/verify_something.dir/build
gmake[3]: Entering directory '/home/bradwu/Desktop/test-cmake/build'
[ 60%] Generating verify_something.hpp.cpp
/home/linuxbrew/.linuxbrew/Cellar/cmake/3.26.3/bin/cmake -E echo #include\ <something.hpp> > /home/bradwu/Desktop/test-cmake/build/verify_something.hpp.cpp

[ 80%] Building CXX object CMakeFiles/verify_something.dir/verify_something.hpp.cpp.o
/usr/bin/c++  -I/home/bradwu/Desktop/test-cmake -std=gnu++17 -MD -MT CMakeFiles/verify_something.dir/verify_something.hpp.cpp.o -MF CMakeFiles/verify_something.dir/verify_something.hpp.cpp.o.d -o CMakeFiles/verify_something.dir/verify_something.hpp.cpp.o -c /home/bradwu/Desktop/test-cmake/build/verify_something.hpp.cpp
cc1plus: fatal error: /home/bradwu/Desktop/test-cmake/build/verify_something.hpp.cpp: No such file or directory
compilation terminated.
gmake[3]: *** [CMakeFiles/verify_something.dir/build.make:80: CMakeFiles/verify_something.dir/verify_something.hpp.cpp.o] Error 1
gmake[3]: Leaving directory '/home/bradwu/Desktop/test-cmake/build'
gmake[2]: *** [CMakeFiles/Makefile2:113: CMakeFiles/verify_something.dir/all] Error 2
gmake[2]: Leaving directory '/home/bradwu/Desktop/test-cmake/build'
gmake[1]: *** [CMakeFiles/Makefile2:146: CMakeFiles/verify_interface_header_sets.dir/rule] Error 2
gmake[1]: Leaving directory '/home/bradwu/Desktop/test-cmake/build'
gmake: *** [Makefile:150: verify_interface_header_sets] Error 2

Verbose doesn't provide more useful information, and honestly the error log is pretty bad. The error fails because it cannot find the verify_something.hpp.cpp, even with verbose on nothing indicate the reason for failure to generate this file.

Tested on cmake version 3.26.3.

Can someone test this on newer cmake version? We should file an issue upstream if no more helpful error message is generated.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tested on cmake version 3.30.3, same output, I will file a issue upstream, let's omit the verbose flag.

version 3.30.3 with verbose log 
⋊> ~/D/test-cmake cmake --build build --verbose                                                                                                                                (base) 13:45:00
Change Dir: '/home/bradwu/Desktop/test-cmake/build'

Run Build Command(s): /home/linuxbrew/.linuxbrew/Cellar/cmake/3.30.3/bin/cmake -E env VERBOSE=1 /usr/bin/gmake -f Makefile
/home/linuxbrew/.linuxbrew/Cellar/cmake/3.30.3/bin/cmake -S/home/bradwu/Desktop/test-cmake -B/home/bradwu/Desktop/test-cmake/build --check-build-system CMakeFiles/Makefile.cmake 0
/home/linuxbrew/.linuxbrew/Cellar/cmake/3.30.3/bin/cmake -E cmake_progress_start /home/bradwu/Desktop/test-cmake/build/CMakeFiles /home/bradwu/Desktop/test-cmake/build//CMakeFiles/progress.marks
/usr/bin/gmake  -f CMakeFiles/Makefile2 all
gmake[1]: Entering directory '/home/bradwu/Desktop/test-cmake/build'
/usr/bin/gmake  -f CMakeFiles/something_lib.dir/build.make CMakeFiles/something_lib.dir/depend
gmake[2]: Entering directory '/home/bradwu/Desktop/test-cmake/build'
cd /home/bradwu/Desktop/test-cmake/build && /home/linuxbrew/.linuxbrew/Cellar/cmake/3.30.3/bin/cmake -E cmake_depends "Unix Makefiles" /home/bradwu/Desktop/test-cmake /home/bradwu/Desktop/test-cmake /home/bradwu/Desktop/test-cmake/build /home/bradwu/Desktop/test-cmake/build /home/bradwu/Desktop/test-cmake/build/CMakeFiles/something_lib.dir/DependInfo.cmake "--color="
Dependencies file "CMakeFiles/something_lib.dir/something.cpp.o.d" is newer than depends file "/home/bradwu/Desktop/test-cmake/build/CMakeFiles/something_lib.dir/compiler_depend.internal".
Consolidate compiler generated dependencies of target something_lib
gmake[2]: Leaving directory '/home/bradwu/Desktop/test-cmake/build'
/usr/bin/gmake  -f CMakeFiles/something_lib.dir/build.make CMakeFiles/something_lib.dir/build
gmake[2]: Entering directory '/home/bradwu/Desktop/test-cmake/build'
gmake[2]: Nothing to be done for 'CMakeFiles/something_lib.dir/build'.
gmake[2]: Leaving directory '/home/bradwu/Desktop/test-cmake/build'
[ 40%] Built target something_lib
/usr/bin/gmake  -f CMakeFiles/verify_something.dir/build.make CMakeFiles/verify_something.dir/depend
gmake[2]: Entering directory '/home/bradwu/Desktop/test-cmake/build'
[ 60%] Generating verify_something.hpp.cpp
/home/linuxbrew/.linuxbrew/Cellar/cmake/3.30.3/bin/cmake -E echo #include\ <something.hpp> > /home/bradwu/Desktop/test-cmake/build/verify_something.hpp.cpp

cd /home/bradwu/Desktop/test-cmake/build && /home/linuxbrew/.linuxbrew/Cellar/cmake/3.30.3/bin/cmake -E cmake_depends "Unix Makefiles" /home/bradwu/Desktop/test-cmake /home/bradwu/Desktop/test-cmake /home/bradwu/Desktop/test-cmake/build /home/bradwu/Desktop/test-cmake/build /home/bradwu/Desktop/test-cmake/build/CMakeFiles/verify_something.dir/DependInfo.cmake "--color="
gmake[2]: Leaving directory '/home/bradwu/Desktop/test-cmake/build'
/usr/bin/gmake  -f CMakeFiles/verify_something.dir/build.make CMakeFiles/verify_something.dir/build
gmake[2]: Entering directory '/home/bradwu/Desktop/test-cmake/build'
[ 60%] Generating verify_something.hpp.cpp
/home/linuxbrew/.linuxbrew/Cellar/cmake/3.30.3/bin/cmake -E echo #include\ <something.hpp> > /home/bradwu/Desktop/test-cmake/build/verify_something.hpp.cpp

[ 80%] Building CXX object CMakeFiles/verify_something.dir/verify_something.hpp.cpp.o
/usr/bin/c++  -I/home/bradwu/Desktop/test-cmake -std=gnu++17 -MD -MT CMakeFiles/verify_something.dir/verify_something.hpp.cpp.o -MF CMakeFiles/verify_something.dir/verify_something.hpp.cpp.o.d -o CMakeFiles/verify_something.dir/verify_something.hpp.cpp.o -c /home/bradwu/Desktop/test-cmake/build/verify_something.hpp.cpp
cc1plus: fatal error: /home/bradwu/Desktop/test-cmake/build/verify_something.hpp.cpp: No such file or directory
compilation terminated.
gmake[2]: *** [CMakeFiles/verify_something.dir/build.make:80: CMakeFiles/verify_something.dir/verify_something.hpp.cpp.o] Error 1
gmake[2]: Leaving directory '/home/bradwu/Desktop/test-cmake/build'
gmake[1]: *** [CMakeFiles/Makefile2:113: CMakeFiles/verify_something.dir/all] Error 2
gmake[1]: Leaving directory '/home/bradwu/Desktop/test-cmake/build'
gmake: *** [Makefile:91: all] Error 2

RUN cmake --install build --prefix /opt/beman.exemplar
RUN find /opt/beman.exemplar -type f
21 changes: 11 additions & 10 deletions src/beman/exemplar/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,21 @@ add_library(beman::exemplar ALIAS beman.exemplar)

target_sources(beman.exemplar PRIVATE identity.cpp)

target_include_directories(
beman.exemplar
PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../../../include>)
set(INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/../../../include/)

target_sources(
beman.exemplar PUBLIC
FILE_SET HEADERS
BASE_DIRS ${INCLUDE_DIR}
FILES ${INCLUDE_DIR}/beman/exemplar/identity.hpp)

set_target_properties(beman.exemplar PROPERTIES VERIFY_INTERFACE_HEADER_SETS ON)

install(
TARGETS beman.exemplar
EXPORT beman.exemplar
DESTINATION ${CMAKE_INSTALL_LIBDIR})

install(
DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/../../../include/
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
FILES_MATCHING
PATTERN "*.hpp")
DESTINATION ${CMAKE_INSTALL_LIBDIR}
DeveloperPaul123 marked this conversation as resolved.
Show resolved Hide resolved
FILE_SET HEADERS)

if(BUILD_TESTING)
include(GoogleTest)
Expand Down