Check build hints at compile time instead of shipping them inert - #5586
Open
shai-almog wants to merge 209 commits into
Open
Check build hints at compile time instead of shipping them inert#5586shai-almog wants to merge 209 commits into
shai-almog wants to merge 209 commits into
Conversation
A build hint is a `codename1.arg.<name>=<value>` line that reaches a builder as
`request.getArg(name, default)`. Nothing checked the name, so a misspelling was
accepted, copied into the build request, never read, and silently discarded: a
green build with the setting simply not applied.
Our own agent reference had been shipping `android.xPermissions`,
`android.minSdkVersion` and `android.sdkVersion` for exactly that reason. The
builders read `android.xpermissions`, `android.min_sdk_version`, and nothing at
all.
Most hints can now be written as annotations on the application's main class,
where javac does the checking: a misspelled name is an unknown symbol, a wrong
value type is a type error, and a value outside a hint's supported set is an
unknown enum constant.
@iOS(newStorageLocation = true, themeMode = IosThemeMode.MODERN)
@android(minSdkVersion = 24, useAndroidX = true)
@desktop(titleBar = DesktopTitleBar.NATIVE)
public class MyApplication extends Lifecycle {
}
The builders are untouched: `BuildHintAnnotationProcessor` converts the
annotations back into the same key/value pairs and `CN1BuildMojo` merges them
before the command-line overlay, the CN1Lib merges and both preflights, so a
library still appends onto an annotation-supplied value and `-D` still wins.
`Simulator` publishes them as system properties at startup so `cn1:run` sees
hints that no longer live in the properties file.
The properties file is untouched too. It stays the way to set the long tail and
the open-ended families such as `android.permission.<NAME>` that an annotation
cannot express, with no new warnings or errors. Declaring one hint both ways is
a build error.
One catalog, five generated views
---------------------------------
The hint set had been described in five places that had drifted apart: a prose
table in the developer guide, a runtime scraper of that table in the Settings
tool that guessed each type by string-matching the description, a fifteen-entry
schema in the simulator, a fourteen-entry separator map in the plugin, and a
hand-written agent reference. Only 147 of ~520 names appeared in more than one.
`maven/build-hint-catalog` is now the single source of truth (529 hints: 457
mined from the builders, 56 documented-but-unread, 16 dynamic families; 82
exposed as annotation attributes). The annotations, the binding table the
processor reads back, the guide's table, the simulator's editor schema and the
agent reference are all generated from it. The guide's table goes from 208 rows
to 529 with no prose lost.
Enums are emitted only where the accepted set is demonstrable from the code that
reads the hint -- `HardeningPreflight` rejects an unknown `harden.level`,
`IOSDependencyManager` throws on an unknown `ios.dependencyManager`, and
`GenerateDesktopAppWrapperMojo` silently falls back to `native` on an unknown
`desktop.titleBar`, which is the failure this removes.
Generated projects
------------------
The archetype and all four initializr templates now carry the annotations, and
`cn1:migrate-build-hints` moves an existing project over. Eleven in-repo
projects are migrated. `java.version` deliberately stays in the properties file:
it picks the toolchain that compiles the class the annotations live on.
Gates
-----
`scripts/check-build-hint-catalog.sh` fails when code reads a hint the catalog
does not describe, and when our own docs or templates name one that no builder
reads. Its baseline is empty, so it is a hard gate rather than a ratchet.
`scripts/gen-build-hint-annotations.sh --check` fails on generated-file drift.
Both run in the Java 8 leg of PR CI.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d2cfcfde3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
`.gitignore` carries a repo-wide `**/build/*`. The catalog's package is `com.codename1.build.shared`, so all 13 of its sources sat under a path segment named `build` and `git add` silently skipped them. Only `pom.xml` was committed: the module built locally from the working tree and produced an empty jar in CI, which is why `codenameone-maven-plugin` then failed with `cannot find symbol` on `BuildHints` and nearly every job went red. The sibling `platform-feature-catalog` lives in the same package and is fine, because it was added before that rule existed -- tracked files stay tracked, so nothing ever pointed at the hazard. Un-ignore `build` when it is a Java package rather than a build output directory, with the rationale beside the rule so the next file added there is not lost the same way. `maven/core/build/*` and `CodenameOne/build/*` stay ignored. Also from review: - Every bare `open()` in the four Python scripts now uses a context manager, so the handle closes even if parsing or `json.dump` raises, and the writes state their encoding. - The generator no longer emits an IP literal as an annotation default. PMD reads `default "127.0.0.1"` as hardcoded configuration, and the default clause is documentation only -- the processor emits a hint solely for members the developer actually wrote -- so the value moves to the javadoc where it belongs. - Files the migration touched that never carried a copyright header now have the complete one. The archetype's `__mainName__.java` is excluded instead: it is a template for the user's own application class, and stamping a Codename One GPL header onto it would put our licence on their code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Pin generated projects to a version containing the annotations
The updated Initializr starter archives now import and use com.codename1.annotations.buildhints, but GeneratorModel.java:44 still generates projects pinned to CN1 7.0.267, whose core artifact predates this package. Consequently every newly generated barebones, Kotlin, Grub, or Tweet project fails compilation on the unresolved annotations unless the user manually changes the CN1 version; either defer these template changes or update the generated runtime/plugin version to the first release containing them.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The archived bootstrap ran generation at module scope, so gen_external.py's `import gen_catalog` -- which only wants three helper functions -- rewrote every catalog source as a side effect. Generation and its diagnostics now live in `main()` behind a `__main__` guard, and the module-level file reads became `load_license()` / `load_mined()` / `load_docs()`, so importing does no I/O and cannot fail on inputs the archived copy deliberately does not carry. Verified both directions: importing leaves the catalog untouched, and running the two scripts end to end still reproduces the committed catalog byte for byte. Also drops `json` and `subprocess` from check-build-hint-catalog.py. Both were left from an earlier version that shelled out to the miner instead of importing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 12 screenshots: 12 matched. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d727c7d976
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
Collaborator
Author
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 163 screenshots: 163 matched. Benchmark Results
|
| Metric | Duration |
|---|---|
| SIMD kernel backend | SSE2 (x64) / NEON (arm64) native kernels |
| SIMD int-add (64K x300) | java 60ms / native 3ms = 20.0x speedup |
| SIMD float-mul (64K x300) | java 61ms / native 4ms = 15.2x speedup |
| SIMD kernel correctness | PASS (native result == scalar reference) |
| Base64 native bridge | unavailable (CN1 + SIMD + image benchmarks only) |
| Base64 payload size | 8192 bytes |
| Base64 benchmark iterations | 6000 |
| Base64 SIMD byte path | gated to scalar (CPU autovectorizes scalar; explicit SIMD not beneficial here) |
| Base64 CN1 encode | 187.000 ms |
| Base64 CN1 decode | 128.000 ms |
| Base64 SIMD encode | 108.000 ms |
| Base64 encode ratio (SIMD/CN1) | 0.578x (42.2% faster) |
| Base64 SIMD decode | 97.000 ms |
| Base64 decode ratio (SIMD/CN1) | 0.758x (24.2% faster) |
| Image encode benchmark iterations | 100 |
| Image createMask (SIMD off) | 22.000 ms |
| Image createMask (SIMD on) | 16.000 ms |
| Image createMask ratio (SIMD on/off) | 0.727x (27.3% faster) |
| Image applyMask (SIMD off) | 40.000 ms |
| Image applyMask (SIMD on) | 72.000 ms |
| Image applyMask ratio (SIMD on/off) | 1.800x (80.0% slower) |
| Image modifyAlpha (SIMD off) | 36.000 ms |
| Image modifyAlpha (SIMD on) | 30.000 ms |
| Image modifyAlpha ratio (SIMD on/off) | 0.833x (16.7% faster) |
| Image modifyAlpha removeColor (SIMD off) | 31.000 ms |
| Image modifyAlpha removeColor (SIMD on) | 25.000 ms |
| Image modifyAlpha removeColor ratio (SIMD on/off) | 0.806x (19.4% faster) |
Contributor
Cloudflare Preview
|
Collaborator
Author
|
Compared 163 screenshots: 163 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 163 screenshots: 163 matched. |
Collaborator
Author
|
Compared 163 screenshots: 163 matched. |
Collaborator
Author
|
Compared 163 screenshots: 163 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
Three separate gates rejected generated output. Each is fixed in the generator so the class of problem cannot come back through a catalog edit. Unmappable characters. The prose is imported from the developer guide, which uses typographic punctuation, and `CodenameOne/src` is also compiled by an Ant javac step with ASCII encoding where a single em dash is `error: unmappable character for encoding ASCII` -- a build failure, not a warning. A Unicode escape would not have helped: javac expands `\uXXXX` before it strips comments, so the character reappears. `toAscii` now folds the punctuation that actually occurs, and *refuses* anything it has no mapping for rather than dropping it, because silently deleting a character from a hint's documentation is the worse outcome. Broken table. `ios.spm.packages` is documented as `identity|url|requirement`, and a bare `|` starts a new AsciiDoc cell, so asciidoctor reported "dropping cells from incomplete row" for the whole 529-row table. Cells are escaped now. Vale. The guide enforces the Microsoft style as errors, and the generated table feeds it, so the catalog's prose has to satisfy it too: contractions, no "and so on", no stray adverbs. A default value is not prose, though -- the one remaining hit was `android.file_paths`, whose default is an XML fragment -- so a quoted default now carries the `// vale-skip:` comment .vale.ini documents for individual false positives. Also fixes a data bug the guide exposed. The miner preserved Java escape sequences instead of decoding them, so `android.file_paths` and `android.facebook_permissions` recorded defaults containing literal backslashes that the build never sees, and those reached the rendered table. The miner decodes escapes and re-quotes safely, and the two catalog entries are corrected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96bff9038a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`killedThreadReportsItselfFinished` failed the Java 21 leg with "FormTest timed out after 5000ms; edt=initialized pendingSerialCalls=0". The waits in this class used a 5000ms deadline, which is exactly the `@FormTest` timeout in EDTTestInterceptor -- so on a loaded runner the poll loop consumed the entire harness budget and the interceptor fired first. The report then said only that the method timed out, with nothing about which condition never became true. The waits now use 2000ms, well inside the harness budget and still roughly two thousand times the ~1ms these threads actually take to stop. A genuine regression now fails on the test's own assertion, which names what went wrong. Pre-existing (the test arrived with #5526) and unrelated to the build hint work: core-unittests has no dependency on the JavaSE port, so none of the simulator registration in this branch runs there, this branch changes nothing under com.codename1.db or EasyThread, and the Java 8 leg passed the same commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a343fe3335
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A mojo's defaultPhase does not bind it to a project -- the project's POM has to -- and nothing turns a build hint annotation back into a codename1.arg.* pair except the process-annotations goal. So migrating a project without that binding deleted working properties and replaced them with annotations no goal ever reads: the hints vanished from the build with no diagnostic anywhere. Five projects in this branch were already in that state. gamebuilder, docs/demos, video-builder and cn1playground bind the plugin but not that goal, so the binding is added. input-validation-app's common module has no build section at all, so its migration is reverted rather than inventing a lifecycle for a demo app. The goal now checks the reactor for the binding and refuses with the execution block to paste, so this cannot happen to anyone else. Three more from the same review: - The deletion pass recognized only `key=value`. `Properties.load` also accepts `key:value`, `key value`, escaped separators inside the key, and logical continuation lines; a declaration it failed to match was left behind while the annotation was added, so the next build failed with the duplicate-hint error this goal exists to prevent. Keys are parsed the way Properties.load defines them now, with a unit test per form. - The settings file was read as ISO-8859-1 and written back as UTF-8, turning any unrelated non-ASCII byte -- an accented displayName, say -- into mojibake. It is written back as ISO-8859-1. - cn1.androidTheme and cn1.nativeTheme are deprecated aliases of and.themeMode and nativeTheme, which the builders honour as fallbacks. Neither declared aliasOf, so conflict detection missed them and one value silently won. Also: the generation script rebuilt the generator only when its class was absent, so editing a catalog source and rerunning regenerated every view from the previous build's bytecode -- reporting success while ignoring the edit, and passing --check on a tree that was genuinely stale. It always rebuilds now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0edef42ca4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…plicating a hint docs/demos is the developer guide's snippet project: deliberately incomplete code fragments that illustrate @entity, @route, @AppIntent and @mapped. Binding process-annotations there put those snippets in front of the other processors, which correctly rejected six of them, so the migration is reverted and its two hints are back in the properties file. That the project omitted the goal was the point, not an oversight. The other three newly bound projects were checked rather than assumed: gamebuilder, video-builder and cn1playground each run process-annotations cleanly and emit 6, 3 and 5 hints respectively. Settings could still create the duplicate the migration is careful to avoid. In a generated project ios.themeMode and its neighbours are annotations, but the Build Hints UI decides a hint is inactive from the properties file alone and its Add button writes a property -- producing a second declaration that fails the next build. The tool now reads META-INF/codenameone/build-hints.properties, the file the processor writes on every build and deletes when the last annotation goes, and renders those hints read-only with the attribute that owns them: "Set by @iOS(themeMode) on the main class." An unbuilt project has no such file and behaves as before. Also fixes the SpotBugs finding this branch introduced: `backslashes % 2 == 1` in the continuation scan is false for negative odd numbers, so it is `!= 0`. The count cannot go negative, but the idiom is wrong regardless of that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Keep Initializr templates compatible with the pinned runtime
Every Initializr source archive now imports com.codename1.annotations.buildhints and uses the new annotations, while GeneratorModel.CN1_PLUGIN_VERSION still rewrites generated projects to 7.0.267, whose codenameone-core predates that package; the generated common POM also omits process-annotations. Consequently all newly downloaded Initializr projects fail compilation instead of receiving the defaults removed from common.zip's settings file. Leave these templates property-backed until Initializr targets the release containing this feature, or bump the generated version and bind the processor.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
Every project the archetype and the initializr produce is pinned to a released Codename One version -- the initializr hardcodes 7.0.267 in GeneratorModel.CN1_PLUGIN_VERSION -- and no released core carries com.codename1.annotations.buildhints. So a generated project would import annotations that do not resolve and fail to compile before the user has written a line, and the settings those templates stopped declaring would simply be gone. The templates are reverted to exactly their previous state: the archetype's __mainName__.java and codenameone_settings.properties, and the initializr's common.zip and four source archives. They can move to annotations in a follow-up once a release containing the package is out. The generated build hint table is dropped from the agent skill reference for the same reason -- it documented a form those projects cannot use yet -- so the generator no longer rewrites markdown at all. What stays from that area is unrelated to annotations: the skill reference described build hints that no builder reads, so a reader copying them got a green build and no effect. android.xPermissions is spelled android.xpermissions, android.minSdkVersion is android.min_sdk_version, and android.sdkVersion, android.googlePlayVersion, build.compile, build.timeout, javascript.html5, javascript.bundleResources and ios.orientation do not exist at all. Those corrections are right for the published version too, and the catalog gate now holds our own documentation to them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment directly above this line already states the rule it broke: `<source>` and `<sourceDir>` are ordinary words, so those two lookups are scoped to the plugin and goal that give them meaning. `<sourceDirectory>` was read across the entire POM. Checkstyle's <sourceDirectories><sourceDirectory> names a directory to C8C8 ANALYSE, and PMD's does the same, so an analysis-only path became a compile root -- and a dormant copy of the main class under it was then searched ahead of the compiled source, which is how annotation-owned hints come to look editable and Settings saves the duplicate the next build refuses. compileRootsOnly() drops the sections that configure something other than compilation before the element is looked for. Plugin configuration is reached through activePluginBlock, which knows which plugin and which goal it is reading; anything searched by element name alone has no such evidence, and an element name is not evidence. Two tests, both failing before the change with the analysis-only path in the root list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
configuredBuildDirectory already stripped the sections that configure something other than compilation before taking <directory> as a direct child. The <sourceDirectory> fix a commit ago wrote that rule out a second time and got a shorter list -- no <filters>, no <extensions> -- so a <filter> or <extension> naming a directory in one still became a compile root. Two copies of one rule, drifting, which is the shape this branch has now produced eight times. NON_COMPILE_SECTIONS is that rule once. The new test asserts both readers answer for the same POM, so an element added to either set cannot miss the other; running it against the shorter list fails with filter/only in the root list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 189238c461
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The introductory example still wrote `newStorageLocation = true`, which stopped compiling when that attribute became a Toggle: a developer copying the primary usage example got an incompatible-types error before any build hint could be processed. It survived the Toggle conversion for the reason this branch keeps repeating. The same example existed twice -- once here, once inside BuildHintCodeGenerator.packageInfoSource -- and the conversion updated the generator's. That copy is dead code, left from the emitter that used to generate this package, so the only visible one was the one nobody edited; it is deleted rather than fixed, since a second copy of documentation is exactly what went stale. The sweep missed it too, and specifically: the grep excluded annotations/buildhints/ to skip the attribute DECLARATIONS, which also skipped the only doc example in the tree. Re-run without that exclusion, this was the single remaining hit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c9d5f2b5e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Three reads matched a serialized substring while the rest of this reader goes through the trimming helpers, so a POM that pretty-prints -- still a perfectly ordinary POM -- lost a compile root the build really has: - <activeByDefault>\n true\n</activeByDefault> activates the profile for Maven and did not for this scan, so a root the profile declares went missing. It now goes through declaresValue, like the goal, id, phase and extension reads already did. - <artifactId>\n kotlin-maven-plugin\n</artifactId> resolves the plugin for Maven; pluginBlock and pluginBlocks looked for the exact string "<artifactId>kotlin-maven-plugin</artifactId>" and found nothing, so the Kotlin and build-helper roots vanished. Matched by trimmed VALUE now. - <relativePath/> is Maven's way of saying there is no local parent. elementValue cannot see a self-closing element, so it read as ABSENT and the reader walked up to ../pom.xml -- inheriting properties, a build directory and managed roots from a POM the project does not inherit from. Present-and-empty is a statement, and a different one from absent. Each is pinned by a test that fails when only that one is reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3c088db6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two more ways a root Maven really compiles went missing from the scan. The single-exit rewrite merged the MANAGED executions into whatever block was chosen and discarded the active block's own. A module that declares the add-source execution itself and inherits <sources> from plugin management therefore ended up with configuration and no binding, so compileGoalConfiguration read the goal as unbound. The active block's executions now travel with it. And <inherited>false</inherited> says a declaration applies to the POM that wrote it, not to its children, so an ancestor's copy is not part of this module's build at all -- the walk took it anyway, and a root it configures is one the module never compiles. That strip happens in pomChain() rather than in the walk that was reported. There are seven walks over that list; fixing the reported one is how this branch has produced ten review rounds, so the rule goes where the list is built and a walk added later cannot be the one that forgets. depth 0 is the module's own POM, which applies its declarations whatever it says about children. Each half fails its own test when reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21c0b4005b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…xecution Three things asked for, plus a defect each one uncovered. @hint(valuePattern=...) is a regex the whole value must match, checked by the processor against the value it is about to write. It exists for the hints whose TYPE cannot express their shape: String says nothing about ios.interface_orientation, whose value is a colon separated list of four known constants -- and the builder lowercases it and asks whether it CONTAINS each one, so a misspelling matches nothing, every orientation stays in the plist, and the app ships rotating in directions it was told not to. First user is that hint; the pattern was checked against java.util.regex itself, since (?1) is not supported there and would have compiled to nothing useful. android.min_sdk_version becomes AndroidMinSdk. It is not an arbitrary number: the builder parses it and compares it against real thresholds (multidex below 21, Android Auto below 23), so a level outside the range is a typo. android.targetSDKVersion deliberately stays an int -- the server's default for it is the highest platform IT has installed, an open-ended domain this framework build cannot enumerate ahead of time. That surfaced the same rule-copy bug the binding table had: cn1:migrate-build-hints derived the constant from the wire value, which gives V23 for "23" and TRUE for "true" -- source that does not compile, from the goal whose job is to migrate a project without breaking it. The recorded name now lives on BuildHints.Hint.constantFor and the migrator's own copy is deleted. And a child that redeclares an inherited execution id with <phase>none</phase> switches it off. The managed set was appended unfiltered AND the chosen block could itself be the ancestor's, so bindsGoal found an enabled copy either way and reported a root the build does not compile. Both places filter by disabled id now. Also fixes a test of mine that was timing-dependent: it wrote the manifest before the class, so the staleness fallback saw a class NEWER than its manifest and called it stale -- passing only when both writes landed in the same filesystem tick. Timestamps are set explicitly, as the older tests in that file already did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c3183b0a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The by-id execution merge kept the phase and the goals and dropped the <configuration>. Redeclaring an inherited id with only <configuration><sources> is how Maven says "same execution, different directory", so the merge left the goal on one execution and the root on another -- and compileGoalConfiguration, which looks for the source ON the bound execution, found none and omitted a directory Maven compiles. This is a regression I introduced myself: the configuration was in the merge during a restructure that broke three other tests, and backing that restructure out took the good part with it. It is added back on its own, against the narrow merge rather than a rewrite. And cn1:migrate-build-hints rendered a string that violates its hint's new valuePattern. The processor then refuses the annotation the goal just wrote, and the goal reacts to a failed verification build by rolling the WHOLE migration back -- so one misspelled ios.interface_orientation would cost a project the migration of every other hint, rather than staying in the properties file where it already was. It is refused up front now, the way an enum outside its domain and a non-canonical int already were. Each half fails its own test when reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c12106abe2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
target/classes was hardcoded. A project that configures <build><outputDirectory> compiles somewhere else and process-annotations writes the manifest there, so the lookup found nothing, every annotation-owned hint looked editable, and Add wrote the duplicate declaration the next build refuses -- the exact failure this manifest exists to prevent, reached by reading the wrong directory. Resolved in Maven's own layering: the configured <outputDirectory>, else <directory>/classes, else target/classes. As a DIRECT child of <build>, through the same section filter <directory> already used, because the compiler and resources plugins carry <outputDirectory> elements of their own and the first one in the build element is not necessarily the build's. Read from the POM because the binding does not carry it. A launcher that resolved it could pass it and this would become the fallback; that is the better shape and it needs a change on the Maven side to be worth anything. Tested: the direct-child rule, including that a plugin's own <outputDirectory> is neither preferred nor adopted. NOT covered by a new test: the wiring that makes loadAnnotationOwnedHints use this instead of the literal. Its binding comes from a static ProjectIO.loadBinding() that this test class has no way to stub, and faking the assertion would be worse than saying so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf58f2c841
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two more places where one reader knew a rule and its twin did not. The disabled-execution rule was applied to MANAGED declarations and not to active ones. Maven merges the active <plugins> of a parent and a child by plugin and then by execution id, while this reader parses each POM independently -- so a child that switches an inherited execution off with <phase>none</phase> still got the parent's enabled copy, and a root the build does not compile went into the scan. Stripped in pomChain(), beside the <inherited>false</inherited> strip and for the same reason: there are seven walks over that list and this is the one place none of them can forget. Per plugin, since an execution id means nothing across plugins. And the Settings source scan claimed ownership from the attribute NAME. `@Android(appBundle = Toggle.DEFAULT)` is written down but emits no hint -- wireValue treats it exactly as an omitted attribute -- so a properties line for it is legal, and claiming ownership hid the editor, refused Add, and reported a value the build accepts as a duplicate. That is a hole the Toggle conversion opened: before it, there was no way to write a hint attribute that says nothing. Compared against the constant the CATALOG records rather than the name DEFAULT. Every unset constant is spelled that way today and none of this should depend on it staying so. Each half fails its own test when reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb9a419f12
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Both refine yesterday's fixes rather than adding new rules. Inheritance is applied one level at a time, so an intermediate parent that disables the grandparent's execution disables it for the leaf too -- and the leaf never mentions it. Every ancestor was filtered against the leaf alone, so a three-level chain let the grandparent's enabled copy through with its root. pomChain() now carries the POMs it has already passed and filters each ancestor against all of them. And the unset constant is matched as an identifier TOKEN rather than as the text after the last dot. `Toggle./* default */DEFAULT` is an ordinary way to write it, the processor emits nothing for it, and the raw suffix comparison saw `/* default */DEFAULT`, claimed ownership, and hid a properties value the build accepts. Comments and string literals are dropped before the last identifier is taken, so a string that merely CONTAINS the word is not mistaken for the constant either. Character.isLetterOrDigit and StringBuilder.substring are not in the Codename One runtime API this tool compiles against; the bytecode compliance gate caught both, and the identifier test is spelled out instead. Each half fails its own test when reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…erver Four things, all the same complaint: the annotations are the source of truth, so nothing beside them should restate what they say. BuildHintAnnotationBinding is deleted. It was a generated table listing every annotation type by name, so the set of them existed twice and adding one meant remembering to regenerate. The processor now ENUMERATES the annotation package off the compile classpath -- directories and jars -- and reads what a member sets from the annotation itself. Read as bytecode, not reflection: @hint has CLASS retention, so loading the types and asking them returns nothing. sourceDigest() does the same by package PREFIX, since it is static and has no classpath to consult. consumedBy is gone from the annotations. It named the build service's own classes -- AndroidGradleBuilder, IPhoneBuilder -- inside something that ships to applications. A hint is a name and a value; how the service is organised behind it is not part of any contract a client has, may change witho CEA0 ut an app noticing, and a client-side copy of it can only go stale. The catalog keeps it for the hints IT describes, which is build-side code describing build-side code. IosThemeMode, AndroidThemeMode and NativeThemeMode become one ThemeMode. Three near-identical enums are three things to keep in step, which is worse than the overlap that separated them. Where a hint really accepts less than all of it, the attribute says so with @hint(valuePattern) -- and the generated catalog records only the values that pattern admits, so the editor and the documentation still show `auto, modern, hololight, legacy` for Android and never offer it `ios7`. Passing a constant the builder does not know is now a build error naming it, which three enums could not do either. And the developer guide says outright that build hints do not promise source compatibility, on purpose: a changed hint is MEANT to stop compiling, the failure is always on the client and never on the build service, and the properties form remains as the escape hatch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c383d00b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Deleting the three theme enums left five application main classes and the playground's generated access registry naming types that no longer exist, so those modules could not compile. The sweep that was supposed to catch this piped grep through `head`. Every line it printed came from maven/, the scripts/ hits were past the tenth, and I read "only test files reference these" off output that had been truncated. The check is re-run here with no limit and reports nothing left. GeneratedCN1Access and GeneratedAccess_com_codename1_annotations_buildhints are regenerated from the current sources, so they list ThemeMode, Toggle and AndroidMinSdk and no longer reference the deleted enums. The package-info example was wrong the same way -- it is the first thing a developer reads about this package and it named IosThemeMode. Verified by compiling: gamebuilder, certificatewizard, guibuilder and settings all build against the rebuilt core. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2b599d61e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Maven takes the NEAREST declaration that names a phase. A leaf that redeclares an id its parent disabled turns the execution back on and keeps the goal and configuration it inherits -- and unioning every level that says <phase>none</phase> kept the parent's veto, so a root the build really compiles was dropped from the scan. Resolved per id from the leaf outwards: the first level that names a phase decides, and a level that says nothing about the phase decides nothing and passes the question further up. That is the same walk as the previous fix for the opposite case, which is the point -- one rule now answers both directions instead of one direction each. Reverting to the union fails the new test with the grandparent's root gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56f819b1e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…cope Two more places where this reader answered a question the Maven-side reader already answers differently. Maven maps <sourceDirs> and <sources> to a LIST and ignores what the children are called -- AbstractCN1Mojo.addSourceDirsFrom iterates them for that reason -- while this searched for the single spelling <sourceDir>. A POM writing <sourceDirs><source>gen/kt</source></sourceDirs> lost a root Maven really compiles. Worth recording: a test of mine failed on exactly that shape earlier in this branch and I "fixed" the FIXTURE, concluding the code was right. The fixture was valid Maven and the code was wrong. Reading the other implementation would have settled it in a minute. The container is matched attribute-tolerantly, because <sourceDirs combine.children="append"> is the element and matching <sourceDirs> literally missed exactly the execution that asks for both lists. And a production root is no longer judged by its path. Maven compiles src/integrationTest/java as MAIN code when an add-source execution says so; guessing test scope from a segment dropped that root and the annotated main class in it. Test roots are excluded by the element and the goal that declare them -- <testSourceDirectory>, add-test-source, test-compile -- which is where the distinction actually lives. Each half fails its own test when reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
LanguageTool reported 66 matches and failed the guide build. 65 were EN_ELLIPSIS and one was DOUBLE_PUNCTUATION, and none of them were in anything this branch touched -- the same failure is on feat/macos-appkit-port, which is what said it was not mine before I started editing prose. EN_ELLIPSIS is a toolchain artifact. Asciidoctor renders `...` as an ellipsis CHARACTER followed by a zero-width space and the rule wants whitespace before it, so there is nothing an author can write differently: the source says "..." and the renderer decides the rest. It joins the curated disabled set beside COMMA_PARENTHESIS_WHITESPACE, which is there for the same reason. DOUBLE_PUNCTUATION was real prose, so it is fixed rather than silenced: an inline image sat between two sentence-ending periods and rendered as "checkbox. . This". The sentence now ends after the image. The "choose from of archetypes" typo in the same sentence goes with it. Worth flagging separately: the gate downloads LanguageTool-latest-snapshot, so a rule added upstream fails every branch on the day it lands, with no change on our side. Pinning it is a separate decision. Verified locally: both rendered files report 0 matches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b422972087
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
@target allowed TYPE, so `@Hint(name = "ios.pods")` written straight onto an application class compiled, set nothing and reported nothing -- the processor only reads the group annotations. That is the exact silence these annotations exist to remove, reproduced by the annotation that describes them. ANNOTATION_TYPE covers both real placements, because a build hint annotation and its members are both annotation declarations, and javac now refuses the third. The class javadoc still pointed at [#def], which no longer exists; it now points at HintUnset, which is what the default clause actually says. Reverting the target fails the new test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reading the annotation package off the compile classpath made process-annotations depend on that classpath being resolved, and the goal never asked for it: getCompileClasspathElements() threw, the helper returned an empty list, the package was not found, and every annotated hint was skipped. The manifest was then absent and CN1BuildMojo refused the build -- blaming the project's POM for a goal that had in fact run. The old generated table needed no classpath, so nothing about the previous arrangement made this visible. It shipped for one commit and Car Android Auto caught it. requiresDependencyResolution = COMPILE is the fix. The catch is loud now rather than debug: an unresolved classpath means no hints reach the build, which is precisely the silence this feature exists to remove, and it should never be inferred from a quiet log. The test reads the scope out of the BYTECODE, because @mojo has CLASS retention and getAnnotation() returns null for it -- the same fact that makes the build hint annotations unreadable by reflection, which is why the first version of this test passed vacuously against a null annotation. Verified end to end: maven/integration-tests/build-hint-annotations-test.sh generates a project, annotates it, and confirms the hints reach the build request. That script would have caught this before the push; unit tests that construct a ProcessorContext directly never could. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78093c7d31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+2933
to
+2935
| com.codename1.maven.annotations.AnnotatedClass cls = readClass(element, expectedMain); | ||
| if (cls == null) { | ||
| return null; |
There was a problem hiding this comment.
When the current output contains the annotated main class but no manifest because process-annotations is now unbound or skipped, a stale manifest in another classpath element with the same main-class stamp reaches this branch without a colocated class and is treated as current. The merge then applies its obsolete hints and returns, suppressing the intended “processor never ran” failure. Fresh evidence after the prior colocation-order fix is the case where no authoritative manifest exists: retain support for separately packaged resources by comparing this manifest's source digest with the current main class found elsewhere on the classpath, rather than accepting an unverifiable digest.
Useful? React with 👍 / 👎.
Comment on lines
+987
to
+992
| if (recorded != null && recorded.length() > 0) { | ||
| String actual = found.jar != null | ||
| ? digestOfJarEntry(found.jar, entry) | ||
| : digestOfFile(classFileBeside(found.file, main)); | ||
| if (actual != null) { | ||
| return recorded.equals(actual) ? null |
There was a problem hiding this comment.
When the simulator classpath contains the current main class in one element and only a stale same-app manifest in a resource-only element, actual is null because this check looks beside the manifest, and the timestamp fallback is equally unable to compare anything; findAnnotationManifest() consequently returns the manifest as the apart fallback and publishes obsolete annotation hints. Fresh evidence after the earlier preference-for-colocation fix is the no-current-manifest case: compare the recorded class digest with the expected main class found elsewhere on the simulator classpath, which also preserves legitimate layouts that package resources separately.
Useful? React with 👍 / 👎.
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
1 participant
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.
The problem
A build hint is a
codename1.arg.<name>=<value>line that reaches a builder asrequest.getArg(name, default). Nothing checked the name, so a misspelling was accepted, copied into the build request, never read, and silently discarded — a green build with the setting simply not applied.Our own agent reference had been shipping keys in exactly that state:
skill/references/build-hints.mdandroid.xPermissionsandroid.xpermissions(AndroidGradleBuilder.java:1206)android.minSdkVersionandroid.min_sdk_versionandroid.sdkVersionandroid.targetSDKVersionis the real one)The change
Most hints can now be written as annotations on the application's main class, where javac does the checking: a misspelled name is an unknown symbol, a wrong value type is a type error, and a value outside a hint's supported set is an unknown enum constant.
The builders are untouched.
BuildHintAnnotationProcessorconverts the annotations back into the same key/value pairs, andCN1BuildMojomerges them before the command-line overlay, the CN1Lib merges and both preflights — so a library still appends onto an annotation-supplied value and-Dstill wins.Simulatorpublishes them as system properties at startup socn1:runsees hints that no longer live in the properties file.The properties file is untouched too. It stays the way to set the long tail and the open-ended families such as
android.permission.<NAME>that an annotation cannot express, with no new warnings or errors on that path. Declaring one hint both ways is a build error.Scope: generated projects are deliberately not migrated here
Every project the archetype and the initializr produce is pinned to a released Codename One version — the initializr hardcodes
7.0.267inGeneratorModel.CN1_PLUGIN_VERSION— and no released core carriescom.codename1.annotations.buildhints. A generated project would import annotations that do not resolve and fail to compile before the user has written a line.So the archetype's
__mainName__.javaandcodenameone_settings.properties, and the initializr'scommon.zipand four source archives, are unchanged. They move to annotations in a follow-up once a release containing the package is out.scripts/skindesigner(7.0.255) stays on properties for the same reason, andcn1:migrate-build-hintsrefuses any project whose core lacks the package.The in-repo tool projects that build against 8.0-SNAPSHOT from source are migrated:
gamebuilder,video-builder,cn1playground,certificatewizard,guibuilder,fidelity-app,purchase-test-app,settings,hellocodenameone,protocol-e2e.docs/demosis deliberately excluded: it is the developer guide's snippet project, full of intentionally incomplete fragments, and running the annotation processors over it fails by design.One catalog, generated views
The hint set was described in five places that had drifted apart: a prose table in the developer guide, a runtime scraper of that table in the Settings tool that guessed each type by string-matching the description, a fifteen-entry schema in the simulator, a fourteen-entry separator map in the plugin, and a hand-written agent reference. Only 147 of ~520 names appeared in more than one.
maven/build-hint-catalogis now the single source of truth — 529 hints: 457 mined from the builders, 56 documented-but-unread (external), 16 dynamic families, 82 exposed as annotation attributes across 7 annotations and 10 enums. The annotations, the binding table the processor reads back, the developer guide's table and the simulator's editor schema are all generated from it.The guide's table goes from 208 rows to 529 with no prose lost, and gains Type, Default and Annotation columns it never had.
Enums are emitted only where the accepted set is demonstrable from the code that reads the hint —
HardeningPreflightrejects an unknownharden.level,IOSDependencyManagerthrows on an unknownios.dependencyManager, andGenerateDesktopAppWrapperMojosilently falls back tonativeon an unknowndesktop.titleBar, which is precisely the failure this removes.Settings tool
It no longer scrapes the guide's AsciiDoc and guesses types; it reads the catalog. It also validates closed value domains, and refuses to edit a hint an annotation already owns — reading
META-INF/codenameone/build-hints.propertiesand showing "Set by@Ios(themeMode)on the main class" — because writing a property for such a hint would create the duplicate declaration that fails the next build.Gates
scripts/check-build-hint-catalog.shfails when code reads a hint the catalog does not describe, and when our own docs or project templates name one that no builder reads. Its baseline is empty, so it is a hard gate rather than a ratchet.scripts/gen-build-hint-annotations.sh --checkfails on generated-file drift.Object/Annotationmethod-name rule, enum domains, separators matching whatLibraryHintMergerused to define).Both scripts run in the Java 8 leg of PR CI.
Verification
maven/integration-tests/build-hint-annotations-test.shgenerates a project from the archetype, adds the annotations by hand (the template no longer ships them), and checks the emitted wire values — a list joined with the hint's own separator, an enum written as the catalog's value rather than the constant name, and an attribute nobody set written not at all — that they reach the real build request, and that declaring one hint twice fails with a message naming both the hint and the annotation attribute.Also confirmed empirically, since the whole "unset means absent" design rests on it: javac records a member the developer wrote even when its value equals the annotation's default, and omits one left unwritten.
Notes for review
com.codename1.annotations.buildhints, not...annotations.build:.gitignorecarries a repo-wide**/build/*rule that silently swallowed every source under thecom.codename1.build.sharedpackage path. The rule now un-ignoresbuildwhen it is a Java package rather than a build output directory.android.permission.,harden., …) and are modelled as dynamic families rather than settable keys.Pre-existing problems surfaced but not fixed here
cn1:testbuild hints are broken today.RunTestsMojo.java:253sets the forked runner's CWD totarget/cn1-reports, where neither ofloadCodenameOneSettings()'s fallbacks looks, sobuildHint()returns null there for properties-file hints too.<skipTests>true</skipTests>), so one had rotted against aGridLayout(3, 2)that became(5, 2)in d15cfa5. The skip is now a property (default unchanged) and the assertion tracks the column count.GeneratedAccess_*files are stale against the current core; building it regenerates ~3,300 lines. Left alone to keep this change scoped.Follow-up
Once a release carrying
com.codename1.annotations.buildhintsis published:__mainName__.javaand settings template to annotations.common.zipand the four source archives, bumpGeneratorModel.CN1_PLUGIN_VERSION, and bindprocess-annotationsin the generated common POM.scripts/initializrandscripts/skindesignerthemselves.🤖 Generated with Claude Code