Replace the Plexus container with the Sisu Plexus shim - #911
Merged
Conversation
plexus-container-default is the original Plexus container and cannot read META-INF/sisu indexes, so it blocks any move to JSR-330 annotations. Swap it for org.eclipse.sisu.plexus, which supplies PlexusContainer and PlexusTestCase and reads both component descriptors and annotated beans. Guice is "provided" in the Sisu POM, so it has to be declared explicitly. Nothing else changes yet: the shim reads META-INF/plexus/components.xml with classpath scanning off, so the existing descriptors keep working untouched. Drop version.sisu-maven-plugin, which pinned the managed Sisu artifacts to 0.9.0.M3 and was otherwise unused; maven-parent supplies 1.0.1, which is Java 8 bytecode just like 0.9.0.M3. WagonTestCase.customizeContext() went with it. The no-argument signature never overrode PlexusTestCase.customizeContext(Context) -- not in the Sisu shim and not in plexus-container-default either -- so the method has never been called and the "test.repository" value it set is read nowhere.
WagonTestCaseConfigurator took its per-use-case configuration as a nested <useCaseConfigs> tree inside the component descriptor. The Sisu shim cannot inject that: asking for a PlexusConfiguration fails to convert, and asking for a String yields only the element's text content, which is empty. Point the descriptor at a classpath resource instead and parse it on demand. A plain string injects cleanly, and the configuration itself is unchanged.
maven-scm-manager-plexus brings plexus-container-default 1.0-alpha-9, which puts a second copy of PlexusContainer and PlexusTestCase on the test classpath alongside the shim. The alpha-9 copy won, and its tearDown is not null-guarded, so every test failed with a NullPointerException that hid the real cause.
The container injects AbstractScmManager's provider map as an immutable map, so setScmProvider throws UnsupportedOperationException and every wagon-scm test that forces a particular provider implementation fails in setUp. Swap in a mutable copy first. It has to be done reflectively: the field is private with only a protected setter, and DefaultScmManager declares a field of the same name for the providers it has injected, so the copy has to target the field AbstractScmManager itself writes to.
slachiewicz
marked this pull request as ready for review
August 8, 2026 14:34
This was referenced Aug 8, 2026
slachiewicz
pushed a commit
that referenced
this pull request
Aug 8, 2026
The -Dssh-tests suite has been dead for years. Three faults kept the tests that use the embedded Apache MINA sshd from passing: * ShellCommand flushed the channel's output stream after calling ExitCallback.onExit, which closes it. The resulting SshChannelClosedException tore down the session, so the command after every executeCommand failed with "session is down". * The expected modification time on a get was compared at millisecond precision, but the scp "T" header carries whole seconds. * The SFTP subsystem was not registered on the embedded server. OpenSSH 9 and later drive scp over SFTP, so wagon-ssh-external's tests, which shell out to the host's scp, could not connect at all. ScpWagon now maps a missing file to ResourceDoesNotExistException when the server reports it with the scp fatal-error code (2) and says so in the message, not only with the warning code (1) that OpenSSH uses. CI passes -Dssh-tests -Dssh-embedded=true, which runs the 42 embedded jsch tests. This is the 3.x change from #904 without its fourth part. There, a TestPrompter and a test component descriptor had to be supplied by hand, because plexus-interactivity-api stopped shipping META-INF/plexus/components.xml in 1.3 and plexus-container-default could see neither the sisu index nor DefaultPrompter's constructor injection. Since #911 the tests run on the Sisu shim, which reads that index and satisfies the constructor, so the real DefaultPrompter resolves and the workaround is not needed here. Verified: 42 tests, 0 failures without it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Groundwork for moving Wagon off Plexus and onto JSR-330. This PR only swaps the
container; the
@plexus.componentjavadoc tags are untouched and follow in asecond PR.
plexus-container-defaultis the original Plexus container and cannot readMETA-INF/sisuindexes, so nothing can become@Namedwhile it is in place.org.eclipse.sisu.plexussuppliesPlexusContainerandPlexusTestCaseand readsboth component descriptors and annotated beans, so it is the natural replacement.
It reads
META-INF/plexus/components.xmlwith classpath scanning off, which is whythis PR stands alone: every existing descriptor keeps working untouched.
Why now
maven-parent49 bansplexus-container-defaultin itsdrop-legacy-dependenciesprofile. That profile has no activation today, so
masterstill builds for CI --this is preparation for the ban becoming unconditional, not a fix for a broken build.
The swap was not a drop-in
Four behavioural differences between the shim and the original container turned up
only by building the whole reactor, and each has its own commit:
WagonTestCaseConfiguratortookits per-use-case configuration as a
<useCaseConfigs>tree inside the descriptor.Asking for a
PlexusConfigurationfails to convert; asking for aStringyieldsonly the element's text content, which is empty. The configuration now lives in its
own classpath resource, with the descriptor holding a plain string path. The
configuration itself is unchanged. This changes how TCK consumers configure use
cases, which is the one deliberate compatibility break here -- all three in-tree
consumers are updated.
ComponentConfiguratoris registered.lookup(ComponentConfigurator.ROLE)fails;
BasicComponentConfiguratorships in the Sisu jar and is now instantiateddirectly.
maven-scm-manager-plexusdrags inplexus-container-default1.0-alpha-9, puttinga second
PlexusContainer/PlexusTestCaseon the wagon-scm test classpath. Thealpha-9 copy won and its
tearDownis not null-guarded, so every test failed with anNPE that hid the real cause. Now excluded.
AbstractScmManager.setScmProviderdoes aputon its provider map, so every wagon-scm test that forces a particular providerimplementation failed in
setUp. The test now installs a mutable copy first.WagonTestCase.customizeContext()is deleted rather than adapted: the no-argumentsignature never overrode
PlexusTestCase.customizeContext(Context)in either container,so it has never been called and the value it set is read nowhere.
Status
Full reactor green, all 17 modules, including the HTTP TCK (40 tests, plus the 2
correctly skipped as unsupported) and wagon-scm.
Not for
wagon-3.x-- this belongs to the 4.0.0 line.