slachiewicz opened a new pull request, #1677:
URL: https://github.com/apache/maven-dependency-plugin/pull/1677
Removes `maven-compat`. No baseline change — the pom is already maven 3.9.16
/ resolver 1.9.25, so `<prerequisites>` is untouched.
`GetMojo`'s constructor **loses** a parameter: the legacy
`org.apache.maven.repository.RepositorySystem` was only ever used for
`injectMirror`, `injectProxy` and `injectAuthentication`.
### Not via `MavenRepositorySystem` — it is not visible to plugins
That swap was made first and it does not work. maven-core does not export
`org.apache.maven.bridge` to plugin class realms. In plexus-classworlds, a
plain package name matches **recursively** while the `.*` form matches only
classes directly in that package — so maven-core exports
`org.apache.maven.artifact.**`, `org.apache.maven.repository.**`,
`org.apache.maven.settings.**`, `org.apache.maven.RepositoryUtils` and
`org.eclipse.aether.**`, and nothing under `org.apache.maven.bridge`. Same in
3.9.16 and 4.0.0-rc-5.
It compiled and 412 unit tests passed. It failed only under `-Prun-its`:
```
Failed to execute goal ...:get (default-cli) on project get-gav:
A required class was missing while executing ...:get:
org/apache/maven/bridge/MavenRepositorySystem
```
So for changes touching maven-core internals, `mvn verify` is not a
sufficient bar — the plugin-testing harness uses one flat classpath and has no
realm isolation.
Selection now lives in the plugin, in a new `RepositorySessionInjector`.
That is viable because the session-based inject methods need no privileged API:
they only read `session.getMirrorSelector()` / `getProxySelector()` /
`getAuthenticationSelector()` and write onto the `ArtifactRepository`.
Two details replicated from maven-core rather than assumed:
`createArtifactRepository` substitutes `new ArtifactRepositoryPolicy()` for
null policies — a naive `new MavenArtifactRepository(...)` would reintroduce
the null-policy trap — and it honours
`ArtifactRepositoryLayout2.newMavenArtifactRepository`.
### Proxy and authentication: preserved, with three narrow deltas
**Mirrors are unchanged.** maven-core's `getMirror(ArtifactRepository,
List<Mirror>)` and resolver's `DefaultMirrorSelector` were compared directly:
same two-pass lookup, same `matchPattern` handling of `*`, `external:*`,
`external:http:*`, `!id` and comma lists, same layout matching.
**Proxies — three differences, all moving toward what the rest of Maven
already does:**
1. **An `http` proxy now also applies to `https` repositories** when no
`https` proxy is defined. The legacy `equalsIgnoreCase` did not fall back, so a
repository that previously bypassed the proxy will now be proxied. **This is
the release-note item.**
2. `dav:` / `davs:` are normalised to http/https before matching; the legacy
code matched the literal string and never matched.
3. `nonProxyHosts` matching is now case-insensitive. Same `|` splitting and
`.`/`*` escaping.
**Authentication — two differences, both inert:** a `<server>` with no
credentials yields `null` rather than `Authentication(null, null)`, which
`RepositoryUtils.toAuthentication` collapses to null anyway; and duplicate
`<server>` ids go first-wins to last-wins.
Suggested release note: *"`dependency:get` now selects proxies through the
repository session, so an `http` proxy also applies to `https` repositories
when no `https` proxy is configured, matching how Maven proxies every other
download."*
### The existing auth test was not testing anything
`test:test:1.0` persists in the checked-in
`target/test-classes/unit/get-test/target/local-repo`, so without `clean` the
test passed **with authentication injection deleted outright**. Fixed with a
`@TempDir` local repository. After that, deleting the inject call fails all
three of `testRemoteRepositoriesAuthentication`, `testRemoteRepositoriesProxy`
and `testRemoteRepositoriesNonProxyHosts` — and the proxy test asserts the
failure names the proxy host, so it cannot pass for the wrong reason.
The new tests were also run against **unmodified master's** settings-list
implementation: 6/6 pass. Same tests, both implementations, same outcomes — so
they characterise behaviour rather than merely agreeing with the new code.
### Numbers
| | unit | ITs |
|---|---|---|
| baseline `origin/master` | 410 / 0 / 0 / 1 skip | 99 passed, 0 failed |
| after | 420 / 0 / 0 / 1 skip | 99 passed, 0 failed |
Full 94-project IT suite both sides, per-IT diff empty across all 86
reported projects, `dependency:analyze-only` clean.
### One judgement call
`SnapshotArtifactRepositoryMetadata` is vendored into the test tree rather
than deleting its two usages. `TestCopyDependenciesMojo2.assertArtifactExists`
iterates `artifact.getMetadataList()` and asserts a file exists — dropping the
attachment makes that loop empty and silently removes the check. Vendoring is
safe here: the whole hierarchy above it is in maven-core and only the leaf is
compat-only. Same approach as MPLUGIN-384.
--
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]
