Support -D system properties for compiler options (#275) - #29
devin-ai-integration[bot] wants to merge 3 commits into
Conversation
…lpers#275) Co-Authored-By: Andreas Igel <andreas.igel@computacenter.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| if (value == null) { | ||
| value = System.getProperty(argument.getCompilerArgument()); | ||
| } | ||
|
|
There was a problem hiding this comment.
I guess the ordering is wrong, right? I I am executing it with -A or with -D it both should have a higher priority then the config in pom.xml!? Otherwise you can not just check things, you always would need to change pom.xml?
There was a problem hiding this comment.
You're right — -A from pom.xml and -A from the CLI land in the same javac option map, so with the old order a -D could never override a pom setting. Changed in e30c859: precedence is now -D system property > prefixed -A > bare-name -A, test inverted (readValue_CompilerArgAndSystemPropertySet_PrefersSystemProperty), docs updated.
Also corrected the docs regarding Gradle: I verified that -Dsimplebuilder.<option> works with Gradle too (default in-process compiler, and via systemProp.… in gradle.properties); it only fails with options.fork = true, and Gradle doesn't treat the property as a task input (needs clean/--rerun-tasks).
…ers#275) Co-Authored-By: Andreas Igel <andreas.igel@computacenter.com>
|
|
||
| ### Command Line (-D) | ||
|
|
||
| Maven supports compiler options without a `pom.xml` change, for example: `mvn compile -Dsimplebuilder.generateJavaDoc=DISABLED`. |
There was a problem hiding this comment.
Fixed: "Maven and Gradle accept the options as JVM system properties without a build-file change", with a mvn and a gradle compileJava example (9f012cd).
|
|
||
| 1. **Check option names**: Ensure you're using the full option name (e.g., `-Asimplebuilder.generateFieldSupplier`) | ||
| 1. **Check option names**: Ensure you're using the full option name (e.g., `-Asimplebuilder.generateFieldSupplier`); Maven also supports the `-Dsimplebuilder.<option>` fallback | ||
| 2. **Verify processor is running**: Ensure annotation processor is configured correctly |
There was a problem hiding this comment.
It is not the fallback, it is the default. And it is not maven only.
There was a problem hiding this comment.
Fixed: now "the same name also works as a JVM system property (-Dsimplebuilder.<option>), which takes precedence over -A" — no "fallback", no Maven-only wording (9f012cd).
| public String readValue(CompilerArgumentsEnum argument) { | ||
| // Try with full compiler argument name first (e.g., "simplebuilder.verbose") | ||
| String value = processingEnv.getOptions().get(argument.getCompilerArgument()); | ||
| // Try the prefixed JVM system property first (e.g., "simplebuilder.verbose") |
There was a problem hiding this comment.
Naming, that this is the -D option?
There was a problem hiding this comment.
Now explicitly labelled: -Dsimplebuilder.generateFieldSupplier=true (JVM system property, highest precedence), -Asimplebuilder.… (compiler argument), -AgenerateFieldSupplier=true (bare option name, backward compatibility). 9f012cd.
| String value = System.getProperty(argument.getCompilerArgument()); | ||
|
|
||
| // Fall back to simple option name for backward compatibility (e.g., "verbose") | ||
| // Fall back to the full compiler argument name (e.g., "simplebuilder.verbose") |
There was a problem hiding this comment.
Why naming here "full" compiler argument? Isn't it just the compiler argument?
There was a problem hiding this comment.
Agreed, "full" was misleading. Comments now read: -D JVM system property → -A compiler argument → bare option name (backward compatibility). 9f012cd.
| * <ul> | ||
| * <li>{@code -Asimplebuilder.generateFieldSupplier=true} | ||
| * <li>{@code -Dsimplebuilder.generateFieldSupplier=true} as a system-property alternative | ||
| * <li>{@code -Asimplebuilder.builderAccess=public} | ||
| * <li>etc. |
There was a problem hiding this comment.
-D first, because it is handled first? And what about the option name? And where are the option from
Annotations handled? On different place?
There was a problem hiding this comment.
Yes, -D is checked first because it must win: build-file -A options and command-line -D both reach the processor, and only this order lets a CLI -D override the pom/gradle setting without editing it. The bare option name (-Averbose) is the last lookup for backward compatibility. Annotation options are not handled here at all: BuilderConfigurationReader reads @SimpleBuilder.Options per annotated type and merges them on top of this global configuration, giving @Options > -D > -A > default. The readBuilderConfiguration Javadoc now lists all three forms in precedence order and states where annotation options are handled (9f012cd).
Co-Authored-By: Andreas Igel <andreas.igel@computacenter.com>
|
moved to java-helpers#287 |
Summary
mvn compile -Dsimplebuilder.<option>=<value>(and the Gradle equivalent) now works without a-Asimplebuilder.<option>=${simplebuilder.<option>}mapping in the build file. Previously the docs suggested it did, but the processor only readprocessingEnv.getOptions()(i.e.-Aargs), so-Dalone was silently ignored (java-helpers#275).CompilerArgumentsReader.readValuenow resolves:The system property is checked first:
-Aargs frompom.xml/build.gradleand from the CLI land in the same javac option map, so a-Dgiven on the command line would otherwise never be able to override a setting configured in the build file. Only the prefixed name is looked up as a system property — a bare-Dverbose=trueis deliberately ignored to avoid colliding with unrelated JVM properties. Since every option goes throughreadValue, this covers all ofreadBuilderConfiguration,verbose,performanceTracking*anddeactivateGenerationComponentsat once.Effective precedence:
@SimpleBuilder.Options>-Dsystem property >-Acompiler arg > default.Verified end-to-end (not committed): Maven
mvn -pl example clean compile -D…changes the generated builders; Gradle 8.10 with the default in-process compiler (-D…on the CLI orsystemProp.simplebuilder.…ingradle.properties) does too. It does not work with forked javac (<fork>true</fork>/options.fork = true) or IDE builds —-Aremains required there — and Gradle does not treat the property as a task input (compileJavastays UP-TO-DATE withoutclean/--rerun-tasks). All of this is documented indocs/CONFIGURATION.md/docs/DEBUG_LOGGING.md.Tests: three
CompilerArgumentsReaderTestcases (fallback,-Dwins over-A, bare-name property ignored).Closes java-helpers#275.
Link to Devin session: https://app.devin.ai/sessions/d691231fba0842a4b842b275ca4b35ed
Open in Devin Desktop: https://app.devin.ai/desktop/session/d691231fba0842a4b842b275ca4b35ed?variant=devin
Requested by: @AndreasIgel