COMP: Do not expose plug-in autogen dirs to dependents - #1444
Conversation
Dependents get every dependency's <dep>_autogen/include_<CONFIG> on their include path so that AUTOUIC-generated headers can be found. That directory also holds AUTOMOC's output, in a subdirectory named after a hash of the source directory relative to the source tree, and mocs_compilation.cpp includes its moc output with angle brackets. Plug-ins of a project tend to keep their sources in the same relative directory, so they all share that hash. CMake appends a target's own autogen include dir last, so a plug-in whose dependency has a header of the same name wins the search and the plug-in ends up compiling the dependency's meta object instead of its own. That leaves the plug-in's own QObject without a meta object and pulls in an undefined constructor of the foreign class. Skip plug-ins when contributing these directories. Libraries keep them, since they do need each other's generated ui_*.h, and their distinct relative source layouts do not collide.
|
@kislinsk could you please add a minimal test that fails without this change? Without tests, it will take forever the AI agents to find a solution that fixes the issue for CTK without breaking the Slicer build. @hjmjohnson could you please have a look at this? |
Covers which directories ctkFunctionGetIncludeDirs contributes for a library dependency and for a plug-in dependency: both end up on the include path, but only the library contributes its autogen directory. Running the test against the previous behaviour fails on the plug-in case, since that directory was contributed for plug-ins as well.
|
Added one, using the existing On Slicer: the change only affects what a plug-in dependency contributes, libraries are For what it's worth, that is also the case I got wrong first: I initially dropped the |
ctkMacroBuildPlugin passes everything it collected for compiling a
plug-in to target_include_directories() as PUBLIC, so the autogen
include directories contributed by the dependencies end up in the
INTERFACE_INCLUDE_DIRECTORIES of the exported target.
Those paths contain $<CONFIG>. A consumer expands it for every
configuration it generates, while CTK only ever created the directory
for the configurations it was built in, so CMake rejects the imported
target:
Imported target "org_commontk_eventadmin" includes non-existent path
".../CTKPluginFramework_autogen/include_RelWithDebInfo"
in its INTERFACE_INCLUDE_DIRECTORIES.
Single-config generators are unaffected, because the directory is then
called include and does exist, which is why this only shows on
multi-config generators.
A dependency's generated ui_*.h is needed to compile the plug-in, not
by anything downstream, so contribute those directories as PRIVATE.
|
While syncing MITK to this branch our Windows CI hit a third problem from the same
INTERFACE_INCLUDE_DIRECTORIES ".../CTKPluginFramework_autogen/include$<$<BOOL:1>:_$<CONFIG>>"A consumer expands This one is worth a look for Slicer specifically: unlike the moc collision, it does not Note the offending entry comes from CTKPluginFramework, a library dependency, so the |
|
This is not done yet. The current solution (with and without this PR) tends to produce very long file names. So long, that MSVC/Windows is not able to handle such object files any more (should be < 260 chars). E.g.: I try to come up with something less repetitive and shorter... |
The default autogen directory is <target>_autogen, and a plug-in target name is the symbolic name with dots replaced by underscores. It therefore repeats, in full, the name of the directory it sits in. Together with the generated source names, whose length is driven by the same symbolic name, this reaches the 260 character path limit of the Windows toolchain for plug-ins with long names, and the compiler fails with "Cannot open compiler generated file". The autogen directory only has to be unique per target. The plug-in's own binary directory already provides that, so the target name adds nothing but length.
The generated manifest and cached-resource .qrc files were named after the symbolic name of the plug-in. Both are written to the plug-in's own binary directory, so the name is unique without it, and AUTORCC derives the name of the generated source from it. For a plug-in with a long symbolic name that source ends up carrying the name twice, once in the directory and once in the file, which is a noticeable part of the 260 character path limit of the Windows toolchain. The resource prefix, which is what lookups use at runtime, comes from the content of the .qrc file and is unaffected.
|
Two more commits, both aimed at the path length. COMP: Shorten the autogen directory of a plug-in sets AUTOGEN_BUILD_DIR to /autogen instead of the default _autogen. A plug-in target name is the symbolic name with dots replaced by underscores, so the default repeats, in full, the name of the directory it already sits in. The directory only has to be unique per target, and a plug-in has its own binary directory — the macro already warns when the project name and the directory name disagree. This applies to plug-ins only; libraries keep the default layout. That matters for the earlier commits in this PR: ctkFunctionGetIncludeDirs still contributes _autogen/include… for library dependencies, and the filter that keeps those out of the exported interface still matches them. Plug-in autogen directories are no longer contributed at all, so nothing else refers to them by path. COMP: Shorten the names of the generated plug-in .qrc files renames the generated <symbolic_name>_manifest.qrc and <symbolic_name>_cached.qrc to manifest.qrc and cached.qrc. Both are written to the plug-in's own binary directory, so the symbolic name is not needed for uniqueness, and AUTORCC derives the generated source name from the .qrc name. The resource prefix, which is what runtime lookups use, comes from the content of the .qrc and is unchanged. Effect on the longest generated path in an MITK build, for the plug-in that was failing: |
|
Thanks for the updates @kislinsk. I'll wait for @hjmjohnson to have a look at these proposed changes. |
2 participants
Disclaimer: We (MITK) are currently catching up on the latest CTK master branch
since we forked for Qt 6 back in 2023. We found a few bugs that are not caught by
your CI and mostly affect external users of CTK.
Priority: highest. Regression from the commit that added AUTOUIC header
propagation to
ctkFunctionGetIncludeDirs.Dependents receive every dependency's
<dep>_autogen/include[_<CONFIG>]on their include path so that adependency's AUTOUIC-generated
ui_*.hresolves. That directory also holdsAUTOMOC's output, in a subdirectory named after a hash of the source
directory relative to the source tree, and
mocs_compilation.cppincludesits moc output with angle brackets.
Plug-ins of a project therefore share the hash, because they share the
relative layout (for MITK, every plug-in keeps its sources in
src/internal,hash
TLJAIHGYSZ). CMake appends a target's own autogen include dir last(observed at position 92 of 71+ entries, with dependency dirs at 9-44), so a
dependency with a same-named header wins the search.
The result: a plug-in compiles a dependency's meta object instead of its own.
Concretely, every MITK plug-in has
src/internal/mitkPluginActivator.h, sothey all picked up
org.mitk.core.services' version:The fix skips plug-ins when contributing these directories.
${dep}_INCLUDE_SUFFIXESis set only byctkMacroBuildPlugin, never byctkMacroBuildLib, so it is a reliable discriminator. Libraries keep thepropagation, which
Libs/DICOM/Widgetsgenuinely needs forCTKWidgets'ui_ctkThumbnailListWidget.h, and their distinct relative layouts cannotcollide.
Note on an alternative that does not work: putting the target's own
autogen include dir earlier in
my_includeshas no effect, because CMakecollapses it to its own late-appended copy. Verified in the generated
.vcxproj.