add_custom_command fails when OUTPUT is generated into a subdirectory

jjELT · October 27, 2020, 4:23pm

Find attached a minimal example, where I am generating sources for a library my-lib using add_custom_command, an executable my-app depends on:

(I cannot actually upload anything yet as a new user, so find the sample project cmake-generate-sources.zip here: https://gitlab.kitware.com/cmake/cmake/-/issues/21367)

As long as I generate the files directly into my-lib's CMAKE_CURRENT_BINARY_DIR everything works fine. If I want to generate the files into a subdirectory e.g. generate, either configurating or building or both does not work anymore.

I created three different “versions” to demonstrate. You can try each setting the VERSION value in my-lib\CMakeLists.txt line 16.

The documentation states:

If an output name is a relative path it will be interpreted relative to the build tree directory corresponding to the current source directory.

What am I missing?

marc.chevrier · October 27, 2020, 4:27pm

Create the directory before trying to use it!

jjELT · October 27, 2020, 5:29pm

VERSION 1 already automatically creates the subfolder generate as expected. Still, I added the following code just before the existing add_custom_command part:

# Create the directory before trying to use it!
if(EXISTS OUTPUT_DIRECTORY)
	message("Creating output directory '${CMAKE_CURRENT_BINARY_DIR}/${OUTPUT_DIRECTORY}'")
	file(MAKE_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/${OUTPUT_DIRECTORY}")
endif()

# Add command to generate source code files before building the target.
add_custom_command(
...

The configuration for VERSION 1 works fine, but building throws the error:

x86_64-w64-mingw32-g++.exe: error: <CMAKE_BINARY_DIR>\my-lib\test-source-1.cpp: No such file or directory

which is correct, since the path should be <CMAKE_BINARY_DIR>\my-lib\generate\test-source-1.cpp.

The configuration for VERSION 2 does not work, it throws the error:

[cmake] -- Configuring done
[cmake] CMake Error at my-app/CMakeLists.txt:8 (add_executable):
[cmake]   Cannot find source file:
[cmake] 
[cmake]     <CMAKE_SOURCES_DIR>/my-lib/generate/test-source-2.h
[cmake] 
[cmake]   Tried extensions .c .C .c++ .cc .cpp .cxx .cu .m .M .mm .h .hh .h++ .hm
[cmake]   .hpp .hxx .in .txx
[cmake] 
[cmake] 
[cmake] -- Generating done

which of course does not work, since the file is generated into the <CMAKE_BINARY_DIR>.

So I added a third version specifying the full absolute path to the generated files:

...
elseif(${VERSION} EQUAL 3)
	# !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
	# This does NOT work.
	# !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
	set(OUTPUT_DIRECTORY "generate")

	set(GENERATED_HEADER "${CMAKE_CURRENT_BINARY_DIR}/${OUTPUT_DIRECTORY}/test-source-${VERSION}.h")
	set(GENERATED_IMPLEMENTATION "${CMAKE_CURRENT_BINARY_DIR}/${OUTPUT_DIRECTORY}/test-source-${VERSION}.cpp")

	set(INPUT_HEADER_FILE "${CMAKE_CURRENT_SOURCE_DIR}/test-source-${VERSION}.h.in")
	set(OUTPUT_HEADER_FILE "${CMAKE_CURRENT_BINARY_DIR}/${OUTPUT_DIRECTORY}/test-source-${VERSION}.h")

	set(INPUT_IMPLEMENTATION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/test-source-${VERSION}.cpp.in")
	set(OUTPUT_IMPLEMENTATION_FILE "${CMAKE_CURRENT_BINARY_DIR}/${OUTPUT_DIRECTORY}/test-source-${VERSION}.cpp")
endif()

...

elseif(${VERSION} EQUAL 3)

	set(TARGET_SOURCES_HEADER_FILE "${GENERATED_HEADER}")
	set(TARGET_SOURCES_IMPLEMENTATION_FILE "${GENERATED_IMPLEMENTATION}")
endif()

message("INPUT_HEADER_FILE is: ${INPUT_HEADER_FILE}")
message("OUTPUT_HEADER_FILE is: ${OUTPUT_HEADER_FILE}")
...

set(TARGET_SOURCES_HEADER_FILE ${TARGET_SOURCES_HEADER_FILE} PARENT_SCOPE)
set(TARGET_SOURCES_IMPLEMENTATION_FILE ${TARGET_SOURCES_IMPLEMENTATION_FILE} PARENT_SCOPE)

and in the main CMakeLists.txt I am marking the source files as GENERATED as I have read somewhere in this forum:

...

add_subdirectory(my-lib)

message("Parent: TARGET_SOURCES_HEADER_FILE ${TARGET_SOURCES_HEADER_FILE}")
message("Parent: TARGET_SOURCES_IMPLEMENTATION_FILE ${TARGET_SOURCES_IMPLEMENTATION_FILE}")

set_source_files_properties(${TARGET_SOURCES_HEADER_FILE} GENERATED)
set_source_files_properties(${TARGET_SOURCES_IMPLEMENTATION_FILE} GENERATED)

# -------------------------------------------------------------------------------
# Executables
# -------------------------------------------------------------------------------

add_subdirectory(my-app)

Still this does not work.

hsattler · October 27, 2020, 5:48pm

Why do you add the generated sources with PUBLIC?

jjELT · October 28, 2020, 7:11am

I need the header files in my-app, that is why they are PUBLIC. I could add the implementation as PRIVATE, but it does not change anything.

hsattler · October 28, 2020, 8:57am

You need the include directory (means same CMakeLists.txt). Having the files as PUBLIC or INTERFACE makeS the my-app target use them as own files. But generated files can only be seen by targets in the same directory.

dbahadir · October 28, 2020, 9:22am

First some general observations:

You should reconsider your indentation style. (Due to the usage of tabs your code looks pretty messed up in my editor.)
If you want to be able to compile your application successfully when the generated headers are found in version 2, you need to fix your MyApp.cpp first;

--- cmake-generate-sources/sources/my-app/MyApp.cpp
+++ cmake-generate-sources/sources/my-app/MyApp.cpp
@@ -5,6 +5,9 @@
 #if __has_include("generate/test-source-1.h")
 #include "generate/test-source-1.h"
 #endif
+#if __has_include("generate/test-source-2.h")
+#include "generate/test-source-2.h"
+#endif
 
 #include <iostream>

You do not want to use PUBLIC when adding your implementation file as source for your my-lib library, because that would mean that every consumer (in your case the my-app executable) will try to compile that source again when linking (via target_link_libraries) to that library.
That will very likely lead to “duplicate symbols” linker-errors.

--- cmake-generate-sources/sources/my-lib/CMakeLists.txt
+++ cmake-generate-sources/sources/my-lib/CMakeLists.txt
@@ -151,8 +151,9 @@
 target_sources(${TARGET_NAME}	PUBLIC
                                                                        # This works for VERSION 0
                                                                        ${TARGET_SOURCES_HEADER_FILE}
+								PRIVATE
                                                                        ${TARGET_SOURCES_IMPLEMENTATION_FILE}
-
+								PUBLIC
                                                                        # This does NOT work
                                                                        #${OUTPUT_HEADER_FILE}
                                                                        #${OUTPUT_IMPLEMENTATION_FILE}

Although not needed in this particular case, it might be beneficial for your my-lib library to carry its include-search paths as usage-requirements.

--- cmake-generate-sources/sources/my-lib/CMakeLists.txt
+++ cmake-generate-sources/sources/my-lib/CMakeLists.txt
@@ -9,6 +9,8 @@
 
 # Set linker language
 set_target_properties(${TARGET_NAME} PROPERTIES LINKER_LANGUAGE CXX)
+# Set include search path
+target_include_directories(my-lib INTERFACE "${CMAKE_CURRENT_BINARY_DIR}")
 
 # -------------------------------------------------------------------------------
 # Generate test sources.

Explicitly setting the GENERATED property on source files requires a lot of care and understanding, because that property is only visible in the directory scope it was set in/for.
Note, however, that this might change from CMake 3.20 on. (See https://gitlab.kitware.com/cmake/cmake/-/merge_requests/5308)

Now the solution for your example:

If you applied my above points 1 and 2, then you only need the following changes and version 2 will work:

--- cmake-generate-sources/sources/my-app/CMakeLists.txt
+++ cmake-generate-sources/sources/my-app/CMakeLists.txt
@@ -8,6 +8,9 @@
 add_executable(${TARGET_NAME})
 # Set linker language
 set_target_properties(${TARGET_NAME} PROPERTIES LINKER_LANGUAGE CXX)
+# Mark headers of my-lib as generated
+get_target_property(headers my-lib INTERFACE_SOURCES)
+set_source_files_properties("${headers}" PROPERTIES GENERATED 1)
 
 # ===============================================================================
 # Sources and Includes

--- cmake-generate-sources/sources/my-lib/CMakeLists.txt
+++ cmake-generate-sources/sources/my-lib/CMakeLists.txt
@@ -13,7 +13,7 @@
 # -------------------------------------------------------------------------------
 # Generate test sources.
 # -------------------------------------------------------------------------------
-set(VERSION 0)
+set(VERSION 2)
 
 if(${VERSION} EQUAL 0)
        # !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
@@ -131,9 +131,8 @@
 
 elseif(${VERSION} EQUAL 2)
 
-	# This works for some reason when configuring!?! But not when building.
- 	set(TARGET_SOURCES_HEADER_FILE "test-source-${VERSION}.h")
- 	set(TARGET_SOURCES_IMPLEMENTATION_FILE "test-source-${VERSION}.cpp")
+	set(TARGET_SOURCES_HEADER_FILE "${GENERATED_HEADER}")
+	set(TARGET_SOURCES_IMPLEMENTATION_FILE "${GENERATED_IMPLEMENTATION}")
 endif()
 
 message("INPUT_HEADER_FILE is: ${INPUT_HEADER_FILE}")

What this does is:

You give version 2 the correct file-paths to the generated sources and
in my-app/CMakeLists.txt you are retrieving the public source files (aka headers) from your my-lib library and set their GENERATED property in the current scope. This prevents add_executable from complaining that the headers cannot be found while cmaking.

Setting the GENERATED property in that directory might be obsolete if the above mentioned merge-request lands in CMake 3.20. However, CMake 3.20 will still need some months before it is released.

I hope that was understandable and helpful.
Deniz

jjELT · October 28, 2020, 9:32am

The my-app target is supposed to have access to the header files, automatically generated in the my-lib target. That is the whole point of what I am doing here. I therefore cannot add them as PRIVATE sources.

jjELT · October 28, 2020, 9:50am

I think I am actually using 4 spaces as indentation, which seems to be the default in the VisualStudioCode IDE. Also I enabled autoformatting on save, so I guess I am good with the settings?! I do not know why your editor is showing you a messed up format…
I probably didn’t do that, because I never solved the steps earlier. Eventually I would have fixed that.
I absolutely do want the HEADER to be public. But yes, the implementation can be private. I do not understand why both of you think this is not needed?? How else would my-app be able to call the functions defined in that header or use the WORLD definition (#define WORLD "World")?
That is essentially the solution I found a few minutes ago as well.

Find a full working example VERSION 4 in the attached project.

cmake-generate-sources.zip (11.4 KB)

hsattler · October 28, 2020, 10:12am

They don’t need to be public because the target-level dependency of my-app to my-lib assures that they are created before being used there.

What issue are you trying to solve when making them PUBLIC?

jjELT · October 28, 2020, 10:51am

Ahhhh… Ok, now I get it… You guys are correct, I can just add all generated sources as PRIVATE, I tried it out in my sample project.

I thought, that the #include of the header file in the MyApp.cpp file:

...
#if __has_include("my/namespace/my-header-file-4.h")
#include "my/namespace/my-header-file-4.h"
#endif
...

would not work, if the source was not declared as PUBLIC, but it does work, since in the my-lib target I am adding the include directory as PUBLIC

...
target_include_directories(${TARGET_NAME} PUBLIC "${CMAKE_CURRENT_BINARY_DIR}/generated")
...

meaning, it also ’ propagates’ to the my-app target. If I change it to PRIVATE I get

… was not declared in this scope

errors, when trying to build the my-app target

But now I am confused… Why would I ever need to add sources as PUBLIC? I guess I need to read up on that…

Thanks to both of you for your effort and the time you invest in helping!

dbahadir · October 28, 2020, 11:04am

I think I am actually using 4 spaces as indentation, which seems to be the default in the VisualStudioCode IDE. Also I enabled autoformatting on save, so I guess I am good with the settings?! I do not know why your editor is showing you a messed up format…

You probably set the tab width to 4 spaces. However, you are still using tabs and those are not automatically replaced by 4 spaces. My editor of choice shows tabs with a width of 8 spaces. That’s why it is messed up.

I absolutely do want the HEADER to be public. But yes, the implementation can be private. I do not understand why both of you think this is not needed?? How else would my-app be able to call the functions defined in that header or use the WORLD definition ( #define WORLD "World" )?

Because with target_link_libraries(my-app Private my-lib) you are saying to link against the library that is created by target my-lib. And that library already has the compiled source-file included.
If you put the *.cpp file in INTERFACE_SOURCES of my-lib (as you are insisting to do), that source file will be compiled twice, for my-lib and for my-app.
The one compiled for my-lib will go into the created library, the one compiled for my-app will go into the executable created my my-app. However, the library of my-lib will also get linked into that same executable, so your compiled source file will be linked twice into that executable. And the linker will not like that.

dbahadir · October 28, 2020, 11:14am

But now I am confused… Why would I ever need to add sources as PUBLIC ? I guess I need to read up on that…

Thanks to both of you for your effort and the time you invest in helping!

In general, you should never need to use target_sources with keywords PUBLIC or INTERFACE for *.cpp files (aka source files).
*.h files (aka header files) you never need to provide via target_sources in order to successfully compile the target. (However, you should give it the correct include-search path using target_include_directories.)
It can be beneficial to still add the headers to target_sources because some IDEs can then show these or you want to set some properties on these files from some other directory (like I did in my above solution).

Adding the *.cpp files with PUBLIC or INTERFACE to target_sources is only useful in special situations. But for the normal situation (which you have here) that is even plain wrong.

jjELT · October 28, 2020, 11:40am

Excellent. I will go through my project, because until now I always added the header files as PUBLIC. I will switch that to PRIVATE or completely remove the keyword. I guess default would be PRIVATE, right?! I am going to find that out…

I will probably still add the headers (as PRIVATE), since my colleagues are using other IDEs, e.g. Visual Studio and Eclipse CDT.

Thanks again for the information!

dbahadir · October 28, 2020, 12:31pm

You can leave the headers with the PUBLIC keyword as is. No need to change, it (currently) does no harm. (I do not know if this might change in the future but I doubt it.)

Besides, I am not sure if PRIVATE sources are enough to please IDEs or if they must be using INTERFACE or PUBLIC.

Just make sure to properly use target_include_directories so that other targets, that depend on such targets know where to look for the headers they want to #include.

Omitting the keyword in target_sources will not work.

jjELT · October 28, 2020, 1:46pm

I think I will still get some benefits when adding all sources as PRIVATE. My CMAKE code will probably get easier to read. Also I am currently keeping my .CPP and .H files separately in a ‘private’ and ‘include’ folder respectively. I will have a closer look and think about it or just try out some things.

I will mark your long reply as the solution, even if @hsattler was already going in the right direction in the first reply. I just didn’t yet understand how all of this fitted together and if I actually had needed them to be public, then marking them as GENERATED within the my-app directory would have been the solution.