Bug with GENERATED file and target_sources PRIVATE

Version: 3.23.0-rc2

Hi, I believe I have run into a bug regarding generated files via add_custom_command and target_sources when subdirectories are involved.

Bear with me please, as this is a bit weird.

Here is a directory structure:

make_gen.py
CMakeLists.txt
platform/
    CMakeLists.txt
    src.cpp

CMakeLists.txt:

# add_library(platform STATIC)
add_subdirectory(platform)

set(GEN_CPP ${CMAKE_CURRENT_BINARY_DIR}/gen.cpp)

add_custom_command(
        OUTPUT ${GEN_CPP}
        COMMAND python ${PROJECT_SOURCE_DIR}/make_gen.py ${GEN_CPP}
        DEPENDS ${PROJECT_SOURCE_DIR}/make_gen.py
)

target_sources(platform PRIVATE ${GEN_CPP})

platform/CMakeLists.txt:

add_library(platform STATIC) 
target_sources(platform PRIVATE src.cpp)

This results in 'gen.cpp': No such file or directory during the build of the target.
It seems like the generated file isnt registered properly.

However, if I set the target_sources to PUBLIC, it does work, but that is not behavior we want (its a source file being compiled into a library, and we dont want dependent targets to also do that).

Also, if we instead declare the library in the top-level CMakeLists.txt and comment it out in the platform sub-directory, it also works.

Source file properties are a bit strange in their interactions with directories. What happens if you set a minimum CMake version via cmake_minimum_required() to 3.20 (to get CMP0118)?

Ah I should have posted that it, I already have it set to:
cmake_minimum_required(VERSION 3.23.0)

This seems like some kind of path transformation doesn’t work. Is there differing behavior between NInja and Makefile generators?

I’m on windows so I don’t think I can try Makefile behavior.

Ah. Can you post the command which leads up to the given error message then?

Yep, the names dont exactly match above as I tried to make them more generic, but this is the actual error:

[98/109] cmd.exe /c
[99/109] Building CXX object platform\windows\CMakeFiles\gd_platform.dir\__\register_platform_apis.gen.cpp.obj
FAILED: platform/windows/CMakeFiles/gd_platform.dir/__/register_platform_apis.gen.cpp.obj 
C:\PROGRA~1\MIB055~1\2022\COMMUN~1\VC\Tools\MSVC\1431~1.311\bin\Hostx64\x64\cl.exe  /nologo /TP -DAPI_NAME=Vulkan -DDEBUG_ENABLED -DDISABLE_DEPRECATED -DMSVC -DNOMINMAX -DTOOLS_ENABLED -DTYPED_METHOD_BIND -DVK_USE_PLATFORM_WIN32_KHR -DVULKAN_ENABLED -DWASAPI_ENABLED -DWIN32 -DWIN64 -DWINDOWS_ENABLED -DWINMIDI_ENABLED -ID:\Projects\temp\godot-meson -ID:\Projects\temp\godot-meson\platform\windows -ID:\Projects\temp\godot-meson\thirdparty\vulkan -ID:\Projects\temp\godot-meson\thirdparty\vulkan\include -ID:\Projects\temp\godot-meson\build\debug /DWIN32 /D_WINDOWS /EHsc /Zi /Ob0 /Od /RTC1 -MDd /utf-8 -std:c++17 /showIncludes /Foplatform\windows\CMakeFiles\gd_platform.dir\__\register_platform_apis.gen.cpp.obj /Fdplatform\windows\CMakeFiles\gd_platform.dir\gd_platform.pdb /FS -c D:\Projects\temp\godot-meson\build\debug\platform\register_platform_apis.gen.cpp
c1xx: fatal error C1083: Cannot open source file: 'D:\Projects\temp\godot-meson\build\debug\platform\register_platform_apis.gen.cpp': No such file or directory

Now that I look at it, I realize that there are some weird __ going on in the paths…

Ninja has issues with path equivalence as it does string comparisons. You’ll need to use / everywhere; something is using \ as a separator. You can use file(TO_CMAKE_PATH) to do this transformation as needed.

The __ comes from the transform of .. in any relative path to a source file.

1 Like

Okay so, I did a search and I am not using any backslashes anywhere in any CMakeLists.txt files.

I did some further analysis and found that for some reason the custom command does not seem to be generated properly in to ninja.

Unlike above, my actual source structure looks like this:

platform/
    CMakeLists.txt
    gen.cpp # this is where gen.cpp is referenced, but to the current build directory
    windows/
        CMakeLists.txt
        src.cpp

So, just to be clear…

platform/CMakeLists.txt:

# add_library(gd_platform STATIC)
add_subdirectory(windows)

set(GEN_CPP ${CMAKE_CURRENT_BINARY_DIR}/register_platform_apis.gen.cpp)

add_custom_command(
        OUTPUT ${GEN_CPP}
        COMMAND python ${PROJECT_SOURCE_DIR}/scripts/register_platform_apis.py ${GEN_CPP}
        DEPENDS ${PROJECT_SOURCE_DIR}/scripts/register_platform_apis.py
)

target_sources(gd_platform PRIVATE ${GEN_CPP})

platform/windows/CMakeLists.txt:

add_library(gd_platform STATIC) 
target_sources(gd_platform PRIVATE src.cpp)

So I took a look at the ninja files in both cases. The only difference between the builds is where the gd_platform target is declared.

When the gd_platform target is added in platform/windows/:

build D$:\Projects\temp\godot-meson\build\debug\platform\register_platform_apis.gen.cpp: CUSTOM_COMMAND || cmake_object_order_depends_target_gd_platform
  COMMAND = cmd.exe /c
  restat = 1

When the gd_platform target is added in platform/:

build platform\register_platform_apis.gen.cpp | ${cmake_ninja_workdir}platform\register_platform_apis.gen.cpp: CUSTOM_COMMAND ..\..\scripts\register_platform_apis.py || core\gd_authors core\gd_cert core\gd_donors core\gd_license core\gd_virtual gd_generated gd_shaders scene\gd_fonts scene\gd_icons thirdparty\vulkan\vulkan.lib
  COMMAND = cmd.exe /C "cd /D D:\Projects\temp\godot-meson\build\debug\platform && python D:/Projects/temp/godot-meson/scripts/register_platform_apis.py D:/Projects/temp/godot-meson/build/debug/platform/register_platform_apis.gen.cpp"
  DESC = Generating register_platform_apis.gen.cpp
  restat = 1

In the first case, the COMMAND seems completely butchered :confused:

So, I think I’ve managed to narrow it down a bit. So far it seems that any GENERATED file via a custom command wont be registered properly if the target is not declared in the same CMakeLists.txt. I ran into the same issues in another spot…

The only workaround I have managed to do is to immediately declare a new interface library, and then link that library to the target…

eg.

set(LOGO_SCRIPT ${PROJECT_SOURCE_DIR}/scripts/platform_logo.py)
set(LOGO_PNG ${CMAKE_CURRENT_SOURCE_DIR}/logo.png)
set(LOGO_GEN ${CMAKE_CURRENT_BINARY_DIR}/logo.gen.h) 

add_custom_command(
    OUTPUT ${LOGO_GEN}
    COMMAND 
        python ${LOGO_SCRIPT} ${LOGO_PNG} ${LOGO_GEN} windows
    DEPENDS ${LOGO_PNG} ${LOGO_SCRIPT}
)
add_library(gd_icon_windows INTERFACE)
target_sources(gd_icon_windows PRIVATE ${LOGO_GEN})
target_link_libraries(gd_platform PRIVATE gd_icon_windows)

If I were to move the gd_platform target into that directory, then the icon would be registered via target_sources, but then the other generated files would no longer work.

Yes that’s the reason. When you are creating a file as an output of an add_custom_command(), a target defined in the same directory scope must depend on that output file. It is not enough for a target in a different directory to depend on it, that’s not supported. The add_custom_command() docs state the following, which probably should be stated in reverse instead to highlight this constraint:

A target created in the same directory (CMakeLists.txt file) that specifies any output of the custom command as a source file is given a rule to generate the file using the command at build time.

Policy CMP0118 only covers whether the GENERATED property is visible to a different directory scope. The actual dependencies are separate from that.

Aha I see, I must have missed it.

Then is the ideal solution to this to use an interface library like I have above? The only downside I seem to find is that it pollutes my target list in CLion and other IDE’s :sweat_smile:

Personally I wouldn’t use that directory structure to begin with. I recommend defining the target at the higher level, then only add sources to it from that level or below. It’s a bit strange to be adding a source from a level above where the target is defined.

I don’t disagree, and flipping the where it was declared wasn’t an issue. But add_custom_command only seems to work for targets declared in the current directory, not sub directories either. So even in a top-down approach we still need intermediate INTERFACE targets it seems.

If I move the target declaration up to platform in this case, then the generated code in platform works. However, generated files in platform/windows/ now need an interface target to work. And vice versa.

Sorry, I was a bit short on time when making that last reply. You do need some kind of target defined in the same scope as the one you have your add_custom_command() for generating the file. Typically, I see projects create a custom target using something like:

add_custom_target(gd_icon_windows_gen_src
    DEPENDS ${LOGO_GEN}
)

and then you’d have to ensure generation happens before the real target tries to use it, which you could achieve with:

add_dependencies(gd_icon_windows gd_icon_windows_gen_src)

There are other ways of achieving the same thing, using something other than a custom target, but the above would be more like the canonical pattern projects typically use. You can’t get away from adding a target in your scenario, so may as well make it a simple custom target rather than trying to get fancy with something else.

I also encountered this bug, similar directory structure, with “file(GENERATE ...)” instead of “add_custom_command(...)”, and cmake version 3.23.2. Fortunately I found this defect report.

As you mentioned it worked if you put everything in the top-level “CMakeLists.txt”, so I replaced “add_subdirectory(subdir)” in the top-level “CMakeLists.txt” with “include(subdir/CMakeLists.txt)”, which also worked around the bug.

So the problem is the new scope.

Well, more specifically, it’s that some of these things are mediated by directory properties. Without add_subdirectory, a new set of properties isn’t created.