slachiewicz opened a new issue, #49:
URL: https://github.com/apache/maven-executor/issues/49
### New feature, improvement proposal
Grounded in six integration-test suites migrated off maven-verifier on
2026-09-21 (apache/maven-ear-plugin#556,
apache/maven-remote-resources-plugin#303, apache/maven-apache-resources#131,
apache/maven-surefire#3484 and #3485, apache/maven-build-cache-extension#536,
apache/maven-integration-testing#445 and #446; about 1,500 test files, about
2,100 lines of helper code written six times over), the executor issues those
migrations produced (#44, #45, #46, #47), the guide corrections in
apache/maven-verifier#217, and a source survey of the five runtime consumers of
maven-invoker (deprecated by apache/maven-invoker#164, last release 3.3.0).
Line references are to tag `executor-1.0.0`.
## P1 — blocks migrations
1. **Embedded Maven 4 must not hang, and hangs must fail (#47).** All three
`Mode.AUTO` consumers hung on every 4.0.0-rc-6 cell and were cancelled at the
CI timeout; all now force `FORKED`. Start with what
`ToolboxExecutorTool.mojo()` already does for 4.x (`--raw-streams`, line 129)
plus a closed stdin, and honour `executionTimeout` in embedded mode:
`EmbeddedMavenExecutor.execute()` never reads it, only
`ProcessBuilderExecutorSupport` does (lines 79-95). `Mode.AUTO` should refuse
embedded when it cannot work rather than select it; maven-verifier's silent
fork-on-failure is what kept these suites green for two years.
2. **Local-repository and artifact paths without a plugin (#44).** The only
`ExecutorTool` runs `eu.maveniverse.maven.plugins:toolbox` through the Maven
under test; the core IT suite alone has about 440 path call sites
(`deleteArtifacts` 302, `verifyArtifactPresent` 84, `getArtifactPath` 29,
metadata 28). Ship a default tool that resolves the repository
(`-Dmaven.repo.local` argument, then the system property, then settings, then
`~/.m2/repository`) and computes default-layout artifact and metadata paths.
Keep the toolbox tool for `dump()` and opt-in use.
## P2 — core correctness, each hit by more than one port
3. **Stream ownership (#45).** Pump threads close caller streams
(`ProcessBuilderExecutorSupport.java:124,137`); embedded does not; one sink for
both streams races two closes. Flush, never close, and state that `stdOut()`
and `stdErr()` may be the same object.
4. **Lifetime (#46).** The constructor-time `System` snapshot forces a
JVM-lifetime executor; a per-execution executor exhausts the heap after about
100 builds (apache/maven-surefire#3484, run 35594937350) even though `close()`
does dispose realms (`EmbeddedMavenExecutor.java:189-193`). Snapshot in
`execute()`, and make `close()` reclaim enough to allow per-test executors.
5. **`toBuilder()` throws on `argument()` (#16 item 1).** `Impl.arguments()`
is unmodifiable (`ExecutorRequest.java:394-396`), `Builder(List)` stores it
(`:221`), `argument()` calls `add` (`:239`). This blocks the
one-template-request-per-test-class pattern.
6. **`MAVEN_ARGS` leaks into the forked child (#16 item 2).** `env.remove`
acts on the local map (`ForkedMavenExecutor.java:143`) while `pb.environment()`
inherits the parent's; a CI runner that sets it changes every IT's arguments.
7. **`Builder.logFile(Path)`** tee-ing both streams in arrival order,
compatible with `grabOutputAsString`. Six ports wrote this; build-cache reads
`log.txt` from 13 files, surefire's `OutputValidator` and the core ITs'
`loadLogLines` read it too.
8. **`userProperty(k, v)`** appending `-Dk=v`, with Javadoc on
`jvmSystemProperty` saying it lands in `MAVEN_OPTS`
(`ForkedMavenExecutor.java:125-141`). Three ports and the guide's first version
mapped `setSystemProperty` to the wrong one.
## P3 — test-support module, so the seventh port writes nothing
9. **`maven-executor-verifier`**: Java 8, depends only on core. Autoclean
and `-e -B` defaults, `verifyErrorFreeLog` (`[ERROR]` scan minus Velocity
noise), `verifyTextInLog`/`NotInLog`, `verifyFile*`,
`verifyArtifact*`/`deleteArtifacts` on item 2,
`loadLines`/`loadProperties`/`filterFile`/`newDefaultFilterMap`,
`ResourceExtractor`, a per-execution log file. Seed it from
apache/maven-integration-testing#445 (Java 8, toolbox-free), not from
apache/maven master's helper (Java 17, toolbox-bound).
10. **JUnit 5 extension (#20)** shaped like build-cache's
`IntegrationTestExtension`: per-test project copy, injected Verifier, log in
the test directory. That shape moved 130 test files with zero edits.
11. **Installation discovery** behind an explicit factory, never silent.
`discoverInstallationDirectory()` throws without `maven.home`, and three of
four plugin ports had to forward it to Failsafe through
`systemPropertyVariables`.
## P4 — runtime replacement for maven-invoker
Five consumers embed maven-invoker at compile scope to launch Maven from
inside a running Maven: maven-invoker-plugin (`AbstractInvokerMojo`),
maven-archetype (`IntegrationTestMojo`, `CreateProjectFromArchetypeMojo`,
`FilesetArchetypeCreator`), maven-release (`InvokerMavenExecutor`),
maven-javadoc-plugin (`JavadocUtil`); maven-gpg-plugin uses it at test scope
(`InvokerTestUtils`). The union of what they call, verified per file:
| Invoker API | maven-executor 1.0.0 | Status |
|---|---|---|
| `setBaseDirectory` / `setPomFile` | `cwd(Path)` / `-f` argument | direct |
| `setGoals`, `addArg(s)`, `setProperties` | `arguments(...)`, `-Dk=v` |
composable; item 8 |
| `setBatchMode`, `setDebug`, `setOffline`, `setShowErrors`,
`setShowVersion`, `setUserSettingsFile`, `setGlobalSettingsFile`,
`setToolchainsFile`, `setGlobalToolchainsFile` | `-B -X -o -e -V -s -gs -t -gt`
arguments | composable; a typed builder would stop nine ports re-deriving the
flags |
| `setLocalRepositoryDirectory` (all five) | `-Dmaven.repo.local=` |
composable |
| `setMavenHome` (invoker-plugin, release, javadoc, gpg) |
`ForkedMavenExecutor(Path)` | direct |
| `setJavaHome` (invoker-plugin, release, javadoc); `setMavenOpts` (javadoc)
| `environmentVariable("JAVA_HOME"/"MAVEN_OPTS", ...)`, as invoker does
(`MavenCommandLineBuilder.java:191,195`) | composable |
| `addShellEnvironment` (release, gpg) | `environmentVariable` | direct |
| `setInputStream(System.in)` (release interactive prompts, gpg) |
`stdIn(InputStream)` | direct in forked; #45 must not close `System.in` |
| `setOutputHandler` / `setErrorHandler` (all five, line-oriented) | raw
`OutputStream` only | **missing**: a line consumer (`Consumer<String>`) so mojo
logs stay line-buffered; today each port writes a splitting stream |
| `setTimeoutInSeconds` (gpg) | `executionTimeout(Duration)` | direct,
forked only |
| `setMavenExecutable` (`mvnw` relative to basedir,
`AbstractInvokerMojo.java:514`; resolution in
`MavenCommandLineBuilder.java:513-527`) | none: forked always runs
`<installation>/bin/mvn` (`ForkedMavenExecutor.java:104-106`) | **missing**:
`forExecutable(Path)`; the invoker plugin's `invoker.mavenExecutable` and
wrapper-based ITs have no path otherwise |
| `Invoker.setLogger` | none | missing, low value; a `Consumer<String>`
debug hook suffices |
| `getExitCode()` / `getExecutionException()` (launch failure vs build
failure; release `:121-129`, invoker-plugin `:2176-2184`) | `exitCode()` /
unchecked `ExecutorException` | equivalent; document that `ExecutorException`
is the launch-failure channel |
Two runtime facts set the constraints. The outer `mvn` script passes
`-Dmaven.home` (`bin/mvn:323`), so the running installation is discoverable
without configuration. Embedded mode inside a running Maven is not viable:
`EmbeddedMavenExecutor` replaces `System` properties, `System.out` and the
context classloader for the duration (`:149-162, 237-239, 289, 330`), which is
a second Maven mutating the first's JVM state. The runtime story is therefore
forked only; say so in Javadoc and never let `Mode.AUTO` pick embedded when
`maven.home` equals the current process's.
Runtime priorities: (a) `forExecutable(Path)` and `mvnw` support, without
which maven-invoker-plugin cannot migrate; (b) line-oriented output consumers;
(c) a `stdIn` that is not closed (#45); (d) a typed `MavenRequest.Builder` with
the flag conveniences from the table; (e) confirmed `executionTimeout` and
`destroyForcibly` behaviour, for a plugin that must not orphan a child on
Ctrl-C.
## Where things live
Runtime conveniences (a)-(e) belong in core: they are executor concerns,
dependency-free, and the plugins cannot take a test module at compile scope.
The Verifier-shaped helpers (P3) belong in a separate module in this
repository, not in a maven-verifier 3.0: apache/maven-verifier#186 exists to
close that repository; the README already plans a 2.x line "with extra features
on top of the executors"; the helpers version with `ExecutorHelper` and item 2;
and apache/maven's own ITs would consume it.
Not recommended: helpers in the core artifact; the toolbox tool as the
default path provider in an ASF test artifact; a `verifier.forkMode`-style
system property in place of the explicit `Mode` API.
## Sequencing
- 1.0.1: items 1, 3, 5, 6; runtime (c).
- 1.1: items 2, 4, 7, 8; runtime (a), (b), (d).
- 2.x: items 9, 10, 11.
Items 2 and 9 together let the core ITs and build-cache delete their in-repo
Verifier copies. Runtime (a) and (b) let maven-invoker-plugin drop
maven-invoker, which is the migration that retires that repository.
Happy to split this into one issue per item once the shape is agreed.
--
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]
