Restore Maven 2 compatibility - revert "Drop deprecated package org.codehaus.classworlds"#145
Open
slachiewicz wants to merge 4 commits intomasterfrom
Open
Restore Maven 2 compatibility - revert "Drop deprecated package org.codehaus.classworlds"#145slachiewicz wants to merge 4 commits intomasterfrom
slachiewicz wants to merge 4 commits intomasterfrom
Conversation
This reverts commit ff94d8a
…ied Sisu dependency org.eclipse.sisu:org.eclipse.sisu.plexus references ClassRealm, ClassRealmAdapter, and ClassRealmReverseAdapter from the legacy package in compiled bytecode (ClassRealmConverter in sisu.plexus). Removing the package breaks all Maven 3+ builds at runtime with ClassNotFoundException. PR #141 removed this package and had to be reverted. Add COMPATIBILITY.md documenting the exact Sisu dependency, what preconditions must be met before any future removal, and update Javadoc and README accordingly. - Enhanced Javadocs for all classes in org.codehaus.classworlds - Added detailed compatibility warnings and pointers to modern replacements - Fixed accidental removal of imports in DefaultClassRealm - Corrected inheritance for ConfigurationException to restore binary compatibility - Add package-info.java with compatibility warnings for Maven 2/Sisu - Add deprecation notices and usage notes to key legacy classes - Fix StackOverflowError in legacy Configurator due to recursive adapter calls - Add LegacyCompatibilityTest to ensure basic functionality of the legacy layer - Update README.md with backward compatibility information
There was a problem hiding this comment.
Pull request overview
This PR restores legacy Maven 2 / Classworlds 1.x binary compatibility by reintroducing the deprecated org.codehaus.classworlds API surface and documenting why it must remain available for Eclipse Sisu (and therefore Maven 3+).
Changes:
- Re-add the
org.codehaus.classworldscompatibility layer (adapters, legacy exceptions, wrappers). - Extend test coverage to validate legacy behaviors (e.g., leading-slash resource lookups) and basic compatibility smoke tests.
- Update build configuration and documentation to export/describe the legacy package.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/org/codehaus/plexus/classworlds/realm/DefaultClassRealmTest.java | Adds assertions covering legacy leading-slash resource behavior via the compatibility adapter. |
| src/test/java/org/codehaus/plexus/classworlds/launcher/LauncherTest.java | Sets protocol handler system property for launcher tests. |
| src/test/java/org/codehaus/classworlds/LegacyCompatibilityTest.java | Adds smoke tests to ensure legacy API types can be instantiated and used. |
| src/site/xdoc/index.xml | Documents the legacy compatibility layer and why it must remain. |
| src/main/java/org/codehaus/classworlds/package-info.java | Adds package-level documentation and deprecation notice for the legacy namespace. |
| src/main/java/org/codehaus/classworlds/ClassRealm.java | Reintroduces the legacy ClassRealm interface referenced by Sisu bytecode. |
| src/main/java/org/codehaus/classworlds/ClassRealmAdapter.java | Adds new→old adapter implementation (legacy facade over modern realm). |
| src/main/java/org/codehaus/classworlds/ClassRealmReverseAdapter.java | Adds old→new adapter implementation (modern facade over legacy realm). |
| src/main/java/org/codehaus/classworlds/ClassWorld.java | Reintroduces the legacy ClassWorld wrapper API. |
| src/main/java/org/codehaus/classworlds/ClassWorldAdapter.java | Adds new→old adapter for ClassWorld. |
| src/main/java/org/codehaus/classworlds/ClassWorldReverseAdapter.java | Adds old→new adapter for ClassWorld. |
| src/main/java/org/codehaus/classworlds/DefaultClassRealm.java | Adds legacy DefaultClassRealm wrapper implementation. |
| src/main/java/org/codehaus/classworlds/Launcher.java | Adds legacy Launcher wrapper around the modern launcher. |
| src/main/java/org/codehaus/classworlds/Configurator.java | Adds legacy Configurator wrapper around the modern configurator. |
| src/main/java/org/codehaus/classworlds/ConfiguratorAdapter.java | Adds adapter class intended to bridge configurator behavior. |
| src/main/java/org/codehaus/classworlds/ClassWorldException.java | Reintroduces legacy base exception type. |
| src/main/java/org/codehaus/classworlds/DuplicateRealmException.java | Reintroduces legacy exception type. |
| src/main/java/org/codehaus/classworlds/NoSuchRealmException.java | Reintroduces legacy exception type. |
| src/main/java/org/codehaus/classworlds/ConfigurationException.java | Reintroduces legacy exception type. |
| src/main/java/org/codehaus/classworlds/BytesURLConnection.java | Reintroduces legacy byte-array URLConnection helper. |
| src/main/java/org/codehaus/classworlds/BytesURLStreamHandler.java | Reintroduces legacy byte-array URLStreamHandler helper. |
| README.md | Adds a section describing the compatibility layer and constraints. |
| pom.xml | Updates bundle exports and test JVM args to include the legacy package. |
| COMPATIBILITY.md | Adds a detailed explanation and “rules of engagement” for the legacy package. |
Comments suppressed due to low confidence (1)
src/test/java/org/codehaus/plexus/classworlds/launcher/LauncherTest.java:45
java.protocol.handler.pkgsis a global JVM property and this test overwrites it without restoring the previous value, which can leak into other tests (and into user environments when running subsets). Save the previous value insetUp()and restore/clear it intearDown(); also consider appending to the existing value instead of replacing it.
@BeforeEach
void setUp() {
System.setProperty("java.protocol.handler.pkgs", "org.codehaus.classworlds.protocol");
this.launcher = new Launcher();
this.launcher.setSystemClassLoader(Thread.currentThread().getContextClassLoader());
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+61
to
+68
| /** set world. | ||
| * this setter is provided so you can use the same configurator to configure several "worlds" | ||
| * | ||
| * @param world The classWorld to configure. | ||
| */ | ||
| public void setClassWorld(ClassWorld world) { | ||
| config.setClassWorld(world); | ||
| } |
Comment on lines
+104
to
+136
| protected void loadGlob(String line, ClassRealm realm) throws MalformedURLException, FileNotFoundException { | ||
| loadGlob(line, realm, false); | ||
| } | ||
|
|
||
| /** | ||
| * Load a glob into the specified classloader. | ||
| * | ||
| * @param line The path configuration line. | ||
| * @param realm The realm to populate | ||
| * @param optionally Whether the path is optional or required | ||
| * @throws MalformedURLException If the line does not represent | ||
| * a valid path element. | ||
| * @throws FileNotFoundException If the line does not represent | ||
| * a valid path element in the filesystem. | ||
| */ | ||
| @SuppressWarnings("RedundantThrows") | ||
| protected void loadGlob(String line, ClassRealm realm, boolean optionally) | ||
| throws MalformedURLException, FileNotFoundException { | ||
| config.loadGlob(line, realm, optionally); | ||
| } | ||
|
|
||
| /** | ||
| * Filter a string for system properties. | ||
| * | ||
| * @param text The text to filter. | ||
| * @return The filtered text. | ||
| * @throws ConfigurationException If the property does not | ||
| * exist or if there is a syntax error. | ||
| */ | ||
| @SuppressWarnings("RedundantThrows") | ||
| protected String filter(String text) throws ConfigurationException { | ||
| return config.filter(text); | ||
| } |
| super(false); | ||
| this.config = config; | ||
| } | ||
|
|
Comment on lines
+19
to
+37
| /** | ||
| * A compatibility wrapper for {@link org.codehaus.plexus.classworlds.realm.ClassRealm} | ||
| * provided for legacy code. | ||
| * | ||
| * <p><b>Note:</b> This is a legacy class provided for backward compatibility with Maven 2. | ||
| * New code should use {@link org.codehaus.plexus.classworlds.realm.ClassRealm}.</p> | ||
| * | ||
| * @author Andrew Williams | ||
| * @deprecated Use {@link org.codehaus.plexus.classworlds.realm.ClassRealm} | ||
| */ | ||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.net.URL; | ||
| import java.util.Enumeration; | ||
|
|
||
| @SuppressWarnings("rawtypes") | ||
| @Deprecated | ||
| public class DefaultClassRealm implements ClassRealm { |
Comment on lines
+34
to
+44
| public class ClassWorldReverseAdapter extends org.codehaus.plexus.classworlds.ClassWorld { | ||
| private static final HashMap instances = new HashMap(); | ||
|
|
||
| public static ClassWorldReverseAdapter getInstance(ClassWorld oldWorld) { | ||
| if (instances.containsKey(oldWorld)) return (ClassWorldReverseAdapter) instances.get(oldWorld); | ||
|
|
||
| ClassWorldReverseAdapter adapter = new ClassWorldReverseAdapter(oldWorld); | ||
| instances.put(oldWorld, adapter); | ||
|
|
||
| return adapter; | ||
| } |
| /** | ||
| * Adds a byte[] class definition as a constituent for locating classes. | ||
| * Currently uses BytesURLStreamHandler to hold a reference of the byte[] in memory. | ||
| * This ensures we have a unifed URL resource model for all constituents. |
Comment on lines
+72
to
+73
| * @param lineNo The number of configuraton line where the problem occured. | ||
| * @param line The configuration line where the problem occured. |
Comment on lines
+72
to
+73
| * @param lineNo The number of configuraton line where the problem occured. | ||
| * @param line The configuration line where the problem occured. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (1)
src/test/java/org/codehaus/plexus/classworlds/launcher/LauncherTest.java:45
- This sets a global system property for the whole JVM but never restores the previous value, which can leak into other tests. Also,
org.codehaus.classworlds.protocoldoes not appear to exist in this codebase, so the property may be ineffective or misleading. Capture the old value in@BeforeEachand restore/clear it in@AfterEach(and ensure the package name is correct if a handler is actually needed).
@BeforeEach
void setUp() {
System.setProperty("java.protocol.handler.pkgs", "org.codehaus.classworlds.protocol");
this.launcher = new Launcher();
this.launcher.setSystemClassLoader(Thread.currentThread().getContextClassLoader());
}
| <instructions> | ||
| <_nouses>true</_nouses> | ||
| <Export-Package>org.codehaus.plexus.classworlds.*</Export-Package> | ||
| <Export-Package>org.codehaus.classworlds.*</Export-Package> |
| <configuration> | ||
| <redirectTestOutputToFile>true</redirectTestOutputToFile> | ||
| <argLine>-ea:org.codehaus.plexus.classworlds @{argLine}</argLine> | ||
| <argLine>-ea:org.codehaus.classworlds:org.codehaus.plexus.classworlds @{argLine}</argLine> |
Comment on lines
+19
to
+37
| /** | ||
| * A compatibility wrapper for {@link org.codehaus.plexus.classworlds.realm.ClassRealm} | ||
| * provided for legacy code. | ||
| * | ||
| * <p><b>Note:</b> This is a legacy class provided for backward compatibility with Maven 2. | ||
| * New code should use {@link org.codehaus.plexus.classworlds.realm.ClassRealm}.</p> | ||
| * | ||
| * @author Andrew Williams | ||
| * @deprecated Use {@link org.codehaus.plexus.classworlds.realm.ClassRealm} | ||
| */ | ||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.net.URL; | ||
| import java.util.Enumeration; | ||
|
|
||
| @SuppressWarnings("rawtypes") | ||
| @Deprecated | ||
| public class DefaultClassRealm implements ClassRealm { |
Comment on lines
+101
to
+105
| return ClassRealmAdapter.getInstance(realm.getParentRealm()); | ||
| } | ||
|
|
||
| public ClassRealm getParentRealm() { | ||
| return ClassRealmAdapter.getInstance(realm.getParentRealm()); |
|
|
||
| return getId().equals(((ClassRealm) o).getId()); | ||
| } | ||
|
|
Comment on lines
+69
to
+92
| public org.codehaus.plexus.classworlds.realm.ClassRealm locateSourceRealm(String className) { | ||
| return getInstance(realm.locateSourceRealm(className)); | ||
| } | ||
|
|
||
| public void setParentRealm(org.codehaus.plexus.classworlds.realm.ClassRealm classRealm) { | ||
| realm.setParent(ClassRealmAdapter.getInstance(classRealm)); | ||
| } | ||
|
|
||
| public org.codehaus.plexus.classworlds.realm.ClassRealm createChildRealm(String id) | ||
| throws org.codehaus.plexus.classworlds.realm.DuplicateRealmException { | ||
| try { | ||
| return getInstance(realm.createChildRealm(id)); | ||
| } catch (DuplicateRealmException e) { | ||
| throw new org.codehaus.plexus.classworlds.realm.DuplicateRealmException(getWorld(), e.getId()); | ||
| } | ||
| } | ||
|
|
||
| public ClassLoader getClassLoader() { | ||
| return realm.getClassLoader(); | ||
| } | ||
|
|
||
| public org.codehaus.plexus.classworlds.realm.ClassRealm getParentRealm() { | ||
| return getInstance(realm.getParent()); | ||
| } |
Comment on lines
+119
to
+123
| public boolean equals(Object o) { | ||
| if (!(o instanceof ClassRealm)) return false; | ||
|
|
||
| return getId().equals(((ClassRealm) o).getId()); | ||
| } |
Comment on lines
+67
to
+71
| throw new ConfigurationException(e.getMessage()); | ||
| } catch (org.codehaus.plexus.classworlds.realm.DuplicateRealmException e) { | ||
| throw new DuplicateRealmException(ClassWorldAdapter.getInstance(e.getWorld()), e.getId()); | ||
| } catch (org.codehaus.plexus.classworlds.realm.NoSuchRealmException e) { | ||
| throw new NoSuchRealmException(ClassWorldAdapter.getInstance(e.getWorld()), e.getId()); |
Comment on lines
+72
to
+73
| * @param lineNo The number of configuraton line where the problem occured. | ||
| * @param line The configuration line where the problem occured. |
| /** | ||
| * Adds a byte[] class definition as a constituent for locating classes. | ||
| * Currently uses BytesURLStreamHandler to hold a reference of the byte[] in memory. | ||
| * This ensures we have a unifed URL resource model for all constituents. |
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.
No description provided.