Repository navigation
Swing, SwingX and JavaFX compatibility layers: desktop Java apps as Codename One apps - #5980
shai-almog wants to merge 279 commits into
Conversation
The compliance check reads the registry's members from the shared JDK classes' jar, so the generated class's references to itself are checked against the placeholder. Give it the same constructor and methods. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d Locale a device lacks A class the device has cannot be moved whole by a JDK_SHIMS row, so CompatRewrites redirects single calls, static field reads, constructors and method reference handles to Resources, JdkObjects and JdkLocale. It runs ahead of the relocation, only when a desktop layer is active, and leaves the shared JDK classes and the Android runtime alone. Anything else of ClassLoader has no rule and stays a build error. Adds the JDK_SHIMS rows for Properties, UUID and the executors, and the dependency on the shared JDK classes that the resource flattening shares its naming rule through. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e reads them through CompatResources writes every nested file of the desktop resource directories to the root of the classes directory under ResourceNames.flatName, removes the nested copy, and fails the build when that name belongs to another file. It generates com.codename1.compat.jdk.CompatRegistry (and its part classes) with the shipped paths, the properties bundles and the bundle classes, created with new. CompatRemapper takes the directories through withResourceDirectories, defaulting to the resources directory beside the desktop entry record; the Maven goal adds the generated desktop resources. The end-to-end test compiles an application with javac, remaps it, passes the compliance check against the real device library and runs it against a bundle holding flat names only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e size hints Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nent, list and split pane keys, click counts by place Also: the lifecycle exits through the exit hook once the last window it showed is disposed of, synthesized wheel pointers no longer become mouse events, and the click tests no longer sleep. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nitor, blocking as on the desktop Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Code fixes where the shape allowed it; scoped exclusions, in the slice's own block, where the JDK's API forces the pattern. The table and tree tests no longer sleep out the click count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The decorator package (highlighters and predicates), the renderer package (string/icon values, providers, the table, list and tree renderers), the sort controllers, the hideable columns with their column model and factory, and JXTable on top of them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…us bar, hyperlink JXTitledPanel, JXHeader, JXCollapsiblePane, JXTaskPane and its container, VerticalLayout and HorizontalLayout, JXStatusBar with its constraints, and JXHyperlink with the hyperlink actions, which open an address through Codename One. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ext fields JXMonthView draws one month and selects through the SwingX date selection models; JXDatePicker is a Codename One date Picker that keeps a month view in step. PromptSupport and the JX text fields map the prompt to the hint of the text field behind the component. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AbstractPainter and the layout and area bases, the matte, compound, rectangle, text, image, gloss, pinstripe, checkerboard, busy, alpha and shape painters, and JXLabel, JXButton, JXBusyLabel and JXTitledSeparator over them. Tests draw into a recording graphics. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ighlighters and rollover Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ides columns Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lifecycle and input events Node, Parent, Group, Region and Pane with one lazily created Codename One peer per node, the JavaFX layout pass pushing bounds into the peers in device pixels, capture and bubble dispatch of pointer and key input, focus, hover and pressed state, the stage hosts (form, window, dialog), Application, Platform and FxLifecycle, the styling hooks, and the first controls (Control, Labeled, ButtonBase, Button, Label). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ene graph to zero SpotBugs findings Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HBox, VBox, BorderPane, StackPane, GridPane (with ColumnConstraints, RowConstraints and ConstraintsBase), FlowPane, TilePane and AnchorPane, with the per child constraints, grow and shrink distribution, margins, spans, percentages, wrapping and anchors, and the style hooks the styling contract promises for them. LayoutPanesTest holds each pane to child bounds worked out by hand from the documented layout rules. Region gains a package-private way to position a child inside a margin without resizing it, which BorderPane needs for its edges. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ab panes TextInputControl, TextField, PasswordField, TextArea, CheckBox, ToggleButton, RadioButton, Toggle, ToggleGroup, Hyperlink, Separator, Slider, ProgressIndicator, ProgressBar, ScrollPane, SplitPane, TabPane, Tab, SelectionModel, SingleSelectionModel and IndexRange, each over the matching Codename One component where one exists. The scroll, split and tab panes are built from scene-graph regions so their content stays a real descendant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
javafx.animation: Animation, Timeline, KeyFrame, KeyValue, Interpolator, AnimationTimer, Transition and the pause, fade, translate, scale, rotate, sequential and parallel transitions, all driven by one frame clock (runtime.FrameClock) that registers with the form on screen only while something runs and has a manual mode for tests. javafx.concurrent: Worker, WorkerStateEvent, Task and Service. Codename One has no java.util.concurrent beyond the atomic classes, so a Task is a Runnable rather than a FutureTask, has no blocking get(), and a Service has no executor property. The hand-over between threads uses atomics and Platform.runLater only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds javafx.scene.shape (Shape and its subclasses, Path and its elements, SVGPath with a complete path data parser), javafx.scene.text.Text, javafx.scene.transform (2D), javafx.scene.canvas and javafx.scene.image. Everything draws through the one Renderer over FxPath, which gain an elliptical arc, an elliptical round rectangle and a clip. Node.getTransforms() is folded into the paint matrix and so into localToParent, bounds and picking. A canvas keeps a display list that is replayed on paint and is bounded by clearRect and a cap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…logs ListView and TableView show their items in recycled cells over a shared row flow, with single and multiple row selection. ComboBox, ChoiceBox and DatePicker sit on the Codename One picker. ContextMenu, Menu and MenuBar open as popups in a layer over the owner's form, which every host reaches through SceneInput. Dialog, Alert, TextInputDialog and ChoiceDialog block through a modal stage. Control gains a context menu and a tooltip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…(T) of an absent item as JavaFX has it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llapsible pane's toggle action, action accelerators The click tests no longer wait the click count out; the test base resets it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…odelSupport Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… classes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…yout A text area given its text inside a scroll pane overflowed the stack on the pane's next layout: JViewport.doLayout showed the caret, showing a rectangle validated the viewport, and the viewport -- still invalid, because it was in the middle of that very layout -- laid itself out again. Container.validateTree and JViewport.doLayout no longer start a pass that is already under way, which covers the same shape in JList.ensureIndexIsVisible, ScrollDelegate.reveal (JTable, JTree) and JComponent.scrollRectToVisible. The caret is shown after the widget received the text that moved it, not before, so an appended log is followed to its last line. JXErrorPane's workaround is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
maven/swing-compat and maven/javafx-compat are relocated into an application and translated by ParparVM with it, where a failed cast is not an exception. Both are roots of check-cast-semantics.sh now, and the PR build compiles them so that --require-all finds them. The six findings were all in javafx.beans.binding.Bindings: the create*Binding factories read the result of an erased Callable inside a try that catches Exception. The result is read as an Object and tested with instanceof. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d iOS Metal Also reseeds the five Linux goldens that changed when the port began to honour bold and italic native fonts and scaled images, and the JavaScript ButtonTheme pair whose flat button gradient the GeneralPath clip fix corrected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Compared 183 screenshots: 183 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
|
Compared 198 screenshots: 198 matched. ParparVM vs HotSpot (JDK 25): Linux x64Runner CPU: AMD EPYC 9V74 80-Core Processor (baseline 1 performance regression:
Ratios are ParparVM / JDK 25: below 1.00x ParparVM is faster (time) or smaller (RAM). Median of 5 interleaved, paired rounds; every run's output was verified. A ratio more than 15% (time) / 15% (RAM) away from its baseline in
A change that moves performance ON PURPOSE records the new baseline in Result: performance regression |
|
Compared 198 screenshots: 198 matched. ParparVM vs HotSpot (JDK 25): Linux arm64Runner CPU: Neoverse-N2 (baseline 1 improvement past the baseline, which fails until rebaselined:
Ratios are ParparVM / JDK 25: below 1.00x ParparVM is faster (time) or smaller (RAM). Median of 5 interleaved, paired rounds; every run's output was verified. A ratio more than 15% (time) / 15% (RAM) away from its baseline in
A change that moves performance ON PURPOSE records the new baseline in Result: improved past the baseline: rebaseline required |
BytecodeCompliance: the backend check master added runs inside this branch's scanner. In that mode it reads what master's read and leaves bootstraps and dynamic constants to checkBackendBootstraps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pt, iOS 27, tvOS and Mac Catalyst Windows also reseeds AndroidCompatMain, SurfacesRasterizer and graphics-draw-string, which now draw the bold and italic faces a derived native font asked for, and DesktopMode. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Compared 219 screenshots: 219 matched. |
|
Compared 198 screenshots: 198 matched. Benchmark ResultsDetailed Performance Metrics
|
Apple Watch (watchOS / Core Graphics)Compared 249 screenshots: 227 matched, 9 updated, 13 missing references.
|
|
Compared 180 screenshots: 180 matched. Benchmark Results
Detailed Performance Metrics
|
… follows the runtime The device runtime now has the streaming half of java.util.zip, so the compliance advice and the porting table no longer send a zip stream to com.codename1.io.gzip. Only ZipFile is still absent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
CodenameOne/maven/compat-jdk/src/main/java/com/codename1/compat/jdk/File.java
Lines 293 to 301 in 4a287d0
Make createNewFile's existence check atomic
When two threads use createNewFile() for the same path, both can observe that it does not exist and both then open it for output, so both return true and the later open may truncate data written by the first caller. Desktop code commonly relies on this method's atomic create semantics for lock and sentinel files; serialize the check-and-create operation at least within the application process, or use an exclusive-create primitive where the port provides one.
Refuse append offsets that overflow the storage API
When appending to a file larger than Integer.MAX_VALUE, narrowing its length to int wraps the offset before it is passed to FileSystemStorage. Depending on the port this either fails or writes at an earlier position and corrupts the existing file instead of appending; since the underlying API cannot express larger offsets, detect this case and fail explicitly rather than truncating the position.
ℹ️ 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".
| if (JAVA_MAIN.matcher(code).find()) { | ||
| candidate(out, prefix + fileClass); |
There was a problem hiding this comment.
When public static void main belongs to a static nested class or another top-level class in the file, this records the source filename's class instead of the declaring class (for example, Outer rather than Outer$Launcher). The import therefore succeeds with an invalid entry record, and DesktopEntryPoints later rejects the outer class because it has no main method; determine the main method's enclosing declaration as is already done for nested JavaFX classes.
Useful? React with 👍 / 👎.
| outstanding.add(job); | ||
| int slot = next; | ||
| next = (next + 1) % workers.length; |
There was a problem hiding this comment.
When two threads call execute concurrently, both mutate the unsynchronized ArrayList and next fields here; concurrent ArrayList.add operations can overwrite one bookkeeping entry even though both jobs are queued on EasyThread. A subsequent shutdownNow() then cannot find and drop the lost entry, so that queued task may run after immediate shutdown; access to the acceptance check, outstanding list, worker selection, and shutdown paths needs one synchronization boundary.
Useful? React with 👍 / 👎.
| } else if (delayAfter > 0 && !outcome.isDone() && !shutdown.get()) { | ||
| again(); | ||
| } | ||
| } finally { | ||
| running.set(false); |
There was a problem hiding this comment.
For scheduleWithFixedDelay, again() installs a one-shot timer while running remains true until the finally block. With a short delay or a thread preemption at this point, the timer can fire first, fail the compare-and-set in run(), and return without scheduling another attempt; the periodic task then stops permanently while its future remains incomplete.
Useful? React with 👍 / 👎.
| if (parent != null && !parent.isDirectory() && !parent.mkdirs()) { | ||
| throw new IOException("Cannot create " + parent); | ||
| } | ||
| Files.copy(src.toPath(), dest.toPath(), java.nio.file.StandardCopyOption.REPLACE_EXISTING); |
There was a problem hiding this comment.
When the destination already contains a developer-created file that was never listed in the previous import record, recorded is null and this unconditional REPLACE_EXISTING silently overwrites it. This occurs both on the first import into a nonempty desktop source tree and when an upstream project later adds a path the developer had independently added; treat an unrecorded existing destination as user-owned rather than replacing it.
Useful? React with 👍 / 👎.
| Entry entry = new Entry(jar.length() + "\t" + jar.lastModified()); | ||
| entry.describe(lib); | ||
| Set<String> shipped = lib.shipped(); | ||
| if (!shipped.isEmpty()) { | ||
| for (Map.Entry<String, List<String>> service : lib.reachedServices().entrySet()) { | ||
| for (String provider : service.getValue()) { | ||
| entry.services.add(service.getKey() + "\t" + provider); | ||
| } | ||
| } | ||
| } | ||
| boolean same = old != null && old.stamp.equals(entry.stamp); |
There was a problem hiding this comment.
When a dependency jar is rebuilt in place with the same byte length and last-modified timestamp, this stamp marks it unchanged even if its entries differ. The call to unpack then treats all previously recorded files as current and skips reading their new bytes, leaving stale relocated classes and resources in the application; this can occur with reproducible artifacts or coarse-timestamp filesystems, so the reuse decision needs a content digest or entry-level comparison.
Useful? React with 👍 / 👎.
| public URI toUri() { | ||
| String storage = toFile().storagePath(); | ||
| try { | ||
| return new URI(storage); | ||
| } catch (URISyntaxException e) { | ||
| throw new IllegalArgumentException(e.getMessage()); |
There was a problem hiding this comment.
When a path contains a space, #, %, or another character that must be escaped in a URI, passing the raw storage path to this constructor throws IllegalArgumentException; for example, Paths.get("a b").toUri() should return a URI containing a%20b. This also breaks File.toURI(), which delegates here, for ordinary filenames containing such characters; construct the URI from decoded path components so Java performs the required percent-encoding.
Useful? React with 👍 / 👎.
| if (has("<artifactId>" + artifactId + "</artifactId>")) { | ||
| return false; |
There was a problem hiding this comment.
When the same artifactId appears anywhere else in the POM—such as only under dependencyManagement, as a plugin dependency, or under a different group—this global text check suppresses the actual module dependency and reports no manual action. The imported sources then compile without the library they require or against an unrelated artifact; deduplicate only matching groupId:artifactId entries in the module's own dependency list.
Useful? React with 👍 / 👎.
|
Compared 192 screenshots: 192 matched. Benchmark Results
Detailed Performance Metrics
ParparVM vs HotSpot (JDK 25): macOS arm64Runner CPU: Apple M1 (Virtual) (baseline 1 performance regression:
Ratios are ParparVM / JDK 25: below 1.00x ParparVM is faster (time) or smaller (RAM). Median of 5 interleaved, paired rounds; every run's output was verified. A ratio more than 15% (time) / 15% (RAM) away from its baseline in
A change that moves performance ON PURPOSE records the new baseline in Result: performance regression |
|
Compared 181 screenshots: 181 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
- An import no longer writes over a file no import wrote. - The entry record names the class a main is declared in, nested or a second top-level class included. - The executor's bookkeeping is safe to submit to from several threads. - A fixed-delay task is armed again only after its run has ended. - A library jar's stamp includes a checksum of its entry listing. - Path.toUri and File.toURI escape the path. - An imported library is looked for in the module's own dependencies, by group and artifact. - Appending to a file past the int offset range fails instead of wrapping. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 534d0f01fc
ℹ️ 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".
| int slot = (next.getAndIncrement() & 0x7fffffff) % workers.size(); | ||
| worker(slot).run(job); |
There was a problem hiding this comment.
When a first submission passes the shutdown check and is paused before worker(slot), a concurrent shutdown() sees no worker to stop; the submitter can then create a new EasyThread and queue the accepted job after killWorkersWhenIdle() has finished. That worker is never asked to stop, so after the job completes isTerminated() remains false and awaitTermination() always times out. Recheck shutdown after worker creation or synchronize worker creation and shutdown.
Useful? React with 👍 / 👎.
| if (ext.group(2).indexOf('.') < 0 && !imported) { | ||
| continue; | ||
| } | ||
| candidate(out, prefix + ext.group(1)).javafx = true; |
There was a problem hiding this comment.
For a valid nested Kotlin application such as class Host { class App : Application() { ... } }, the JVM class is Host$App, but this records only App. The import therefore writes a nonexistent mainClass, and DesktopEntryPoints later rejects it even though the compiled nested application is present; derive the enclosing declaration chain as the Kotlin @JvmStatic path already does.
Useful? React with 👍 / 👎.
| @Mojo(name = "remap-compat", defaultPhase = LifecyclePhase.PROCESS_CLASSES, | ||
| requiresDependencyResolution = ResolutionScope.COMPILE) |
There was a problem hiding this comment.
A Maven desktop application can legitimately declare a plugin or service-provider jar with runtime scope, including a library whose bytecode uses Swing or JavaFX. This mojo requests only the compile dependency set, and the later artifact filter also accepts only SCOPE_COMPILE, so such a jar never reaches CompatLibraries and its toolkit references are not relocated before packaging. Request runtime dependency resolution and include runtime-scoped jars while continuing to exclude provided dependencies.
Useful? React with 👍 / 👎.
| copyTree(from, to); | ||
| deleteTree(from); |
There was a problem hiding this comment.
When moving a directory between parents, File.renameTo falls back to this copy-and-delete path, but deleteTree ignores every failed deletion. If a child or the source directory cannot be removed, move still returns success with the source wholly or partially present alongside the copied target, so callers may proceed believing ownership was transferred. Propagate deletion failures as an IOException instead of unconditionally completing the move.
Useful? React with 👍 / 👎.
| @Override | ||
| public void run() { | ||
| EventQueue.invokeLater(tick); |
There was a problem hiding this comment.
When the event dispatch thread takes longer than the timer delay, every background firing reaches EventQueue.invokeLater even though coalesce defaults to true. A Swing Timer keeps only one pending event in this mode; here the event queue can grow without bound and then deliver a burst of stale actions after the event thread unblocks. Track whether a tick is already queued and suppress subsequent firings unless setCoalesce(false) was requested.
Useful? React with 👍 / 👎.
| if (repeats) { | ||
| timer.schedule(task, initialDelay, Math.max(1, delay)); | ||
| } else { | ||
| timer.schedule(task, initialDelay); |
There was a problem hiding this comment.
When a timer is started with repeats disabled and setRepeats(true) is called before its first firing, Swing begins rescheduling it, but this chooses a one-shot task permanently during start(). The task fires once while tick() now sees repeats == true, so it neither repeats nor calls stop(); isRunning() remains true and subsequent start() calls are ignored. Schedule subsequent firings according to the current repeat flag rather than capturing it only at startup.
Useful? React with 👍 / 👎.
…xes from review - javax.swing.Timer coalesces the periods a busy event dispatch thread missed into one event, and asks whether it repeats at each firing. - A fixed pool stops a worker that a submission started after shutdown had swept the workers, so it terminates; a stopping worker's refusal is a RejectedExecutionException. - Files.move fails when the copied source cannot be deleted. - remap-compat relocates runtime-scoped libraries, as the Gradle build does. - The importer records a nested Kotlin JavaFX application as Outer$Nested. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… ends mid-drain A period that elapses while the queue is drained is a new event, so the coalesced count is bounded rather than exactly one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9919796706
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// The `**/*.fxml` and `**/*.css` files under the desktop resources. | ||
| /// The task is skipped when there are none. | ||
| @InputFiles | ||
| @SkipWhenEmpty |
There was a problem hiding this comment.
When the final .fxml or .css file is deleted, @SkipWhenEmpty makes this task finish with NO-SOURCE without invoking DesktopResourceCompiler.run() (Gradle documentation). The compiler's cleanup for stale FxmlDocuments.java and .cn1css files therefore never runs, while both output directories remain attached to the main source set, so an incremental build can continue compiling and packaging resources that no longer exist. Allow the empty-input invocation to run so it can clear its previous outputs.
Useful? React with 👍 / 👎.
| if (millis < 0) { | ||
| signal.get(); | ||
| } else { | ||
| signal.get((int) Math.max(1, Math.min(millis, Integer.MAX_VALUE))); |
There was a problem hiding this comment.
When a thread is blocked here on an incomplete future and another thread interrupts it, AsyncResource.get() ultimately waits through Util.wait, which catches InterruptedException and resumes waiting. Consequently CompletableFuture.get(), executor Future.get(), and CountDownLatch.await() cannot honor their declared interruption contract and may leave cancellation or shutdown code blocked until an unrelated completion occurs. Use an interrupt-aware wait path and propagate the interruption instead of relying on AsyncResource.get().
Useful? React with 👍 / 👎.
| declared = wireDesktopCompat(common, DesktopProjectImporter.moduleDir(source, module)); | ||
| r = new DesktopProjectImporter(MavenLog.of(getLog())).importProject(source, module, common, mainClass, |
There was a problem hiding this comment.
When entry-point selection later fails—for example, because the source project has multiple candidates and no main-class setting—wireDesktopCompat has already written compatibility and library dependencies into common/pom.xml. The command then reports that nothing was imported, but still leaves the destination project modified. Perform the non-mutating import validation before writing the POM, or restore the original POM when importProject fails.
Useful? React with 👍 / 👎.
| /// Every match from here on, each as its own result. | ||
| public Stream<MatchResult> results() { | ||
| List<MatchResult> all = new ArrayList<MatchResult>(); | ||
| reset(); |
There was a problem hiding this comment.
Calling reset() here makes results() restart at the beginning of the entire input and also discards any configured region. For example, after find() has consumed the first match, the JDK stream contains only the remaining matches, whereas this implementation includes the first match again; matcher.region(...).results() likewise searches outside the requested region. Build the results stream from the matcher's current state instead of resetting it.
Useful? React with 👍 / 👎.
| if (formatter == DateTimeFormatter.ISO_LOCAL_TIME) { | ||
| return ISO_TIME_PATTERN; | ||
| } | ||
| return ISO_DATE_TIME_PATTERN; |
There was a problem hiding this comment.
For every unremembered formatter except ISO_LOCAL_DATE and ISO_LOCAL_TIME, this fallback assumes the local date-time pattern. Consequently common calls such as DateTimeFormatter.ISO_INSTANT.withZone(zone) and ISO_OFFSET_DATE_TIME.withZone(zone) lose their Z/offset and fractional-second formatting and instead emit a plain local timestamp. Distinguish all supported ISO constants rather than mapping them to ISO_DATE_TIME_PATTERN.
Useful? React with 👍 / 👎.
| if (BASIC.indexOf((char) b) >= 0) { | ||
| throw new IllegalArgumentException("Illegal base64 line separator character 0x" | ||
| + Integer.toString(b, 16)); |
There was a problem hiding this comment.
A custom MIME separator containing = passes this validation because = is not in BASIC, even though it is Base64's padding character and the JDK rejects it. The encoder then inserts padding-looking bytes between complete groups; this implementation's own MIME decoder treats such an = at a group boundary as an invalid ending, so getMimeEncoder(4, new byte[]{'='}) can produce output that cannot be decoded. Include the padding character in the forbidden separator set.
Useful? React with 👍 / 👎.
| for (int i = KNOWN.size() - 1; i >= 0; i--) { | ||
| Known k = KNOWN.get(i); | ||
| DateTimeFormatter f = k.formatter.get(); | ||
| if (f == null) { | ||
| KNOWN.remove(i); |
There was a problem hiding this comment.
When formatters are created or used from multiple threads, known() iterates and removes from the shared ArrayList while remember() can concurrently append to it. A removal can shrink the list after another thread has chosen its index, causing get(i) to fail, while racing appends can lose a formatter's metadata and make later withZone or format calls use the wrong fallback behavior. Protect all accesses to KNOWN with one synchronization boundary.
Useful? React with 👍 / 👎.
| // Managed versions and plugin dependencies are not the module's. | ||
| String pom = text.replaceAll("(?s)<dependencyManagement>.*?</dependencyManagement>", " ") | ||
| .replaceAll("(?s)<build>.*?</build>", " "); | ||
| Matcher dep = POM_DEPENDENCY.matcher(pom); |
There was a problem hiding this comment.
When a POM keeps a dependency disabled inside <!-- ... -->, this regex still matches its <dependency> block because comments are never removed. librariesOf() can then wire that stale coordinate into the destination POM, so importing a project that Maven itself builds without the dependency may fail artifact resolution or bundle unwanted code. Strip XML comments or read the Maven model before scanning dependency elements.
Useful? React with 👍 / 👎.
- The importer reads POMs without their XML comments, and the import goal restores the POM when the import fails after wiring it. - JdkTime keeps its formatter table copy-on-write, and an ISO_ constant given a zone keeps its own offset, Z and fraction. - Base64 rejects '=' in a MIME line separator; Matcher.results() no longer resets the matcher. - Rebaseline hello RAM on macOS arm64, Windows x64 and Linux arm64: the hello application ships the compatibility runtimes, so the benchmark translates about 9,600 classes instead of 7,350. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
CodenameOne/maven/compat-jdk/src/main/java/com/codename1/compat/jdk/File.java
Lines 326 to 327 in ec52692
Honor deleteOnExit during orderly application exit
For every remapped desktop application, File.deleteOnExit() silently does nothing, so temporary files that callers expect to be removed during normal termination remain in the application sandbox across launches. This can steadily consume storage and retain sensitive temporary data even when the app terminates through the orderly JdkSystem.exit path, which already runs shutdown hooks. Record requested paths and delete them during that exit sequence instead of returning without registering any cleanup.
ℹ️ 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".
| if (used) { | ||
| throw new IllegalStateException(USED); | ||
| } | ||
| used = true; | ||
| return source; |
There was a problem hiding this comment.
When one stage closes the shared pipeline, only that stage's used flag is set. A downstream stage therefore still passes this check and consumes the source after its close handlers and active flat-mapped stream have already been released; for example, Stream.of(1).map(x -> x).close() followed by count() returns 1 instead of throwing IllegalStateException. Track the closed state in the shared StreamState and reject take() and further operations once any stage closes.
Useful? React with 👍 / 👎.
| kotlinRelocation(t -> new com.codename1.gradle.tasks.RemapCompatAction( | ||
| kotlinDir.get(), compileClasspath, onClickNames, true, null) | ||
| .withDesktopEntryRecord(desktopEntry).withApplicationMain(desktopMain) | ||
| .shippingWhenEmpty(javaSources).execute(t), |
There was a problem hiding this comment.
For a Gradle desktop module with Kotlin sources but no Java sources, compileJava is NO-SOURCE, so this relocation action deliberately becomes the full runtime-shipping pass via shippingWhenEmpty. Unlike the Java action, however, it never calls withApplicationLibraries(main.getRuntimeClasspath()); consequently CompatRemapper.bundleLibraries() sees applicationLibraries == null and skips unpacking and relocating implementation/runtime dependency jars. A Kotlin-only app that depends on a Swing or JavaFX library can therefore ship that library with unresolved desktop references or omit its relocated classes entirely.
Useful? React with 👍 / 👎.
| if (k == null || k.zone == null) { | ||
| return ZonedDateTime.parse(text, formatter); | ||
| } | ||
| return ZonedDateTime.of(localDateTimeFrom(formatter.parse(text)), k.zone); |
There was a problem hiding this comment.
When the formatter has an override zone and the input also contains an offset, this discards the parsed offset and reinterprets the local wall time in the override zone. For example, parsing 2020-01-01T12:00:00+02:00 with ISO_OFFSET_DATE_TIME.withZone(UTC) should produce 10:00Z, but this path produces 12:00Z, shifting the represented instant by two hours. Preserve the parsed instant or offset before converting it to the override zone.
Useful? React with 👍 / 👎.
| int d = hexDigit(c); | ||
| if (d < 0 || digits >= 16) { | ||
| throw new IllegalArgumentException("Invalid UUID string: " + name); |
There was a problem hiding this comment.
This applies a 16-digit limit independently to every UUID group even though their maximum widths are 8-4-4-4-12. As a result, malformed inputs such as 100000000-0-0-0-0 are accepted and then truncated by the masks below to the all-zero UUID, while UUID.fromString is required to reject them; this also lets multiple distinct untrusted strings collapse to the same identifier. Validate each group against its own width before accumulating it.
Useful? React with 👍 / 👎.
| public static ZonedDateTime plusDays(ZonedDateTime value, long days) { | ||
| return ZonedDateTime.of(value.toLocalDateTime().plusDays(days), value.getZone()); |
There was a problem hiding this comment.
When date-based arithmetic lands in a daylight-saving overlap, reconstructing the result with ZonedDateTime.of always selects the earlier valid offset instead of retaining the original offset when possible. For example, subtracting one day from 2024-10-28T02:30+01:00[Europe/Berlin] should produce the later +01:00 occurrence on October 27, but this returns the earlier +02:00 occurrence and changes the represented instant by an hour. Resolve the new local date-time using value.getOffset() as the preferred offset; the month and year helpers have the same issue.
Useful? React with 👍 / 👎.
…ry fixes from review - Adding days, months or years to a zoned value keeps the offset it had when the result falls in an hour the clocks go back over. - Text with an offset, parsed by a formatter with an override zone, is a moment shown in that zone, for the ISO constants and for patterns. - A stream stage made from a pipeline that was since closed is refused. - A Kotlin-only desktop module on Gradle bundles its runtime libraries and compiles its FXML, as the javac pass does. - UUID.fromString documents that an overlong group is truncated, which is what the JDK does, and a test holds it to that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What this adds
An existing Swing or JavaFX project builds as a Codename One application for every target, the way an Android module does with the Android compatibility layer. The sources are imported with
cn1:import-desktop-projectintocommon/src/main/desktopand built unmodified.maven/swing-compat:java.awt, Java2D,javax.swing,java.beans,javax.imageio, a SwingX subset and the FlatLaf entry points, written over Codename One components. Authored already relocated, because the JDK owns those package names; applications compile against the JDK's Swing.maven/javafx-compat: thejavafx.*subset (scene graph, controls, charts, layout panes, properties and bindings, animation, effects, Canvas), authored under the real names.maven/fxml-compiler: FXML and JavaFX CSS compiled at build time; nothing on a device parses XML or reflects.maven/compat-jdk: the JDK classes a device lacks (streams,java.util.function,java.nio.file, regex,CompletableFuture,java.timegaps and more), shared with the Android layer.maven/build-engine: one composed relocation pass over every layer, generated entry points and registries, records and pattern switches desugared, and the compliance check extended so that unsupported API is a build error reported under its original name and source line.scripts/desktop-compat-samples), screenshot tests inhellocodenameone, an integration test, a real-port interaction harness (maven/integration-tests/desktop-compat-realport) and a benchmark harness (scripts/desktop-compat-benchmarks).docs/developer-guide/Desktop-Interop.asciidoc, with measured size, memory and start-up tables.Verified locally
Not verified / known gaps
desktop-compat.ymladditions,desktop-compat-benchmarks.yml) have never run on GitHub. Windows is unmeasured; macOS runtime numbers needrun-macos-bench.sh.DesktopCompat*tests have to be seeded from the first CI run.🤖 Generated with Claude Code