Skip to content

build: Add explicit dependency for codegen#5640

Merged
kodiakhq[bot] merged 2 commits into
acts-project:mainfrom
stephenswat:build/add_codegen_targets_to_all
Jul 8, 2026
Merged

build: Add explicit dependency for codegen#5640
kodiakhq[bot] merged 2 commits into
acts-project:mainfrom
stephenswat:build/add_codegen_targets_to_all

Conversation

@stephenswat

@stephenswat stephenswat commented Jul 6, 2026

Copy link
Copy Markdown
Member

This commit establishes an explicit dependency between the targets we use in the code generation and the generated files. By default, CMake establishes no such dependency for INTERFACE targets, but CMake 3.19 allows us to add private sources to INTERFACE targets. This ensures that when ACTS is built, the codegen files are always ready to be installed. This is especially important for detray, which may opt not to even generate the files if no target depends on them, but they should still be available to detray clients.

@stephenswat

Copy link
Copy Markdown
Member Author

Tagging @niermann999.

niermann999
niermann999 previously approved these changes Jul 6, 2026
@github-actions github-actions Bot added this to the next milestone Jul 6, 2026
@github-actions github-actions Bot added the Infrastructure Changes to build tools, continous integration, ... label Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

📊: Physics performance monitoring for 79a6662

Full contents

physmon summary

❗️: Downstream build failure

  • eic-shell EICrecon (cc @acts-project/epic-contacts)

@paulgessinger

Copy link
Copy Markdown
Member

Does it not do that if it depends on the detray interface target? I'm changing detray to also contain compiled code in #5641, in that case at least it should trigger the generation, shouldn't it?

@stephenswat

Copy link
Copy Markdown
Member Author

Does it not do that if it depends on the detray interface target? I'm changing detray to also contain compiled code in #5641, in that case at least it should trigger the generation, shouldn't it?

With the current detray CMake setup, the generation of this code is not triggered, even when we export a detray interface target. Let me see what the mechanism is with #5641.

@stephenswat

Copy link
Copy Markdown
Member Author

Confirmed that #5641 also makes this problem go away.

@stephenswat

stephenswat commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

@paulgessinger @niermann999 as far as I can tell there are actually 3 solutions to this problem:

  1. To add the ALL keyword as I have done in this PR. Downside to this is that it might, in select pathological cases, cause the build system to generate more files than necessary, because it will cause all files to be generated by default, even the ones we don't install. Note that this doesn't mean we would e.g. generate detray codegen files when not building detray, because in that case the CMakeLists.txt file would never even be loaded.
  2. To turn detray_core into a PUBLIC library rather than an INTERFACE one. That's simple and implemented in refactor(detray): Detray-level closed-set detector instantiation #5641. However it is worth noting that this only masks the problem, if at any point we add more codegen to INTERFACE targets or we turn detray back into an interface library for any reason, the problem will reappear.
  3. Finally, as of CMake 3.19, it is possible to add private sources to INTERFACE targets (if that sounds confusing, I agree) which would explicitly establish a dependency between the target and the code generation in a way that works for both INTERFACE and PUBLIC targets. I think that's the most durable solution, and it avoids the possible over-generation of files that the ALL solution carries. This would carry the requirement to upgrade the minimum CMake version from 3.14 to 3.19.

Please let me know what y'all think.

@paulgessinger

Copy link
Copy Markdown
Member

@stephenswat I like it. If we raise the CMake floor maybe we should do it in a separate PR though.

@stephenswat stephenswat marked this pull request as draft July 7, 2026 13:16
@stephenswat

Copy link
Copy Markdown
Member Author

Waiting for #5647 so we can refactor this.

@stephenswat stephenswat force-pushed the build/add_codegen_targets_to_all branch from d36be50 to 264078a Compare July 7, 2026 13:25
@stephenswat stephenswat changed the title build: Add codegen targets to default target build: Add explicit dependency for codegen Jul 7, 2026
@stephenswat stephenswat force-pushed the build/add_codegen_targets_to_all branch from 264078a to b66ac75 Compare July 7, 2026 13:34
This commit establishes an explicit dependency between the targets we
use in the code generation and the generated files. By default, CMake
establishes no such dependency for INTERFACE targets, but CMake 3.19
allows us to add private sources to INTERFACE targets. This ensures that
when ACTS is built, the codegen files are always ready to be installed.
This is especially important for detray, which may opt not to even
generate the files if no target depends on them, but they should still
be available to detray clients.
@stephenswat stephenswat force-pushed the build/add_codegen_targets_to_all branch from b66ac75 to 9fd2fd8 Compare July 8, 2026 07:32
@stephenswat stephenswat marked this pull request as ready for review July 8, 2026 07:37
@sonarqubecloud

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

@kodiakhq kodiakhq Bot merged commit acc4063 into acts-project:main Jul 8, 2026
44 checks passed
@github-actions github-actions Bot removed the automerge label Jul 8, 2026
@andiwand andiwand modified the milestones: next, v47.0.0 Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge priority Infrastructure Changes to build tools, continous integration, ...

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants