Uh oh!
There was an error while loading. Please reload this page.
Make the database portable and encryptable (#3848) - #5526
Conversation
The database API was five unrelated implementations sharing an interface. Cursors counted from zero on some ports and one on others, iOS reported success on an empty result set and returned null for every blob, the simulator could not seek at all, and no port could encrypt anything. This lands the port-independent half: - package-info.java now carries the normative contract every port must satisfy: zero-based positions, first() lands on a row, execute() runs a whole script while the parameterized forms take exactly one statement, typed parameter binding, flat transactions, IOException with a chained cause, idempotent close. - AbstractDBCursor derives all navigation from two primitives, rewind() and stepForward(), so every port gets identical semantics rather than each reimplementing them. Seeks rewind and re-step, which is what Android's windowed cursor already does on a window miss; buffering rows instead would mean materializing every column of every row stepped past. - SQLStatementSplitter splits a script the way SQLite does, respecting string literals, quoted identifiers, comments and CREATE TRIGGER bodies. - DatabaseConfig, DatabaseEncryptionException and ManagedKeys add keyed opens. Managed keys are resolved in the core so every platform derives identical material from an alias, and a key that cannot be stored is fatal rather than a silent downgrade to plaintext. - db.legacy restores each platform's previous behaviour for the ten changes that alter a previously successful result. It is read lazily, because the generated stubs set it after Display.init. Blob parameters now raise IOException rather than RuntimeException, and the truncated javadoc samples in Database, Cursor and Row are replaced with complete ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The simulator was the weakest database implementation, which mattered more than it sounds: it is where people develop. Its cursor could not seek at all, because the JDBC driver only produces TYPE_FORWARD_ONLY result sets and first(), last(), prev() and position() each threw outright. execute() silently ran the first statement of a script and discarded the rest. rollbackTransaction() left the connection outside autocommit, so every following statement quietly joined a new implicit transaction. Every query leaked its PreparedStatement. - SECursor now extends AbstractDBCursor, rewinding by re-executing the statement. The simulator has working random access for the first time. - execute(String) splits the script and runs each statement, rather than trusting a driver to decide how much of it to run. - The parameterized forms reject a multi-statement script instead of dropping its tail. - Statements are closed on the success path, cursors are closed with the database, close() is idempotent and rollback restores autocommit. - getColumnName reports the result set label, matching getColumnIndex, so an aliased column can be found under the name it was found by. The shaded driver moves from org.xerial to io.github.willena, which is the same driver with SQLite3MC compiled in: same package, same config, verified identical on plaintext databases, plus the SQLCipher-compatible cipher the simulator needs to open a database written on a device. getV4Defaults() is required over getDefault() - the latter selects SQLite3MC's own variant, which real SQLCipher cannot read. That driver also stops being frozen. Freezing assumed the shaded content never changed; it now carries a crypto-bearing engine that has to track upstream security releases. SEDatabaseConformanceTest runs the portable contract against the real SEDatabase headlessly in about two seconds, including both the strict and legacy modes and the encrypt/decrypt round trip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
iOS was the port the "radically different implementations" complaint is
really about, and it had real bugs behind the divergence:
- sqlDbClose called sqlite3_free on the connection handle. That never
closed it, leaked the file descriptor, skipped the WAL checkpoint and
handed the pointer to the wrong allocator. Now sqlite3_close_v2.
- sqlCursorValueAtColumnBlob was { return nil; }, so iOS could not read
a blob at all, in either direction.
- Opening a database called sqlite3_config(SQLITE_CONFIG_SERIALIZED) and,
on failure, sqlite3_shutdown(). That has to run before
sqlite3_initialize() to do anything, and calling shutdown with
connections open is undefined behaviour. Replaced with per-connection
SQLITE_OPEN_FULLMUTEX.
Behaviour now matches the portable contract:
- CursorImpl extends AbstractDBCursor, so last(), prev() and position()
work instead of throwing "Unsupported", and first() lands on a row and
reports false for an empty result set rather than reporting success and
leaving the statement unpositioned.
- Parameters bind by runtime type through new statement natives. They
used to be stringified, which stored an Integer as TEXT, and a comment
conceded it "will probably fail with blobs".
- Parameter count mismatches and multi-statement scripts in the
parameterized forms are rejected rather than silently mis-executed.
- Errors carry sqlite3_errmsg unconditionally; the dead XMLVM branches
that gated error reporting are gone.
- finalize() is removed from the database and cursor. Closing sqlite
handles from the GC thread is the "platform specific nuance" that
defeated ThreadSafeDatabase.
- Custom file:// database paths work, matching Android and the simulator.
Keying is a separate native that reports success rather than throwing, so
the Java side can tell a wrong key from a failure to open the file
without the native layer naming a core exception class.
isDatabaseEncryptionSupported() asks the linked engine via PRAGMA
cipher_version rather than assuming, so it reports honestly on a build
that does not bundle a cipher-capable SQLite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Android was already the most capable port, so this is mostly tightening rather than rebuilding: - A null element in a String[] now binds SQL NULL. bindString rejects null, so passing one used to fail the whole statement. - execute(sql, (Object[]) null) no longer dereferences a null array. - execute(String) runs a whole script. execSQL refuses anything after the first statement, so the script is split and run statement by statement. - executeQuery forces the window fill before returning, so malformed SQL is reported there rather than from the first next(). rawQuery is lazy. - Transactions use the shared flat-transaction guards, so a nested begin is rejected here as it already was everywhere else. - Exceptions carry their cause and are no longer printStackTrace'd on the way out. - Cursors are invalidated when the database closes, close() is idempotent, getRow() off a row throws, getColumnIndex is case insensitive, and wasNull() is false before any value has been read. - Blob query parameters work, bound through a cursor factory, which is the only supported route: rawQuery can carry text arguments only. This is what androidx.sqlite does for the same reason. Encryption lives in a new com/codename1/impl/android/cipher package built on net.zetetic:sqlcipher-android. It compiles against classes that are only on the classpath of app builds that use encryption, so it is excluded from the port's own javac and reached purely by reflection, letting the builder delete it for every app that never touches DatabaseConfig. That gating is why the package is a near copy of AndroidDB rather than a shared supertype: any shared type naming net.zetetic would have to live in the part of the port that must stay deletable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both ports inherited the base openOrCreateDB, which returns null, so Database.openOrCreate() handed back null and calling code failed with a NullPointerException. They now have a full implementation that satisfies the same contract as every other port, encryption included. Neither runs a JVM, so JDBC was never an option; they needed a C binding. That is cheap because both are ParparVM C targets whose CMake project already compiles every .c in the source root. - The engine is SQLite3 Multiple Ciphers, bundled once in the translator and emitted only for applications that use com.codename1.db. iOS shares the same copy, so those three targets run one engine at one version, and the simulator's JDBC driver is built from the same upstream project. - The amalgamation is named .h deliberately. The iOS project generator lists .h but excludes it from the compile phase; CMake globs *.c for sources; and the ParparVM native symbol scanner reads only .c and .m. Named .c it would be compiled twice without its build options, named .inc it would ship inside the .ipa as 13MB of dead weight. - cn1_sqlite3.c is the single translation unit that compiles it, with the build options set immediately before the include so they cannot leak into unrelated sources. It is gated internally, so an emitted but disabled build produces an empty object rather than a link error. - The binding itself is shared. Both ports need identical code but mangle their entry points from different Java classes, so the logic lives once in cn1_db_sqlite_impl.h and each port's .c expands CN1_DB_DEFINE_NATIVES for its own prefix. Verified that every declared native has both its plain and its _R_ symbol in both ports. - iOS stops linking the system libsqlite3 when the bundled engine is used, rather than carrying two SQLite implementations in one process. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The JavaScript port sat on WebSQL, which Chrome removed in 119 and Firefox never implemented, so its database was dead on every current browser. What it did support was thin: transactions were printlns, getBlob threw, position(n) always returned the first row, close() did nothing, and the bridge busy-waited a CN1 thread on a lock. It now runs the same SQLite build the other ports use, compiled to WebAssembly, inside the application's own worker. Every call after the first is an ordinary synchronous call; only the initial load suspends, through the runtime's existing yield-on-promise support, so the lock and its 200ms poll are gone. Storage uses the opfs-sahpool VFS rather than the default OPFS one. The default needs crossOriginIsolated, which needs COOP/COEP response headers, which we cannot require of the arbitrary static hosting these bundles are deployed to. Browsers without synchronous OPFS access fall back to memory with a console warning, because silently losing every write on reload is not a failure anyone should discover in production. Gating, so nobody pays for what they do not use: - iOS emits the bundled engine, and drops the system libsqlite3, only for applications that reference DatabaseConfig. Everyone else keeps the system SQLite exactly as before. - Windows and Linux emit it for anything referencing com.codename1.db, since they have no system SQLite at all, and its cipher only when encryption is configured. - Android's SQLCipher package is deleted unless DatabaseConfig is referenced, and the AAR arrives through a new PlatformFeatureCatalog entry keyed on that same class. - The JavaScript builder prunes the 1.5MB engine from bundles that never open a database. The catalog entry is keyed on DatabaseConfig rather than the db package on purpose, and two new tests hold that line: every database application references com.codename1.db, so keying it there would bundle SQLCipher for all of them and push the minimum Android SDK from 19 to 23 for people who never asked for encryption. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The contract and the encryption are only real if they are checked, and the portability claim in particular is the kind that fails silently: a cipher misconfiguration produces files each platform reads perfectly well on its own and nothing else can touch. - Seven device tests run the shared conformance suite on every port through the existing screenshot harness. They are assertion only, so they take no screenshots and sit before the ordering-sensitive graphics baselines. Ports without a database self-skip, so a port turns green on its own once it has one. - Two of the seven run in legacy mode, which is what makes the compatibility promise testable rather than aspirational: they fail the moment a refactor changes what db.legacy restores. - Two Port Status features expose the results publicly, split so a threading regression cannot blank the whole database row. - scripts/ci/db-cipher-interop.sh checks our encrypted files against the stock sqlcipher client in both directions, with a raw key to isolate the cipher configuration and a passphrase leg to cover the key derivation. Wired into the pull request workflow. The developer guide's SQL section said the iOS SQLite "isn't threadsafe" and warned that the garbage collector closing a connection would crash the app. That was true, and this branch is what fixes it, so the section is rewritten and extended with encryption, key management, threading, cursor cost and the legacy compatibility table. ThreadSafeDatabase is un-deprecated. Its note blamed platform nuances; the nuance was the iOS finalizers, now gone. Its close() was fire and forget, so it returned before the database was closed and a following delete() raced it, which is fixed here too. The cursor inner classes are static: with an explicit owner field the implicit outer reference was dead weight, which SpotBugs flagged on iOS and would eventually have flagged everywhere. 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:ce77b834d3
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
shai-almog
commented
Aug 6, 2026
Companion PR with the build-side gating: codenameone/BuildDaemon#172 |
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
- The Ant build for the JavaSE port links whichever sqlite-jdbc is pinned in cn1-binaries, which has no org.sqlite.mc, so importing the driver's config builder broke that build for everyone. JavaSEPort now writes the SQLCipher connection properties out literally, which needs no extra class at compile time, and reports isDatabaseEncryptionSupported() by probing for the cipher-capable driver rather than assuming it. The simulator therefore answers honestly under either build. - The Windows cross-compile failed to link. The sample application now uses com.codename1.db, but that integration test drives the translator directly rather than through the builder, so the engine was never emitted and the natives had no definitions. Two fixes: the shared binding header is always emitted and defines every entry point either way, as real bindings or as stubs that raise a clear IOException, so an application always links however the translator was invoked; and the integration tests ask for the engine explicitly, so those ports actually exercise the database instead of only ever self-skipping. Verified that both branches of the header export an identical symbol set. - The developer guide requires snippets to live in docs/demos and be included by tag. Migrated with the repository's own migration script. The snippet harness had no com.codename1.db import, which is why all three failed to compile once moved; added, since it is a core package the guide documents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cloudflare Preview
|
The Maven build already excluded it, but the Ant target compiles every source in the port, so it tried to build the package against net.zetetic and failed for anyone building that way -- including BuildDaemon CI, which clones this repo and runs the Ant target. Mirrors the exclusion into both places the ARCore and AI packages already use: the javac in Ports/Android/build.xml and the excludes property in nbproject/project.properties. 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:d595bd94da
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Compared 12 screenshots: 12 matched. |
Compared 149 screenshots: 149 matched. Benchmark ResultsDetailed Performance Metrics
|
Compared 149 screenshots: 149 matched. Benchmark ResultsDetailed Performance Metrics
|
Compared 149 screenshots: 149 matched. Benchmark ResultsDetailed Performance Metrics
|
Review findings, all eight real: - Database.encrypt() could never work on Android. The system SQLite has no cipher, so a plaintext database opened through it can never be re-keyed. Added openOrCreateDBForRekey(), which Android routes through SQLCipher (an empty key opens an unencrypted file, which can then be re-keyed). - A managed key resolves its keystore alias from the database name, and every port passed null when re-keying, so changeKey(managed()) raised a NullPointerException instead of encrypting. Each Database now retains the name it was opened under. - Two threads first-opening the same managed database could each see nothing stored, generate different keys and overwrite each other, leaving one of them holding data nobody could ever read. The read-generate-store sequence is now serialized. - isKeyHardwareBacked() inferred hardware backing from the API level, but emulators and plenty of real devices back AndroidKeyStore keys in software. It now asks the key itself, via KeyInfo. Applications are told they may use this to refuse to store sensitive data, so it has to be true. - checkEndTransaction() cleared the flag before the engine had ended the transaction, so a failed commit left the transaction open while the API believed it was closed, and the recovering rollback was rejected. Splitting out markTransactionEnded() means the flag drops only on success. A conformance check covers the failed-commit path. - An encrypted Android database opened by file:// URL had no toNativePath() conversion, so java.io.File treated the URL as a literal relative name. - Calling next() past the end repeatedly re-derived the row count each time, inflating it, after which last() would seek to a row that does not exist. Verified the new check fails against the old code (5 became 8). - PRAGMA rekey interpolated the key directly, so a passphrase containing a quote produced a different statement. Both Android and the simulator now go through one helper that quotes text and passes a raw key literal through untouched. CI failures: - Six SpotBugs findings in core-unittests, a module the earlier local runs had not covered: boxed constructors, a default-encoding String, and a Boolean-returning method that could return null. - The arm64 Linux and Windows cross-builds failed compiling the engine's ARM AES intrinsics. Where the compiler defines __ARM_FEATURE_CRYPTO the engine uses them directly, which is what Apple's toolchain does, so iOS is unaffected; otherwise it tags individual functions with __attribute__((target)), which the cross-compiling clang does not honour for these intrinsics. Rather than require ARM crypto extensions of every chip, that path now uses the software implementation. - DatabaseStatementLegacyTest failed on Android because the legacy expectation was wrong, not the code: only iOS ran a whole script before this branch, through sqlite3_exec. Android's execSQL and the simulator's PreparedStatement both dropped everything after the first statement. Corrected in the suite and in both places it is documented. - The migrated guide snippet fixture needed a copyright header. 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:7f2f2c70ff
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Compared 217 screenshots: 217 matched. |
The fifth job in a day to die before touching our code, this time on "Plugin maven-install-plugin:2.5.2 or one of its dependencies could not be resolved". Same wrapper and the same resolution-only pattern as the others, so a failure in what this actually builds still fails on the first attempt. 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:f33c50e9d8
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…tinct Two ways one application's managed key could still be lost. synchronized covers threads in one VM and nothing between two, and an application can run in more than one process -- Android components declared with their own android:process, or two runs of a desktop build. Both could find nothing stored, generate different keys and each overwrite the other, leaving the database encrypted with a key that no longer exists. SecureStorage.setIfAbsent is the operation that was missing: it stores only when there is nothing there and answers with what the store ended up holding, so a caller that lost the race takes the winner's key instead of overwriting it, and both open the database with the same one. iOS implements it through SecItemAdd, which refuses a duplicate inside the keychain daemon and so is atomic between processes; the default is the best a store without that can do, and says so -- the check and the write are still two steps. The namespace sanitizer folded every character it could not carry onto "_", which is not reversible: com.acme.foo$bar and com.acme.foo_bar became one namespace, as did "My App" and "My_App", so two applications shared the store the namespace exists to keep apart. It now escapes those characters the way ManagedKeys.accountName escapes the account half of the same name, the escape character included. The keychain add is verified by building: the port jar rebuilt, the project regenerated from it, the symbol present in the staged IOSNative.m and xcodebuild reporting BUILD SUCCEEDED. Its second symbol also corrected -- I had spelled it _R_int_R_int, which resolves to nothing; the alias this file uses carries no return suffix, and the same mistake in secureStorageEntryStatePlain is fixed with it. 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:6cf81644f8
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
…cryptable-database # Conflicts: # docs/developer-guide/languagetool-accept.txt
Reading the store, generating a key and writing it back is three operations, and the review is right that reading again afterwards closes nothing: two processes can each complete all four steps and each believe its own key is the one stored. Only iOS was safe, because SecItemAdd refuses a duplicate inside the keychain daemon. The other four now have a gate of their own. Windows and Linux gained fileCreateExclusive, which is CREATE_NEW and O_CREAT|O_EXCL -- decided by the filesystem, so it cannot be won twice. The caller that creates the gate stores the value; the caller that finds it waits briefly for that value to land and never writes. The simulator takes a file lock, which is the mutual exclusion the JVM offers between processes rather than between threads, and re-reads Preferences inside it: that store caches per process, so without the sync the check would consult a copy taken before the other process wrote. Android creates the gate with createNewFile(), which is atomic, but its store is SharedPreferences and there is no way to make one process see another's write -- the cache is per process and MODE_MULTI_PROCESS was withdrawn for being unreliable. So the caller that loses reports nothing rather than writing, which turns a permanent silent corruption into a transient failure: KEY_UNAVAILABLE now, and the next launch reads the file fresh and finds the key. Also merges master, whose only conflict was two additions to the LanguageTool accept list, both kept. 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:a3d927401a
ℹ️ 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".
| java.io.File dir = new java.io.File(AndroidNativeUtil.getActivity() | ||
| .getApplicationContext().getFilesDir(), "cn1securestorage"); |
There was a problem hiding this comment.
Use the application context for the creation gate
When a managed database is first opened concurrently by components in separate android:process services before any Activity exists, getActivity() returns null, causing gateFile() to return null and setIfAbsent() to fall back to the non-atomic superclass implementation. Both processes can then generate and persist different keys through their independently cached SharedPreferences, potentially leaving the database encrypted with the key that lost the race. Build the gate path from AndroidNativeUtil.getContext().getApplicationContext() instead, as the storage code below already does for background-service support.
Useful? React with 👍 / 👎.
| } | ||
| boolean created; | ||
| try { | ||
| created = gate.createNewFile(); |
There was a problem hiding this comment.
Clear creation gates when entries are removed
After the first successful setIfAbsent(), this gate file remains permanently. If Database.forgetManagedKey() later removes the corresponding preference, reopening a newly created managed database under that alias finds no value but cannot recreate the gate; the false createNewFile() result leads to a null return and KEY_UNAVAILABLE on every subsequent attempt until app data is manually cleared. Successful removal of an account must also retire its gate, with synchronization that preserves the cross-process exclusion.
Useful? React with 👍 / 👎.
Three jobs on this branch timed out overnight and none of them reached any of our code: vm-tests spent its ninety minutes in "Install native build tools", and the Windows cross-compile and the website build each ran to GitHub's six hour ceiling in their own apt steps. All three had gone through scripts/ci/apt-get-update.sh a moment earlier, at 03:01 to 03:05 UTC, with the azure mirror answering Ign: on every index. Two things were missing. apt had no timeout, so a mirror that accepts the connection and then stalls is waited on forever -- and Acquire::Retries never comes into play, because nothing ever fails. And the settings were passed as options to apt-get update, so the apt-get install that follows in every caller inherited none of them. Both are fixed in one place: the script now drops the timeouts, retries and IPv4 preference into /etc/apt/apt.conf.d, which every later apt call in the job picks up, and runs the update itself under a five minute ceiling with three attempts. The two jobs that ran for six hours also had no timeout-minutes of their own, which is why a hang cost that much; they now have one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
These three do not go through scripts/ci/apt-get-update.sh -- they run as root in the CI container, without sudo -- so the timeouts that script installs never reach them. A stalled mirror there is still a hang rather than a failure, which is what cost three jobs their whole run overnight. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
It arrived with the master merge without one, and the copyright gate is diff scoped: merging master pulled the file into this pull request's scope, where it failed. Nothing else about the file changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…test that never ran I added the seven database tests to the manifest and wrote "not-run" beside them in all eleven port reports, which is a statement that they were published and never executed. They do run: every port on this branch reports all seven passing, so the reports are replaced with the real thing -- run 32242868198 and its siblings on this head, not-run 0 across android, both iOS renderers, both Linux architectures, JavaScript, mac-native, tvOS, watchOS and both Windows architectures. The reason a hand-written absence survived is that nothing objected to it. A registered test sitting at "not-run" renders on the page exactly like one that runs and passes, so the contract now rejects it: a port that genuinely cannot do something reports "skip" from the suite itself, which is evidence, while "not-run" is the absence of evidence and the answer to it is to run the suite and check the report in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
VectorMapShapes failed on tvOS with a 4K frame whose basemap covered part of the viewport and whose remainder was the background colour -- 36% of the pixels, two and a half million of them past a quarter of the range, so not sampling noise. The cap never fired: no CN1SS:WARN, and the two sibling map tests matched, so the wait believed the map was rendered. isMapReady() derives the visible tile set from the component's current width and height, which means a run of "ready" answers is only worth anything if every one of them was asked about the same viewport. A layout pass that enlarges the map after the count reaches two leaves the tiles for the new area unrequested and unrendered, and the capture takes the frame in between. The file already carries the sibling of this hazard -- a first ready before the host's final layout pass, which resets the pixel ratio and clears the rendered cache -- and mitigates it with a minimum settle; this is the same fault line at the other end. The poll now resets its counter whenever the map's size differs from the size the last answer was given for, so two consecutive readies mean two readies for the viewport that will be captured. 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:81f14b2311
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…a gate Both faults are in the gate I added, and both end the same way: an alias that can never be used again, reporting KEY_UNAVAILABLE long after its database is gone. remove() cleared the entry and left the gate. The next create then found no value of its own and a gate it could not take, so it reported nothing -- for good, since nothing removes that file. Android, Linux and Windows all now delete the gate as part of the removal, after the entry rather than before: a gate dropped first would let a second caller create a key while the old value was still in place. The gate was also named from account.hashCode(), and a hash is not a name: "Aa" and "BB" hash alike, so two aliases shared one file and whichever asked second could never create its key. The name is now derived from the account through the same reversible escape the namespace uses, in one place all four ports call -- the simulator included, where the file is a lock rather than a gate and a collision only costs a wait, but there is no reason for it to be the one place that hashes. Also merges master, and adds the header to the file it brought in without one, which is what the copyright gate was failing on. 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:c483d140f3
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…e it Two ways the gate I added could strand a managed key, and the capture race the JavaScript suite hit on the way past. A gate that is a file's existence outlives the process that made it. A run that died between creating the gate and storing the value left an alias that could never be created again: every later caller read that file as a live writer, waited for a value that was never coming, and reported KEY_UNAVAILABLE. The gate is now a lock -- flock on Linux, an unshared handle on Windows, FileChannel on Android -- which the operating system releases when the process ends however it ends. There is nothing left behind to recover, which is also why remove() no longer deletes that file: it gates nothing by existing, and deleting it under a process that holds the lock would let a second one lock a different file. The migration took the shared entry rather than copying it. Applications that shared an account name under one OS user all depend on that entry, and the first one to upgrade removed it -- so a later one saw nothing, generated a replacement and could no longer read its own database. Adoption now copies and leaves the source for whoever else still needs it, on the desktop ports and in the simulator alike, and marks itself adopted so this application stops consulting it: without that mark a forgotten key would come straight back from the entry it was copied from. Separately, graphics-draw-image-rect was captured with the top half of its grid drawn and the bottom half blank. That test already asks for a longer wait before it is declared ready, but the capture that follows asked for a fixed short one, and a test that draws in stages can be still for three frames between two of them. Both waits now come from one table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DurankGts
commented
Aug 21, 2026
My App now is failing getRow() in data base. In ios is frezzed. yesterday all it was ok....what do you update that cause this problem now? How can I used last code in your build server. I need to launch and app today to production an now is faling this the error >Error:java.io.IOException: The cursor is not on a row. Call next(), first(), last() or position(int) and check that it returned true before calling getRow() public synchronized Service getService(int id) throws IOException { |
DurankGts
commented
Aug 21, 2026
this issue fail in ios an android in this moment. |
DurankGts
commented
Aug 21, 2026
I have with this code very years. I fix now an I will check in all my code where I do this bad, but you must to reconize that your code permitted call getRow first after this The cursor is not on a row. Call next(), first(), last() or position(int) and check that it returned true before calling getRow() |
shai-almog
commented
Aug 21, 2026
Set |
`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>
* Check build hints at compile time instead of shipping them inert
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>
* Commit the catalog sources that .gitignore was swallowing
`.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>
* Stop the bootstrap doing its work at import time
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>
* Make the generated docs and sources survive the ASCII and prose gates
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>
* Give ThreadSafeDatabaseTest headroom under the FormTest timeout
`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>
* Refuse to migrate a project that never runs process-annotations
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>
* Do not migrate the guide's snippet project, and keep Settings from duplicating 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>
* Defer the generated-project templates to a follow-up
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>
* Annotate inside the integration test, not in the archetype
The test asserted the generated project already imports the build hint
annotations, which was true only while the archetype template carried them. Now
that generated projects stay property-backed until a release ships the package,
the test adds the annotations to the generated main class itself.
It annotates only hints the template does not declare -- ios.pods, ios.teamId,
desktop.width, android.installLocation -- because setting one in both places is
a build error, which the last section of the test covers deliberately. It also
now asserts the reverse: a hint the properties file declares must not appear in
the emitted resource, so the two sources stay separate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Four migration and Settings defects from review
Kotlin string interpolation. `quote()` escaped nothing for `$`, so migrating a
Kotlin main class turned any hint value containing one into an interpolated
string -- an `android.gradleDep` of `implementation 'x:y:$version'` either fails
to compile as an unresolved reference or silently resolves to something else.
The target language is threaded into the quoting and `\$` is emitted for Kotlin
only, since Java has no such construct.
Imports in a default-package source. With no package declaration and no existing
import, `head.indexOf("package ")` returned -1 and the arithmetic put the import
at the first newline in the file -- inside the copyright comment. The project
was then left with unresolved annotations and its properties entries already
deleted. The class declaration is the anchor in that case.
Aliases in the Settings tool. Ownership was looked up by exact name, so with
`@Android(themeMode = ...)` owning `and.themeMode`, the row for its deprecated
alias `cn1.androidTheme` still offered Add -- creating the second declaration of
one effective setting that the next build refuses through the alias conflict
check. Both sides of the lookup are canonicalised now.
Credential masking. The scraper this catalog replaced inferred SECRET from names
containing password, secret or token, and the Settings field masks on that type.
Classifying them as STRING rendered a stored certificate password as visible
text. All five -- codename1.mac.certificatePassword, macNative.notarize.password,
windows.msix.password, windows.signing.password and facebook.clientToken -- are
SECRET again, with a test that holds every future credential-shaped name to it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Require the process-annotations binding on the module that owns the main class
The guard accepted an execution anywhere in the reactor, but
ProcessAnnotationsMojo scans only the output directory of the module it is bound
to. A binding on a platform or utility module therefore never sees the main
class that common compiles, and the migration would still delete the working
properties and leave annotations nothing ever reads -- the exact failure the
guard was added to prevent, one level in.
It now resolves the module whose base directory is the Codename One project
directory, which is where the main class lives and where findMainClassSource
looks, and requires the binding there. An execution bound to phase `none` is
declared but never runs, so it no longer counts either.
The refusal names that module and says explicitly that binding the goal
elsewhere in the reactor does not help, since that is the mistake being made.
Verified against a real project: gamebuilder passes with the binding on common,
and is refused when it is moved to the javase module.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Require an execution that can actually see compiled classes
Being declared in a real phase was still not enough.
ProcessAnnotationsMojo returns immediately when `skip` is set, and again when
its output directory does not exist -- which is every phase before `compile`.
An execution configured `<skip>true</skip>`, or bound to `generate-sources`,
therefore emits no annotation resource at all, and the migration would delete
the working properties and leave nothing behind.
The guard now requires the execution to be unskipped and bound at or after
`compile`, taking an absent phase as the goal's own default of
`process-classes`. Skip is read from both the execution and the plugin
configuration.
Verified end to end: gamebuilder proceeds normally, and is refused once its
execution carries <skip>true</skip>.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Stop the generated simulator schema duplicating the hand-written one
The group name is part of the property key, so registering harden.level under
both `hardening` and `Hardening` overwrites nothing -- it creates a second
group, and BuildHintEditor renders every group it finds. The user saw duplicate
controls for one setting, for all of harden.*, nativeTheme, ios.themeMode and
and.themeMode. The comment claiming the hand-written entries take precedence
because the setter never overwrites was simply wrong: the two never collided on
a key.
BuildHintSchemaDefaults now records the hints it describes as it registers them,
and the generated companion skips those. Precedence is explicit rather than
assumed, and it cannot drift, since the record is built from the same set()
calls that do the describing.
Verified by walking the registered properties: 89 hints in the editor, none
appearing under more than one group, and harden.level, nativeTheme,
ios.themeMode and and.themeMode all resolving to their hand-written group.
Also makes the integration test fail when the merged settings file is absent.
It was the only assertion that annotation hints reach the build request, and
skipping it on a missing file meant a regression in goal ordering, target
validation or the merge itself would have left the test green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Prove the annotations are processed instead of predicting it
Four rounds of review went into guessing, from the POM, whether
process-annotations would run: it can be bound on the wrong module, bound to a
phase with no compiled classes, skipped outright, or skipped through a property
expression the model text does not resolve. Each fix closed one case and the
next review found another, which is what a static prediction of another mojo's
behaviour is going to keep doing.
The goal now applies the whole migration, runs process-classes over the module
that holds the main class, and checks that every migrated hint came back out of
the emitted resource. If any did not, both files are put back exactly as they
were and the failure says what was missing. Whatever the next way to not-run
turns out to be, the answer is still correct.
Both files have to move together before that check: leaving the properties in
place while the annotations are added is itself the duplicate-declaration case,
so the build would fail for that reason and never say whether processing works.
The first version of this change had that wrong, and the verification caught it.
Verified on a generated project: 7 hints in, 6 migrated and confirmed emitted,
java.version correctly kept. With the binding removed the goal refuses and both
files come back byte-identical.
The Settings tool has the same problem from the other side. It read ownership
only from the emitted resource, so in the window right after a migration -- the
source declares the annotations, no build has run -- every hint looked unowned
and Add was offered for one the annotations already set. It now falls back to
reading the annotations off the main class, matching attribute names at the top
level of each annotation so a comma, bracket or equals sign inside a value
cannot register as one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Round-trip the rollback snapshot, and stop trusting a stale manifest
The rollback snapshot was taken with the UTF-8 read helper and restored with the
ISO-8859-1 writer, so any raw high byte in an unrelated property -- an accented
codename1.displayName, say -- came back changed while the goal reported that
both files were put back exactly as they were. It is snapshotted with the
properties encoding now, and the two helpers are explicit about which encoding
they use rather than one of them being the default.
Verified on a generated project carrying a raw 0xE9: after a failed migration
the settings file is byte-identical and the byte is still there.
The Settings tool consulted the main-class source only when the emitted manifest
was missing. The manifest is a build artifact and goes stale in both directions
-- absent right after a migration, and out of date the moment an attribute is
added to a project that was built earlier -- so a newly annotated hint looked
unowned and Add wrote the duplicate declaration the next build refuses.
The source is read every time now, because it is the only current statement of
what the annotations declare, and the manifest is merged on top for its origins.
The union is the safe direction: over-reporting ownership only withholds an
editor, while under-reporting breaks the build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Preserve source bytes, and stop comments confusing the annotation scanner
The main class was read and written as UTF-8, so a project whose sources use a
different encoding had its whole file reinterpreted while the annotations were
spliced in: a raw byte in a comment or a string literal came back changed even
when the migration succeeded.
Reading project.build.sourceEncoding would only narrow that to projects which
declare it correctly. Instead both ends use ISO-8859-1, which maps every byte
0-255 to the same char, so decode -> splice ASCII -> encode reproduces the
original bytes exactly whatever the real encoding is. The markers this code
looks for -- package, import, the class declaration -- are ASCII, and every
ASCII-compatible encoding decodes those identically under that scheme.
Verified on a generated project whose main class carries three raw 0xE9 bytes in
a comment, making it invalid UTF-8: all three survive the migration and the
annotations are still inserted correctly.
The Settings tool's source scanner skipped strings but not comments, so a
comment carrying an unmatched delimiter -- @Ios(/* required for issue ( */
teamId = "x") -- lost the annotation's boundary and left teamId editable, which
is the case that writes the duplicate declaration. It now skips line comments,
block comments and character literals as well, through one shared helper used by
both the balancer and the attribute scan.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Refuse a build whose annotations were never processed
The P1 is about the feature, not the migration convenience: a mojo's default
phase does not add an execution to a project, so an existing application that
follows the package documentation and adopts the annotations compiles cleanly
and ships with every annotated hint missing. Nothing said so.
CN1BuildMojo now checks, when no annotation manifest was found, whether the
application classes carry build hint annotations at all -- read out of the class
file's annotation table, so it sees what the compiler emitted rather than what
the source appears to say. If they do, the build fails with the execution to
add. Both a directory and a jar are scanned, because a reactor `package` build
hands the dependency module's jar rather than its output directory, which is
exactly the shape this has to work in. The package documentation now states the
requirement too.
Verified on a generated project: with the binding removed and one @Ios on the
main class the build refuses; with the binding restored it applies the hint and
carries on.
Two more from the same review:
- Verification accepted a manifest an earlier build had left behind, so with
processing now skipped or unbound the check passed against a stale file, the
properties were deleted, and the next clean build dropped the hints. The
resource is removed before the nested build, so what is checked is what that
invocation produced.
- The Settings source scan matched only the imported simple name, missing the
equally valid `@com.codename1.annotations.buildhints.Ios(...)`. Both spellings
are matched now, with the name boundary checked so `@Ios` cannot match
`@IosPrivacy`. The boundary test is hand-rolled because
Character.isJavaIdentifierPart is outside the API subset this class compiles
against -- the bytecode compliance gate caught that.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Do not refuse a build over an annotation that sets nothing
Three from the same review.
@Ios() with every member left at its default is legal Java -- it is what is
left after the last attribute is deleted -- and the processor emits the manifest
for it, stamped with the main class but carrying no hint. The merge judged by
the hint count, read that as "the processor never ran", and refused the build
until the annotation itself was deleted. The manifest's presence is what proves
processing happened, so that is what the check now reads; the refusal still
fires for the case it exists for, annotations in the compiled classes with no
manifest anywhere.
The migration goal restored both files when the verification build failed, but
not when the mutation itself did. If the annotations went in and the properties
rewrite then failed -- unwritable file, full disk, a partial write -- the
project was left declaring the same hint twice, which is exactly the state the
next build refuses to compile: worse than not having migrated at all. The
restore is one helper now and runs for either failure. A dangling javadoc left
over from a removed method went with it.
pr.yml ignores scripts/** and re-includes a fixed list, so a PR touching only
the catalog gate, its miner, or its baseline started no workflow at all -- the
gate could be broken, or its empty baseline relaxed, without ever running. The
five files are re-included in both the pull_request and push filters.
The merge test needed the annotated class present alongside the empty manifest
to trip the refusal at all; without it the test passed against the bug it was
written for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Tell a current annotation manifest from last build's
Three from the same review, all cases where something looked applied and was not.
The main-class stamp says which class produced the manifest, not when. Nothing
clears target/classes between builds, so a project that ran process-annotations
once and then stopped -- goal unbound, skipped, moved to a phase that no longer
runs -- keeps a manifest naming the right class while the annotations beside it
change. The merge accepted it, applied the older values, and the guard added for
exactly this never fired.
The processor now records a fingerprint of the annotations it read, taken over
the raw members rather than the hints they convert into so it moves for anything
the developer can change: a different value, an added or removed attribute, a
whole annotation gained or lost. The merge recomputes it from the main class on
the classpath -- directory or jar -- and refuses a manifest that does not match,
naming it as left over from an earlier build. It refuses only on positive
evidence: no main class name, no class file, no recorded fingerprint, or an
unreadable one, and the manifest is taken at face value as before.
A hint set by @Hardening reached the settings only in createAntProject, which
runs after the early hardening pre-flight and after hardeningCacheKey is read
for the Android up-to-date check. The early pass computed "unhardened" from the
properties file while the finished build recorded "hardened:...", so the keys
never matched and an up-to-date APK was rebuilt on every invocation. Worse, an
unsupported hardening request made through an annotation escaped the refusal
that pass exists to perform. Annotation hints are merged there too, before the
-D overlay so a command-line hint still wins.
Properties.load turns € in the settings file into a real euro sign, and
migrate-build-hints writes the source back through ISO-8859-1 to keep the
untouched part byte-identical -- so emitting the character raw wrote '?' for
anything unmappable and a high byte for anything else, corrupting a UTF-8
source. The verification build would not have noticed: it checks that the hint
came back, not what its value was. Non-ASCII is written as \uXXXX, which Java
and Kotlin both accept, and a backslash before one still survives -- Java
recognises a unicode escape only after an even number of backslashes, so there
is a test pinning that rather than leaving it to luck.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Give the same answer whether or not target/classes was cleaned
The fingerprint added last round covers the annotations and nothing else, so
editing codenameone_settings.properties cannot invalidate it. With processing
skipped or unbound, a line added for a hint an annotation already sets left a
manifest that still matched -- and the merge quietly replaced the value the
developer had just written. The next clean build regenerated the manifest, the
processor saw both declarations, and the build failed. Same source, two
different outcomes, decided by whether target/classes happened to be cleaned.
The merge now refuses a hint the properties file also declares instead of
overlaying it, with the message the processor would have given. Aliases count as
the same setting, so and.captureRecord in the file still collides with
@Android(captureRecord).
This is a safety net rather than the primary check: when the processor runs it
has already failed for the same reason and can point at the offending line. It
only matters in the builds the processor never saw, which are exactly the ones
that were silently wrong.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Catalogue the ten build hints the Wear change added
Merged master, which brought in #5583 (complications on the watch, and a Wear
artifact beside the phone APK). It adds ten hints the builders read, and the
catalog gate failed on the merge result: every hint the code reads has to be
described, and the empty baseline means there is nowhere to park one.
That is the gate working, not a conflict. #5583 was written before the catalog
existed, so it had nothing to add its hints to.
Each row's type and default come from the call site rather than from the name:
android.blockLabel boolean, false
android.surfaces.complicationUpdateSeconds int, 0
android.watchModule boolean, true
android.watchVersionCode int, no default -- unset means
derive from the offset below
android.watchVersionCodeOffset int, 100000000
android.wear.complicationsVersion string, 1.2.1
android.wear.tilesVersion string, 1.4.1
android.wear.protoLayoutVersion string, 1.2.1
android.wear.guavaVersion string, 31.1-android
watchNative.surfaces.deploymentTarget string, 10.0
Catalogued, not annotated: the catalog has to describe every hint, but exposing
one as a typed attribute is a curation decision, and inventing API for somebody
else's feature in a merge commit is not that. They are documented, typed and
value-checked, and can be annotated later without churn.
The one nuance worth recording is watchNative.surfaces.deploymentTarget, whose
default is the watch app's floor rather than the extension's: WidgetKit reaches
back to watchOS 9, but the extension is embedded in the watch app, so the lower
number would advertise support that does not exist.
The regenerated developer-guide table is the only other change -- no annotation
churn, as intended.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Write the new catalog rows in the guide's own voice
The developer guide gate treats Vale warnings as errors, and the ten rows added
in the previous commit brought seven alerts with them -- the guide requires
contractions, so "is not", "cannot", "does not" and "it is" all fail, and
"silently" is on the adverb list.
Reworded in the catalog, which is where the prose lives; the table is generated
from it. The meaning is unchanged in every case, including the two that needed
more than a contraction: "a value other than a whole number" rather than "that
is not", and "refuses to install on ... support the user never gets" rather than
"cannot install on ... does not exist".
Vale is clean across all 116 files of the guide.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Stop the catalog gate claiming coverage it never checked
Six from the same review.
The miner reads literals, so a hint whose name is BUILT rather than written was
invisible to it -- and the gate then printed "all described" while the hint had
no catalog row at all. Two shapes occur:
getArg(HINT, null) NativeVerifyOption, HINT="nativeVerify"
getArg(platform + ".maps.provider", ...) MapsProviderInjector
The first is now resolved: a same-file `static final String` whose value is a
literal is substituted. The second cannot be -- the platform is only known at
run time -- so it is REPORTED rather than skipped. Every site that builds a name
must be listed in scripts/build-hint-computed-sites.txt with what it expands to,
and every expansion must be catalogued or match a dynamic pattern, so a new one
forces a catalog decision instead of disappearing. Only expressions containing a
literal are reported; a helper forwarding a variable gets its literal from its
caller, which the ordinary pass already mines. Verified both failure modes fire
by removing a site and by pointing one at a hint that does not exist.
Five sites, three already covered. The two that were not are exactly the ones
named: android.maps.provider, ios.maps.provider, and nativeVerify with its
ios/linux/windows overrides -- all now in the catalog.
The migration trimmed every value. A string, XML or text block can begin or end
with meaningful whitespace: an ios.glAppDelegateHeader ending in a newline after
a // comment loses it and comments out whatever the builder generates next, and
the verification build does not notice because it checks that the key came back,
not what it holds. Trimming is now confined to the scalar types, where the space
cannot be part of the value.
propertyKeyOf did not decode \uXXXX, so a key written codename1.arg.ios...
was read as u0069os.teamId, the original line was left in place, and the
migration rolled back over a duplicate declaration it had created itself.
The Settings source scan missed a Kotlin `import ... as Alias`, under which the
annotation's own name appears nowhere. The hint read as unowned, Add wrote the
properties line, and the next process-annotations failed on that duplicate.
The simulator published a stale manifest without noticing. Judged on timestamps
rather than the fingerprint the native path uses -- recomputing that means
parsing the class file's annotation table and the simulator has no bytecode
reader -- which is sound in the direction that matters, since process-classes
always follows compile within a build. It warns and declines to publish rather
than running on the previous values of hints it can actually see.
codenameone-build-hint-catalog is a runtime dependency of the plugin, so both
release gates now confirm it. Without that a Central deploy that reports failure
after publishing, or a truncated R2 copy, could advertise a release whose plugin
cannot resolve its own dependency.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Verify the migration the way a real build runs
Three from the same review, two of them the same mistake: the verification build
was described in terms of paths and POMs rather than of the project Maven
actually has in hand.
Pointing Maven at the module's own POM discards the reactor, so the module's
siblings resolve from the local repository instead of from the build. A project
whose main module depends on another module of the same build -- normal, and not
necessarily installed -- failed to resolve there and the migration rolled back
over a build that a plain `mvn package` performs happily. The nested invocation
now runs the reactor root with -pl on the owning module and -am, and falls back
to the module POM only when there is no reactor to run.
The manifest was looked for in projectDir/target/classes, which is a convention
rather than a fact: a module may configure build/outputDirectory, and then a
build that emitted every hint correctly was reported as having produced nothing.
Read off the owning MavenProject now, with the conventional path kept for a
directory that is not a reactor module at all.
pr.yml still ignored two of the gate's own files. scripts/build-hint-computed-sites.txt
is the accounting that keeps the miner honest about names it cannot resolve, so a
PR weakening it was the one PR that would not run it. And
docs/developer-guide/_generated-build-hints.adoc was swallowed by !docs/**, which
sits after the scripts re-inclusions, so a hand edit to a generated table was seen
only by the documentation workflows -- none of which run the drift check that
would put it back. The table is re-included after the docs exclusion, since the
last matching pattern decides.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Read the project as configured, and the source as code
Three from the same review.
The nested verification build carried only skipTests, so a project needing
-Pcustomer to compile, or -Dfeature=true to bind process-annotations, was
verified as a different build than the developer ran. That rolls back a
migration that works -- or, worse, passes one whose processing an outer -D was
switching off. The session's active profiles and user properties are copied in
now, with skipTests reapplied afterwards so a user property cannot quietly
displace it. Profiles come from the request rather than from the resolved
project, because what has to be reproduced is what was typed: a profile
activated by a property activates again on its own terms, one named with -P does
not unless it is passed on.
The simulator looked for the manifest in target/classes, which is the default
and not the fact. A module that configures build/outputDirectory had the device
build apply its annotated hints while cn1:run ignored them -- exactly the
asymmetry this publishing step exists to remove. The configured directory is
already on the simulator's classpath, so the classpath is searched instead of
the layout assumed, conventional path first since it is right nearly always and
costs one stat. Only directories are searched: a jar can carry this resource,
but as a dependency, and a dependency's hints belong to whoever built it. The
staleness check follows the manifest to whichever directory it was found in
rather than recomputing the guess.
Settings found annotation markers with indexOf, so a commented-out
`// @Ios(teamId = "old")` counted as ownership and the tool withheld Add and the
editor for a hint the processor never emits -- indistinguishable, from the
outside, from Settings being broken. Marker discovery now walks the source with
the same skipNonCode scanner the argument reader already used, so what counts as
code is one answer rather than two that can disagree. Tests cover a line
comment, a block comment, a quoted annotation, and a live annotation preceded by
a commented-out copy.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Retry the repository failure that was not on the list
Unrelated to build hints; here because it is what turned this PR's
clean-target (arm64) red, and opening a second PR for it is not wanted.
The job died on
Failed to read artifact descriptor for org.sonatype.aether:aether-spi:jar:1.7
fetching a maven-dependency-plugin transitive, and the retry wrapper then said
"not a transient dependency-resolution error; not retrying" -- because that
wording was in none of the eleven copies of the classifier, which have already
drifted into four different alternations.
It is a fetch failure by definition: Maven reached the repository and could not
read the POM. That is the same class as "Could not transfer artifact", which
every copy already retries, so adding it does not soften the gate the way the
blanket loop those comments warn about would. Checked rather than assumed: the
extended pattern matches the line CI actually printed, and still does not match
a compile error or a test failure.
Added to all eleven -- ten workflows and the CEF smoke script -- rather than to
the one that failed, since the next hiccup lands wherever it lands.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Ask the build where it wrote, and key the gate by what it reads
Three from the same review, each a follow-up to the previous round's fix.
Trying the conventional target/classes before the classpath looked harmless and
was not. A project that moves to a configured output directory without running
clean leaves the old target/classes in place, manifest and class together, so
the staleness check compares two obsolete files against each other, finds them
consistent, and publishes last week's hints while the real ones sit on the
classpath. The classpath is searched first now -- it is the output the build is
actually using -- with the conventional path kept only for a launch that never
passed the module's output directory at all.
The computed-site file was keyed by path, so a builder already listed absorbed a
SECOND computed hint for free: adding `platform + ".maps.apiKey"` beside the
provider expression kept the gate green with the new hint uncatalogued. Keyed by
file AND expression now, whitespace-normalised so a reformat is not a change,
and split from both ends so an expression containing a pipe still parses. The
line number is deliberately not part of the key, so moving code does not churn
the file. Verified by adding exactly that second expression: the gate names it.
Alias discovery still used a raw indexOf, so a commented-out `// import ...Ios
as Old` above the live `import ...Ios as BuildIos` won, and the live @BuildIos
was never looked for -- reinstating the bug the alias support was added for. It
uses the same comment-aware walk as the marker search, and the occurrence must
be the target of an `import` on that line, so a mention of the package in code
is not read as one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Collapse the two capture-record spellings, and stop guessing
Three from the same review.
and.captureRecord is not an abbreviation of android.captureRecord: the builder
reads the long name and then lets the short one override it, so the two name one
setting. Uncatalogued as an alias, @Android(captureRecord) and a properties line
spelling it the short way were both accepted -- and the properties line wins in
the builder, so the compile-checked annotation was silently ineffective, which
is exactly the failure this feature exists to remove. Marked as an alias, with a
processor test asserting the pair now conflicts. and.facebook_permissions has
the same override relationship and is marked too: nothing can conflict with it
today since the long name is not annotated, but recording it means annotating
that name later cannot reintroduce this.
Settings looked for an annotation's argument list with indexOf('('), and
parentheses are optional -- a bare @Ios is legal Java and Kotlin. It therefore
adopted whatever call came next, so `@Ios` above a `configure(teamId = "...")`
read as owning ios.teamId and the tool withheld Add and the editor for a hint
the processor never emits. The next LIVE character after the name must now be
the paren, comments skipped, so an annotation's own list is still found across
one and a bare annotation adopts nothing.
The simulator never checked the manifest's main-class stamp. Change
codename1.mainName without a clean build and the old class and its manifest stay
together in the output directory, perfectly consistent with each other, so the
timestamp check passes them and the previous application's hints get published.
The native merge has refused this since it was written; the simulator does now
too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Let the simulator see a duplicate declaration too
Two from the same review.
Publishing an annotation hint the properties file also declares buries the
error rather than reporting it: buildHint() reads the system property before the
settings file, so the line the developer just added is silently ignored in the
simulator while the device build refuses to run at all. Reachable because editing
the properties file does not touch the class, so the timestamp check still finds
the manifest current. The pair is detected before publishing now, and the
annotation value is withheld with a message naming the hint.
Aliases have to collapse for that check, and the catalog that knows about them is
a build-time artifact this port cannot reach. Rather than give the JavaSE port a
dependency on it, the processor writes the other spellings of each hint into the
manifest -- cn1.buildHints.alias.<name> -- alongside the origin it already
records, and only where a hint has more than one. The conflict check and the
manifest now derive that set from one method instead of two that can disagree.
This is the third time the simulator has lagged a check the native merge already
performed. Recorded in the reply as such: if a fourth appears the two paths want
a shared decision rather than parallel implementations.
skipNonCode had no triple-quote branch, so a Kotlin raw string or Java text block
read as an empty literal followed by a new one -- and an embedded quote then
opened a literal that swallowed the annotation after it, leaving the hint unowned
and Add free to write the duplicate. Both directions are tested: a raw string
containing a quote no longer hides the annotation after it, and an annotation
written inside one is still not ownership.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Take the published hints back out on a simulator reload
A reload re-enters main() in the same JVM rather than starting a process, so
everything published on the previous launch is still set. The rule that protects
a -D value -- an existing value always wins -- then protected the PREVIOUS
build's annotation value too: editing @Desktop(titleBar = ...) and reloading
kept showing the old setting, and deleting the annotation altogether kept it
forever, since the missing manifest takes an early return that touched nothing.
What this method installed is now withdrawn at the top, before any early return,
so a removed annotation and an unreadable manifest clear the value as surely as
a changed one replaces it.
Only what it installed. A -D was never a candidate, because a key that is
already set is skipped rather than published, so it can never enter the withdraw
set -- the command line keeps winning without a special case for it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Read a text block the way its own language does
Two from the same review.
The triple-quote branch closed at the next """, which is wrong in BOTH languages
and in opposite ways -- and getting it wrong over-consumes past a live
annotation, so the hint reads as unowned and Settings writes the duplicate.
Java escapes DO apply, so \""" is an escaped quote and two more, not a
delimiter. Reading it as one made the REAL delimiter open a second
text block that ran past whatever followed.
Kotlin escapes do NOT apply, and a run of four or more quotes closes at its
LAST three: """a"""" holds a" .
So the scanner needs to know which language it is reading, and it can: the
caller already picks the file by extension. The flag is threaded from there
down to skipNonCode, with the two-argument entry point keeping Java rules for
callers that have no file to name.
Separately, the manifest was merged over the source result as a union. It cannot
ADD ownership the source does not show: an attribute deleted from the main class
and not yet rebuilt is precisely that, and the union kept Add and the editor
hidden for a hint nothing owns any more, until the user happened to rebuild with
nothing to suggest that was the fix. The source now decides WHICH hints are
owned and the manifest only supplies their origins -- except when no source file
could be read at all, which the scan now reports as null rather than as an empty
result, because there the manifest is all there is.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Find the hints that live in a table
Three from the same review.
Ten real hints had no catalog row while the gate reported success.
IPhoneBuilder's WALLET_INJECTION_HINTS holds them in a String[][] and reaches
getArg as hintAndMarker[0], so no literal anywhere in the tree sits at a getArg
call -- invisible to every literal search, this gate included, which is exactly
the hole the computed-site accounting was added to close. A subscript now counts
as a computed name alongside a built one; that adds precisely one site, and its
ten expansions are catalogued and accounted for.
An output directory keeps class files whose source is gone. Rename the main
class, update codename1.mainName, skip the clean, and the old annotated .class
still sits there -- so every incremental build failed with a placement error
naming a class the developer had already deleted, and the orphan's hints were
merged in besides. A class that is not the main one and has no source under the
project is now ignored.
Two limits on that, and the second only because the tests caught it: the main
class is never dropped, because failing to find ITS source would silently apply
none of a project's hints, which is worse than any placement message; and a
project with no source tree at all is left alone entirely. Absence of a source
tree is not evidence a class is orphaned, it means this is a layout the lookup
does not know.
The migration read byName(), which returns the ALIAS entry for a legacy
spelling -- and an alias's own isAnnotated() is false even though the setting it
names has an annotation. So cn1.androidTheme, cn1.nativeTheme and
and.captureRecord, the spellings an existing project is most likely to be
carrying, were reported as having no annotation and left behind. Resolved
through the alias now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Render the guide's hint table instead of committing it
3,345 generated lines leave git. The table is rendered from
maven/build-hint-catalog every time the developer guide is built, so it cannot
drift from the catalog and a hand edit has nothing to survive in -- which is
what a generated file living in the repository always eventually invites, and
what the drift gate and the pr.yml path re-inclusion existed to compensate for.
Both of those go with it.
Two renderers, so the step is a script rather than a workflow line:
developer-guide-docs.yml runs it before the Asciidoctor lint, which is ahead of
everything else that reads the guide -- the HTML and PDF build, Vale -- and
scripts/website/build.sh runs it before its own asciidoctor call.
The generator grows a --table-only mode for them. A documentation build has no
business rewriting CodenameOne/src and Ports/JavaSE on its way past, and the
mode's output is byte-identical to what the full run wrote, so this is not a
second implementation of the table.
Forgetting the step cannot be quiet: asciidoctor reports "include file not
found" and the lint fails on it. Verified in both directions -- the render fails
without the table and succeeds after generating it.
scripts/gen-build-hint-annotations.sh still writes the file for a local
asciidoctor run, and it is gitignored, so that copy is never reviewed and never
committed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Migrate a legacy spelling to the name the build emits
Four from the same review, two of them consequences of the alias fix I made
last round.
Verification searched the manifest for the key the FILE used. A legacy spelling
is deleted under its own name and comes back under the canonical one, so
migrating cn1.nativeTheme, cn1.androidTheme or and.captureRecord reported the
hint missing and rolled a correct migration back. The two lists are separate
now: migratedKeys is what gets deleted, verifiedKeys is what must come back.
Declaring one setting under two spellings with different values was resolved by
whichever Properties.stringPropertyNames happened to enumerate last -- and then
both lines were deleted, so a migration could change what the app builds with
and still report success, since the check asks whether the hint came back and
not what it holds. There is no rule to apply: and.captureRecord is read after
android.captureRecord and overrides it, while the theme aliases are each handed
to Display.setProperty and resolved in the framework. So the ambiguous case is
refused, naming both spellings, and the developer decides. Equal values are not
ambiguous and migrate.
Settings accepted a bare @Build or @Android whatever it meant. Those names are
ordinary enough that another library's annotation with a matching attribute read
as ownership, and the editor was withheld for a hint the processor never emits.
The simple name now counts only when an import brings it in from
com.codename1.annotations.buildhints -- by name or on demand -- while the fully
qualified spelling still needs none. Nine existing tests failed on this, all
because their snippets used a bare annotation with no import, which no real
source does; they carry the import now and are better tests for it.
Two identical computed expressions in one file collapsed to one key, so a second
getArg(hintAndMarker[0], ...) over a different table would have been validated
against the first table's expansions and its own hints never checked. The
accounting line carries a call count -- `#2` -- and a mismatch is reported with
the number to record, which forces the second call to be looked at.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Run the docs checks when the table's inputs change
Generating the build hint table instead of committing it took away a trigger
nobody had to think about: while the table was checked in, editing the catalog
produced a diff under docs/ and the documentation workflow ran on it for free.
Now a change to maven/build-hint-catalog or to the renderer changes what the
guide contains without touching a file under docs/ at all, so the Asciidoctor
lint, Vale, and the HTML and PDF build never saw it -- and a malformed table
would have merged and surfaced in the release documentation build.
Both paths are added to the workflow trigger AND to the paths-filter the steps
are gated on. Triggering alone is not enough: the HTML and PDF build and the
steps beside it are conditional on that filter, so they would have started and
skipped.
The render step also moves ahead of everything that reads the guide rather than
sitting just before the lint -- the image and snippet checks read it too. It is
unconditional, because every one of those steps is not, and asciidoctor reports
a missing include as an error, so a future step inserted above it fails loudly
rather than rendering a guide with the table quietly absent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Accept what the runtime accepts, and ask Maven where the sources are
Three from the same review.
A closed domain rejected spellings the runtime honours.
IOSImplementation.installNativeTheme compares against flat, liquid and iphone
alongside the catalogued values, AndroidImplementation against material and
holo, so Settings told a developer that a working configuration was invalid and
then refused to save the edit -- and the migration refused those values as
outside the domain, which is exactly what a project old enough to be carrying a
legacy spelling would have hit.
They are recorded as valueAliases, deliberately NOT as domain values: an alias
must not become an enum constant, because two constants for one behaviour is an
API that asks a question with no right answer. Validation and migration both go
through canonicalValue, so `flat` saves and migrates to IosThemeMode.IOS7, while
the picklist and the annotation still offer one spelling per concept.
`import ...Ios as BuildIos` puts BuildIos in scope and NOT Ios, so counting it
as a simple-name import attributed another library's @Ios to us -- the same
misattribution the import check had just been added to prevent. An import with
an `as` clause now contributes only its alias marker.
The orphan check hard-coded src/main/java. A module may add generated-sources or
replace the conventional root, and Kotlin does not require a file to be named
after the class it declares, so a live class could be read as orphaned -- and
then silently dropped, taking its hints with it and suppressing the placement
error that would have explained it.
It now asks Maven for the compile source roots, which is where generated and
Kotlin roots already are, and looks for the file name the COMPILER recorded in
the SourceFile attribute rather than one derived from the class name. That is
the one reliable link back from a class to its source, and it is what makes the
Kotlin case answerable at all. ClassScanner records it; AnnotatedClass exposes
it. Unknown roots, or a class compiled without debug information, still answer
"has a source", because the only thing this decides is whether to IGNORE a
class.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Identify a class by what it declares, not where it sits
Three from the same review.
Three more real hints had no catalog row. MacNativeBuilder reads them through
parseEntitlementBool(request, hint, def), so every caller passes a literal, none
of the calls is an accessor, and a literal search of accessor calls walks past
all of them. The miner now recognises a helper that forwards one of its own
parameters to getArg/arg/booleanArg and mines that helper's CALLERS instead --
which found exactly the three named, by looking rather than by being told.
Same-file only, deliberately: a private helper's name is not unique across the
tree, and mining calls to a same-named method of an unrelated class would invent
hints rather than find them.
The orphan check matched a SourceFile name anywhere under any source root, so
moving a class to another package without a clean left an orphan that the NEW
file answered for -- App.java satisfying the lookup for the old package's
App.class -- and the stale class stayed, failing the placement check on every
incremental build. The package is part of the match now, read from the file
rather than inferred from its directory, since Kotlin does not require the two
to agree.
Settings had the same conventional-roots assumption on the other side, and
falling through to null there let the caller trust a stale manifest again -- the
bug the source scan exists to prevent, reappearing for anyone whose layout is
merely unusual. It searches the project for a file that DECLARES the class now:
package statement plus a class or object declaration, so a configured root or a
Kotlin file named after something else is found anyway. Bounded in depth, in
queue length and in how many files it will open, and target/ and build/ are
skipped so a compiled copy of the same source cannot answer for it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Describe the plist keys we inject, individually
Two from the same review.
IPhoneBuilder builds a hint name into a local -- `"ios." + privacyKey` -- and
passes the local to getArg, so the accessor's argument is a bare variable and
read as forwarding while the name is assembled two lines up. That is the fourth
route by which a literal reaches an accessor without sitting at one. The miner
recognises a local assembled from a literal now and reports it, which surfaced
this site and no other.
The finding underneath it is sharper than "uncatalogued", and worth stating
exactly: ios.NSBluetoothAlwaysUsageDescription, its Peripheral twin and
ios.NSSpeechRecognitionUsageDescription were matched by the dynamic family
ios.NS*UsageDescription, so the gate was right that they were "described" and
they still had no annotation, no documentation row and no editor entry. That
family exists so an app can set an ARBITRARY Apple key. A key the platform
feature catalog injects is a known one, and known keys get described
individually -- they are now, as @IosPrivacy attributes like the other fourteen.
So the new cross-check requires a CONCRETE row for every injected plist key and
deliberately does not accept the dynamic pattern. My first version did accept it
and was therefore inert; the negative test passing is what showed that, after I
went looking for why an injected unknown key did not fail the gate.
Second finding: matching a source file by name and package alone kept a stale
class when a Kotlin type is renamed in place without renaming its file, or when
one type is deleted from a file holding several -- the survivor answered for it,
the orphan stayed, and the placement check failed every incremental build. The
file must declare the type now: class, interface, enum, object or record.
Unreadable still answers yes, as everywhere else in this guard, because what it
decides is whether to IGNORE an annotated class.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Ask about the main class, not about every class file
Three from the same review, all fallout from the orphan filter.
A nested type's binary name is Main$Wrong and no source declares a type spelled
that way, so the search found nothing, the clas…
Resolves#3848.
The request was database encryption. Encryption is here, but the reason it took a
whole PR is that
com.codename1.dbwas not one API over SQLite -- it was fiveunrelated implementations that happened to share an interface, and there was no
sensible place to add a key to.
What was actually wrong
Verified in the source, not from memory:
openOrCreatenull, callers NPElast()/prev()/position()IOException("Unsupported")position(n)always gave row 0getPosition()basefirst()trueon an empty set, then reads unset memorygetBlob{ return nil; }execute(sql)multi-statementBEGINprintlnno-opsRuntimeExceptionon every portPlus three defects worth calling out on their own:
sqlDbClosecalledsqlite3_freeon asqlite3*, so no iOS connection was ever closed, the WAL wasnever checkpointed and the handle went to the wrong allocator;
SEDatabaseleakeda
PreparedStatementper query; andThreadSafeDatabase.close()was fire andforget, so a following
delete()raced it.And no device test touched
Databaseat all -- 142 test classes in the screenshotsuite, none of them about databases. That is why Windows and Linux were allowed to
ship with no implementation.
What this does
One contract.
com.codename1.db/package-info.javanow states what every portmust do, and
DatabaseConformanceSuitein the framework checks it. Seven devicetests run that suite on every port in CI; two of them run in legacy mode.
One cursor implementation.
AbstractDBCursorderives all navigation from twoprimitives,
rewind()andstepForward(), so ports stop re-deriving it. Seeksrewind and re-step rather than buffering:
sqlite3_column_*is only valid on thecurrent row, so buffering would mean copying every column of every row stepped
past, blobs included. This is what Android's windowed cursor already does on a
window miss.
Encryption, with a passphrase, a keystore-managed random key, or raw bytes.
Managed keys resolve in the core so every platform derives identical material from
an alias, and a key that cannot be stored is fatal rather than a silent downgrade
to plaintext.
Windows and Linux get a database at all.
JavaScript stops using WebSQL, which Chrome removed in 119 and Firefox never
implemented, in favour of the same SQLite compiled to WebAssembly.
Compatibility
Ten behaviours change in ways an application could depend on. All ten are restored
by the
db.legacybuild hint, per platform, and two device tests assert that itreally does restore them -- so the promise is testable rather than aspirational.
The table is in the developer guide.
The hint deliberately does not cover defects, or capabilities that used to throw
and now work. Nobody can depend on
getBlobreturning null.Cost, when unused
Nothing. iOS keeps the system SQLite unless the app references
DatabaseConfig;Android's SQLCipher package is deleted and its AAR never added; Windows and Linux
compile the engine to an empty object; the JavaScript builder prunes 1.5MB from
bundles that never open a database. Two catalog tests hold that line, because the
entry is keyed on
DatabaseConfigrather than the package -- keying it on thepackage would bundle SQLCipher for every database app and push Android's minimum
SDK from 19 to 23 for people who never asked for encryption.
Verification
SEDatabaseConformanceTestcases, all green.android,ios,codenameone-maven-pluginandByteCodeTranslator.scripts/ci/db-cipher-interop.sh, wired into PR CI, writes an encrypted databasewith our engine and reads it with the stock
sqlcipherclient, and vice versa,with both a raw key and a passphrase. This is the check that matters: a cipher
misconfiguration produces files each platform reads happily and nothing else can
touch, which no single-platform test would catch.
sqlcipher4.17.0 client and the realnet.zetetic:sqlcipher-androidAAR, not against assumed APIs.Three things the spikes caught
Worth recording, because each would have shipped broken:
sqlcipher_export()does not exist in SQLite3MC, so the ATTACH-basedmigration everyone writes would have failed.
PRAGMA rekeyworks, and alsopreserves
user_version, whichsqlcipher_exportdrops.getConnection()on the simulator but on first read onthe device ports, so both paths need handling.
SQLiteMCSqlCipherConfig.getDefault()really does produce files real SQLCiphercannot open;
getV4Defaults()is required. One line, and nothing but across-engine test would have found it.
Review rounds
Nineteen findings from the automated reviewers, all real, all fixed. The ones worth knowing about:
Database.encrypt()could never have worked on Android. The system SQLite has no cipher, so aplaintext database opened through it can never be re-keyed; there is now a platform hook that
routes the migration through SQLCipher.
nullwhen re-keying, so
changeKey(managed())raised aNullPointerExceptionrather than encrypting./,\,:and space all to_, socustomer/dbandcustomer_dbshared one key and forgetting either destroyed the other.
and
sqlite3_close_v2then leaves a zombie connection alive forever.isEncrypted()reported every plaintext JavaScript database as encrypted, because that port hasno readable path and a failed header read is indistinguishable from ciphertext.
PRAGMA rekeyinterpolated the key directly, so a passphrase containing a quote changed thestatement.
Two of the fixes are covered by new conformance checks, including one verified by reinstating the
old code and watching it fail: the exhausted-cursor count went 5 to 8 before the fix.
Two decisions worth a second opinion
maven/sqlite-jdbcis no longer frozen. It was pinned and excluded frompublication because a shade of a fixed driver never changed. It now carries the
engine used to read encrypted databases, so it has to track upstream security
releases. Costs ~13.5MB per release, which is what the freeze was avoiding.
compile. It ships a prebuilt amalgamation where SQLCipher would need its
configure script run per build, and it is what the simulator's JDBC driver is
already built from -- so iOS, Windows, Linux, JavaScript and the simulator all
run one engine at one version. Android still uses the SQLCipher AAR because it
cannot compile C in our build; both write the same format, which is the part
that matters.
Companion PR
The build-side gating is mirrored in codenameone/BuildDaemon#172, which is green.
🤖 Generated with Claude Code