Repository navigation
fix: generate timefold-build.properties in the module's build directory (#2629) - #2696
Merged
rsynek merged 2 commits intoOct 14, 2026
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
One or more issues must be addressed before approval.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Fixes multi-module path resolution for generated platform build properties and hands the absolute path to Quarkus.
Changes:
- Uses
${project.build.directory}for generated properties. - Adds system-property handoff and fallback resolution.
- Expands tests and documentation.
| File | Description |
|---|---|
| service/tools/maven-plugin/src/test/java/ai/timefold/solver/tools/maven/ConfigureMojoTest.java | Updated as part of this pull request. |
| service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/ConfigureMojo.java | Updated as part of this pull request. |
| service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AbstractPlatformModelMojo.java | Updated as part of this pull request. |
| service/tools/maven-plugin/README.adoc | Updated as part of this pull request. |
| service/quarkus/deployment/src/test/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldBuildConfigOverridesTest.java | Updated as part of this pull request. |
| service/quarkus/deployment/src/main/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldBuildConfigOverrides.java | Updated as part of this pull request. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
|
@Sumit-Ks1 thank you for this contribution. @rsynek I am assigning you to this since you reported the issue. |
ConfigureMojo resolved the output path with Paths.get(target, ...), which is relative to the JVM working directory (where mvn was launched), not to the module being built. In a multi-module build or when running from a parent directory, the file ended up in the wrong target directory. The consumer (TimefoldBuildConfigOverrides) used the same broken relative path. Both sides being wrong in the same way made the bug invisible, but fixing only the writer would silently break the reader. Changes: - ConfigureMojo now resolves the path from buildDirectory (inherited from AbstractPlatformModelMojo, defaulting to project.build.directory) - The absolute path is published as timefold.build.properties.location System property - TimefoldBuildConfigOverrides reads that property, falling back to the old relative path for backwards compatibility - The property is cleared on every execute() entry to prevent stale values from a previous reactor module - Tests cover multi-module path resolution, property lifecycle, and both modules assert the shared literal - README.adoc documents the corrected path and the handover mechanism Known limitation (pre-existing, not introduced): parallel reactor builds (mvn -T) can race on the single System property. The generated files are correct per module, but the handover property can be overwritten. Closes TimefoldAI#2629
rsynek
force-pushed
the
fix/generate-build-properties-in-module-directory
branch
from
October 13, 2026 18:55
d3fb747 to
7479480
Compare
rsynek
approved these changes
Oct 14, 2026
rsynek
left a comment
Contributor
There was a problem hiding this comment.
Thanks @Sumit-Ks1! One suggestion inline, otherwise looks very good.
Co-authored-by: Radovan Synek <radovan.synek@gmail.com>
This branch is waiting to be deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
Fixes #2629 where
timefold-build.propertieswas generated relative to the working directory (Paths.get("target", ...)) rather than the module's build directory.In a multi-module build or when running Maven from a parent directory, the properties file ended up in the parent's target directory (
A/target/...) rather than the submodule's target directory (A/B/target/...).Furthermore, the Quarkus deployment consumer (
TimefoldBuildConfigOverrides) previously suffered from the identical bug, reading relative to the JVM working directory. Fixing only the writer would have silently broken the reader in multi-module builds.Changes
ConfigureMojo:buildDirectory(inherited fromAbstractPlatformModelMojo, defaulting to${project.build.directory}) instead ofPaths.get("target", ...).timefold.build.properties.locationsystem property.execute()to avoid stale values across reactor modules.TimefoldBuildConfigOverrides:timefold.build.properties.locationfirst if set.Paths.get("target", ...)for backwards compatibility when the system property is not present (e.g. standalone test runs or older builds).ConfigureMojoTestto test custom build directory resolution, system property publication, and lifecycle cleanup.TimefoldBuildConfigOverridesTestto verify handover via system property, fallback behavior, missing/unreadable files, and cross-module constant consistency.README.adocto reference${project.build.directory}and document the handover mechanism.Closes #2629