etzel-mes commented on code in PR #29353:
URL: https://github.com/apache/camel/pull/29353#discussion_r4497149918
##########
dsl/camel-jbang/camel-jbang-run/src/main/java/org/apache/camel/dsl/jbang/run/export/SpringBootExporter.java:
##########
@@ -434,8 +439,8 @@ protected String getDockerfileTemplateName() {
}
@Override
- protected Set<String> resolveDependencies(Path settings, Path profile)
throws Exception {
- Set<String> answer = super.resolveDependencies(settings, profile);
+ protected Set<String> doResolveDependencies(Path settings, Path profile)
throws Exception {
Review Comment:
Done, in both places, with `hasCamelArtifact` and
`camel-platform-http-vertx` kept as you asked. No need to apologise — the
helper is there, so it belongs in this PR.
Three things from doing it.
**One, the prefix breadth.** The old check was a prefix match, so it counted
`camel-platform-http-main` and `camel-platform-http-jolokia` as a server too;
the two artifacts you named do not. I expected that to change an export and
went looking for one: there is none. A run with `--console` records all four
together, so the component is in the set whenever `-main` and `-jolokia` are:
```
camel.jbang.console=true
dependency=mvn\:org.apache.camel\:camel-platform-http-main\:4.24.0-SNAPSHOT
dependency=mvn\:org.apache.camel\:camel-platform-http\:4.24.0-SNAPSHOT
dependency=mvn\:org.apache.camel\:camel-platform-http-vertx\:4.24.0-SNAPSHOT
dependency=mvn\:org.apache.camel\:camel-platform-http-jolokia\:4.24.0-SNAPSHOT
```
So the check is narrower in principle — a set with `-main` or `-jolokia` but
without the component would now get `camel.main.run-controller=true` — and no
export the recorded run produces is such a set. The commit message says this.
**Two, the test does not distinguish the notations**, and I could not write
one that does. With `--dep=camel-platform-http` the silent run also records
`mvn:org.apache.camel:camel-platform-http:<version>`, so the old string check
finds the artifact as well and the test passes before the change too; no option
skips the silent run, so the short notation never reaches the exporter on its
own in an export test. The same holds for
`shouldReplacePlatformHttpInAnyNotation` of the first commit. Both still have
teeth for the detection itself — the new one fails with `boolean http = false`
— and they pin the behaviour of an export with platform-http; the notation half
of both lines is provable by reading rather than by a test. Tell me if you want
it covered another way.
**Three, two checks of the same kind are left**, on purpose, and I would
rather name them than let the next reader find them:
- `CamelMainExporter`, five lines above the three the first commit changed:
`if (!main && answer.stream().anyMatch(s ->
s.contains("camel-platform-http")))`. It misses `camel:platform-http`, but its
substring also matches `-vertx` and `-jolokia`, and for camel-main those should
switch to the main server. An exact artifact comparison would change that,
which is more than this ticket.
- `Exporter.hasOpenapi`:
`s.contains("mvn:org.apache.camel:camel-rest-openapi")`, so one side of the
very `if` I changed in `SpringBootExporter` compares artifacts and the other
compares strings. The swap is safe here, but it has the same test limitation as
above — the recorded run always holds the `mvn:` form — so I did not want to
put an unprovable behaviour change in this PR.
Happy to open an issue for the pair, or to do either here if you prefer.
For the JDK 21 guard I used `Assumptions.assumeTrue` with the same condition
as `runtimeProvider`, since a single test cannot take the guard from a
provider: a parameterized test with an empty argument stream fails instead of
skipping.
One aside, unrelated to this PR: `ExportTest.createCommand` calls
`CommandLineTestHelper.parse` several times, and `CommandLine.populateCommand`
sets the defaults of the options a call does not give, so a later call resets
`--dir`. A test that does not repeat `--dir` in its own arguments exports into
`camel-jbang-core` itself and overwrites the module's `pom.xml` and `AGENTS.md`
(it did here; git restored them). `createCommand` also hands the files as
`Arrays.asList`, which the auto-detected `application.properties` of a profile
cannot be added to, so such a test fails with `UnsupportedOperationException`
in `ProjectExporter.prepare`. I can file that too.
`camel-jbang-run` (257 tests) and the whole `ExportTest` (158) pass.
_Claude Code on behalf of etzel-mes_
##########
dsl/camel-jbang/camel-jbang-run/src/main/java/org/apache/camel/dsl/jbang/run/common/ExportHelper.java:
##########
@@ -216,6 +216,31 @@ public static String exportPackageName(String groupId,
String artifactId, String
return sb.toString();
}
+ /**
+ * Keeps a single dependency per groupId:artifactId in the dependencies of
a pom.xml, which may name an artifact
+ * only once (Maven 3.10 onwards fails the build of a project that names
one twice). Two dependencies with other
+ * coordinates become the same artifact through the mapping of a runtime:
{@code org.apache.camel:camel-console} and
+ * {@code org.apache.camel.quarkus:camel-quarkus-console} are the same
artifact of the Quarkus runtime. The first is
+ * kept, in its position, except that a dependency with a version wins
over one without: the mapping of a runtime
+ * leaves the version of its own artifacts to the BOM, so the coordinates
a user gave with a version survive it.
+ */
+ public static void removeDuplicateArtifacts(List<MavenGav> gavs) {
+ Map<String, MavenGav> kept = new LinkedHashMap<>();
+ for (MavenGav gav : gavs) {
+ String ga = gav.getGroupId() + ":" + gav.getArtifactId();
+ MavenGav existing = kept.get(ga);
+ if (existing == null || !hasVersion(existing) && hasVersion(gav)) {
Review Comment:
Taken, in the last commit (`CAMEL-26697: camel-jbang - Parentheses in the
duplicate removal`). The condition was the intended one, as you say, and the
parentheses mean a reader does not have to apply the precedence rules — and
keep the mix out of the static-analysis reports. The rule is unchanged, and the
two `ExportHelperTest` cases still cover it.
_Claude Code on behalf of etzel-mes_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
