perNyfelt opened a new pull request, #163:
URL: https://github.com/apache/maven-resolver-ant-tasks/pull/163
Builds on #162 by @cstamas (its three commits are included unchanged) and
adds fixes for the points that came up reviewing it. If #162 is merged first,
the last three commits here apply on top of it just as well.
### Review fixes on top of #162
**Resolver 2 sessions are never closed** — `SessionBuilder.build()` returns
a `CloseableSession`, and `DefaultRepositorySystemLifecycle` holds per-session
state (on-session-ended handlers keyed by session id, cached repository
connectors and their transports) until it is closed. The five session creation
sites now use try-with-resources, and the `RepositorySystem` is shut down when
the Ant build that owns it finishes, via a `BuildListener` matched against its
own project. A one-shot `ant` CLI process hid this, but it leaked per task
invocation with Ant embedded (Groovy `AntBuilder` as in example7, IDEs, Gradle).
**Mixed Maven versions on the classpath** — `maven-resolver-supplier-mvn3`
brings its own `maven-model-builder` (3.9.16), so overriding only
`maven-resolver-provider` left model-builder/maven-artifact 3.9.16 next to
maven-model and maven-repository-metadata 3.10.0-rc-1. `maven-model-builder` is
now declared at `${mavenVersion}`; `AntRepoSys` uses
`org.apache.maven.model.building` directly, so it should be a declared
dependency in any case rather than one inherited from the supplier.
**`AntRepositorySystemSupplier` is redundant in resolver 2** —
`getModelBuilder()` and `getRemoteRepositoryManager()` are `public final` on
`RepositorySystemSupplier`, so subclassing to stash them in fields is no longer
needed. Reading those fields also NPEd if the component had not been created
during `get()`; the lazy getters cannot.
**Smaller things** — `getLocalRepoMan()` renamed to `getLocalRepo()` (it
returns a `LocalRepository`, not a manager, since the upgrade); slf4j version
moved to a `${slf4jVersion}` property and the comment explaining `slf4j-nop`
restored; a test covering the stable `WorkspaceRepository` (it gets a random
key when none is given, so a fresh instance per `getRepository()` call defeats
caching keyed on it).
### Open questions for #162 that are not addressed here
- `mavenVersion` is `3.10.0-rc-1`. Fine on master, but a 1.6.2 release cut
before 3.10.0 GA would ship an RC in its dependency set and uber jar — a
release-timing call for the RM.
- `.mvn/placeholder.txt` is deleted, which removes the `.mvn` directory.
Unrelated to the resolver upgrade and harmless in the build; left as is in case
it was deliberate.
- `ComponentsXmlResourceTransformer` is now a no-op (nothing in the shaded
set ships a `components.xml`), but it is left in place as a harmless guard.
`SisuIndexResourceTransformer` still has input.
### Verification
- `mvn verify`: 50 tests pass, rat/checkstyle/enforcer clean. CI on #162 was
already green on JDK 8/17/21/25 across three OSes; every new dependency is
still Java 8 bytecode.
- Uber jar exercised end to end with `ant`: examples 1–6, plus the
`installAndClean` targets of examples 3 and 5.
- Nested `<ant>` sub-build checked explicitly: a parent build that resolves,
runs a sub-build that resolves, then resolves again after the sub-build
finished — the parent's repository system survives the sub-build.
---
- [x] Your pull request should address just one issue, without pulling in
other changes.
- [x] Write a pull request description that is detailed enough to understand
what the pull request does, how, and why.
- [x] Each commit in the pull request should have a meaningful subject line
and body.
- [x] Write unit tests that match behavioral changes, where the tests fail
if the changes to the runtime are not applied.
- [x] Run `mvn verify` to make sure basic checks pass.
- [ ] You have run the integration tests successfully (`mvn -Prun-its
verify`). — no such profile here; the `examples/` builds were run against the
uber jar instead.
- [ ] I hereby declare this contribution to be licenced under the [Apache
License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
--
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]
