You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by jd...@apache.org on 2009/07/10 06:01:42 UTC

svn commit: r792795 - in /maven/maven-2/branches/maven-2.2.x: ./ maven-artifact-manager/src/main/java/org/apache/maven/artifact/manager/ maven-core/ maven-core/src/main/java/org/apache/maven/extension/ maven-core/src/main/resources/META-INF/plexus/

Author: jdcasey
Date: Fri Jul 10 04:01:41 2009
New Revision: 792795

URL: http://svn.apache.org/viewvc?rev=792795&view=rev
Log:
[MNG-4228] NEED ITS. This implements wagon-impl selection using 'maven.wagon.proto=implname' form of system property. Switched out extension loading to use component descriptor lookup instead of component lookup to avoid killing the system in the event that one wagon extension cannot load. This last is because something has screwed up the loading of the dav+http/s wagons, and it affects unrelated ITs.

This implementation needs ITs to verify the selectable wagon behavior, and another to verify the presence and loadability of the dav+http/s wagons.

Modified:
    maven/maven-2/branches/maven-2.2.x/maven-artifact-manager/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java
    maven/maven-2/branches/maven-2.2.x/maven-core/pom.xml
    maven/maven-2/branches/maven-2.2.x/maven-core/src/main/java/org/apache/maven/extension/DefaultExtensionManager.java
    maven/maven-2/branches/maven-2.2.x/maven-core/src/main/resources/META-INF/plexus/components.xml
    maven/maven-2/branches/maven-2.2.x/pom.xml

Modified: maven/maven-2/branches/maven-2.2.x/maven-artifact-manager/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java
URL: http://svn.apache.org/viewvc/maven/maven-2/branches/maven-2.2.x/maven-artifact-manager/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java?rev=792795&r1=792794&r2=792795&view=diff
==============================================================================
--- maven/maven-2/branches/maven-2.2.x/maven-artifact-manager/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java (original)
+++ maven/maven-2/branches/maven-2.2.x/maven-artifact-manager/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java Fri Jul 10 04:01:41 2009
@@ -46,6 +46,7 @@
 import org.codehaus.plexus.component.repository.exception.ComponentLifecycleException;
 import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
 import org.codehaus.plexus.configuration.PlexusConfiguration;
+import org.codehaus.plexus.configuration.PlexusConfigurationException;
 import org.codehaus.plexus.configuration.xml.XmlPlexusConfiguration;
 import org.codehaus.plexus.context.Context;
 import org.codehaus.plexus.context.ContextException;
@@ -82,6 +83,10 @@
 
     private static final String MAVEN_ARTIFACT_PROPERTIES = "META-INF/maven/org.apache.maven/maven-artifact/pom.properties";
 
+    public static final String DELEGATE_PROPERTY_BASE = "maven.wagon.";
+
+    private static final String WAGON_IMPL_CONFIGURATION = "wagonImpl";
+
     private static int anonymousMirrorIdSeed = 0;
     
     private PlexusContainer container;
@@ -98,7 +103,7 @@
     private Map mirrors = new LinkedHashMap();
 
     /** Map( String, XmlPlexusConfiguration ) with the repository id and the wagon configuration */
-    private Map serverConfigurationMap = new HashMap();
+    private Map<String, XmlPlexusConfiguration> serverConfigurationMap = new HashMap<String, XmlPlexusConfiguration>();
 
     private TransferListener downloadMonitor;
 
@@ -108,7 +113,7 @@
 
     private boolean interactive = true;
 
-    private Map availableWagons = new HashMap();
+    private Map<String, PlexusContainer> availableWagons = new HashMap<String, PlexusContainer>();
 
     private RepositoryPermissions defaultRepositoryPermissions;
 
@@ -125,22 +130,29 @@
             throw new UnsupportedProtocolException( "The repository " + repository + " does not specify a protocol" );
         }
 
-        Wagon wagon = getWagon( protocol );
+        Wagon wagon = getWagon( protocol, repository.getId() );
 
         configureWagon( wagon, repository.getId(), protocol );
 
         return wagon;
     }
-
+    
     public Wagon getWagon( String protocol )
         throws UnsupportedProtocolException
     {
-        PlexusContainer container = getWagonContainer( protocol );
+        return getWagon( protocol, null );
+    }
+    
+    private Wagon getWagon( String protocol, String repositoryId )
+        throws UnsupportedProtocolException
+    {
+        String hint = getWagonHint( protocol, repositoryId );
+        PlexusContainer container = getWagonContainer( hint );
 
         Wagon wagon;
         try
         {
-            wagon = (Wagon) container.lookup( Wagon.ROLE, protocol );
+            wagon = (Wagon) container.lookup( Wagon.ROLE, hint );
         }
         catch ( ComponentLookupException e1 )
         {
@@ -152,15 +164,47 @@
 
         return wagon;
     }
+    
+    private String getWagonHint( String protocol, String repositoryId )
+    {
+        // TODO: Implement a better way to get the hint, via settings.xml or something.
+        String impl = null;
+        
+        if ( repositoryId != null && serverConfigurationMap.containsKey( repositoryId ) )
+        {
+            XmlPlexusConfiguration config = serverConfigurationMap.get( repositoryId );
+            
+            Xpp3Dom dom = config.getXpp3Dom();
+            for ( int i = 0; i < dom.getChildCount(); i++ )
+            {
+                Xpp3Dom domChild = dom.getChild( i );
+                if ( WAGON_IMPL_CONFIGURATION.equals( domChild.getName() ) )
+                {
+                    impl = domChild.getValue();
+                    dom.removeChild( i );
+                    break;
+                }
+                
+                i++;
+            }
+        }
+        
+        if ( impl == null )
+        {
+            impl = System.getProperty( DELEGATE_PROPERTY_BASE + protocol, null );
+        }
+        
+        return impl == null ? protocol : protocol + "-" + impl;
+    }
 
-    private PlexusContainer getWagonContainer( String protocol )
+    private PlexusContainer getWagonContainer( String hint )
     {
         PlexusContainer container = this.container;
-
-        if ( availableWagons.containsKey( protocol ) )
+        if ( availableWagons.containsKey( hint ) )
         {
-            container = (PlexusContainer) availableWagons.get( protocol );
+            container = availableWagons.get( hint );
         }
+        
         return container;
     }
 
@@ -194,7 +238,7 @@
         Wagon wagon;
         try
         {
-            wagon = getWagon( protocol );
+            wagon = getWagon( protocol, repository.getId() );
 
             configureWagon( wagon, repository );
         }
@@ -308,7 +352,7 @@
         {
             disconnectWagon( wagon );
 
-            releaseWagon( protocol, wagon );
+            releaseWagon( protocol, wagon, repository.getId() );
         }
     }
 
@@ -413,7 +457,7 @@
         Wagon wagon;
         try
         {
-            wagon = getWagon( protocol );
+            wagon = getWagon( protocol, repository.getId() );
 
             configureWagon( wagon, repository );
         }
@@ -595,7 +639,7 @@
         {
             disconnectWagon( wagon );
 
-            releaseWagon( protocol, wagon );
+            releaseWagon( protocol, wagon, repository.getId() );
         }
 
         if ( downloaded )
@@ -749,9 +793,11 @@
     }
 
     private void releaseWagon( String protocol,
-                               Wagon wagon )
+                               Wagon wagon, String repositoryId )
     {
-        PlexusContainer container = getWagonContainer( protocol );
+        String hint = getWagonHint( protocol, repositoryId );
+        
+        PlexusContainer container = getWagonContainer( hint );
         try
         {
             container.release( wagon );
@@ -1015,10 +1061,11 @@
         this.interactive = interactive;
     }
 
+    @SuppressWarnings( "unchecked" )
     public void registerWagons( Collection wagons,
                                 PlexusContainer extensionContainer )
     {
-        for ( Iterator i = wagons.iterator(); i.hasNext(); )
+        for ( Iterator<String> i = wagons.iterator(); i.hasNext(); )
         {
             availableWagons.put( i.next(), extensionContainer );
         }

Modified: maven/maven-2/branches/maven-2.2.x/maven-core/pom.xml
URL: http://svn.apache.org/viewvc/maven/maven-2/branches/maven-2.2.x/maven-core/pom.xml?rev=792795&r1=792794&r2=792795&view=diff
==============================================================================
--- maven/maven-2/branches/maven-2.2.x/maven-core/pom.xml (original)
+++ maven/maven-2/branches/maven-2.2.x/maven-core/pom.xml Fri Jul 10 04:01:41 2009
@@ -65,6 +65,13 @@
         </exclusion>
       </exclusions>
     </dependency>
+    <!-- NOTE: Listing this last causes it to be the default Wagon implementation
+         when the bare protocol is used to lookup the http/https wagons.
+    -->
+    <dependency>
+      <groupId>org.apache.maven.wagon</groupId>
+      <artifactId>wagon-http-lightweight</artifactId>
+    </dependency>
     <dependency>
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-jdk14</artifactId>

Modified: maven/maven-2/branches/maven-2.2.x/maven-core/src/main/java/org/apache/maven/extension/DefaultExtensionManager.java
URL: http://svn.apache.org/viewvc/maven/maven-2/branches/maven-2.2.x/maven-core/src/main/java/org/apache/maven/extension/DefaultExtensionManager.java?rev=792795&r1=792794&r2=792795&view=diff
==============================================================================
--- maven/maven-2/branches/maven-2.2.x/maven-core/src/main/java/org/apache/maven/extension/DefaultExtensionManager.java (original)
+++ maven/maven-2/branches/maven-2.2.x/maven-core/src/main/java/org/apache/maven/extension/DefaultExtensionManager.java Fri Jul 10 04:01:41 2009
@@ -265,17 +265,9 @@
     {
         if ( extensionContainer != null )
         {
-            try
-            {
-                Map wagons = extensionContainer.lookupMap( Wagon.ROLE );
-                getLogger().debug( "Wagons to register: " + wagons.keySet() );
-                wagonManager.registerWagons( wagons.keySet(), extensionContainer );
-            }
-            catch ( ComponentLookupException e )
-            {
-                // no wagons found in the extension
-                getLogger().debug( "No wagons found in the extensions or other internal error: " + e.getMessage(), e );
-            }
+            Map wagons = extensionContainer.getComponentDescriptorMap( Wagon.ROLE );
+            getLogger().debug( "Wagons to register: " + wagons.keySet() );
+            wagonManager.registerWagons( wagons.keySet(), extensionContainer );
         }
         else
         {

Modified: maven/maven-2/branches/maven-2.2.x/maven-core/src/main/resources/META-INF/plexus/components.xml
URL: http://svn.apache.org/viewvc/maven/maven-2/branches/maven-2.2.x/maven-core/src/main/resources/META-INF/plexus/components.xml?rev=792795&r1=792794&r2=792795&view=diff
==============================================================================
--- maven/maven-2/branches/maven-2.2.x/maven-core/src/main/resources/META-INF/plexus/components.xml (original)
+++ maven/maven-2/branches/maven-2.2.x/maven-core/src/main/resources/META-INF/plexus/components.xml Fri Jul 10 04:01:41 2009
@@ -622,5 +622,32 @@
         <_configuration-file>~/.m2/settings-security.xml</_configuration-file>
       </configuration>
     </component>    
+    
+    <component>
+      <role>org.apache.maven.wagon.Wagon</role>
+      <role-hint>http-sun</role-hint>
+      <implementation>org.apache.maven.wagon.providers.http.LightweightHttpWagon</implementation>
+      <instantiation-strategy>per-lookup</instantiation-strategy>
+      <description>LightweightHttpWagon</description>
+    </component>
+    <component>
+      <role>org.apache.maven.wagon.Wagon</role>
+      <role-hint>https-sun</role-hint>
+      <implementation>org.apache.maven.wagon.providers.http.LightweightHttpsWagon</implementation>
+      <instantiation-strategy>per-lookup</instantiation-strategy>
+      <description>LIghtweightHttpsWagon</description>
+    </component>
+    <component>
+      <role>org.apache.maven.wagon.Wagon</role>
+      <role-hint>https-httpclient</role-hint>
+      <implementation>org.apache.maven.wagon.providers.http.HttpWagon</implementation>
+      <instantiation-strategy>per-lookup</instantiation-strategy>
+    </component>
+    <component>
+      <role>org.apache.maven.wagon.Wagon</role>
+      <role-hint>http-httpclient</role-hint>
+      <implementation>org.apache.maven.wagon.providers.http.HttpWagon</implementation>
+      <instantiation-strategy>per-lookup</instantiation-strategy>
+    </component>
   </components>
 </component-set>

Modified: maven/maven-2/branches/maven-2.2.x/pom.xml
URL: http://svn.apache.org/viewvc/maven/maven-2/branches/maven-2.2.x/pom.xml?rev=792795&r1=792794&r2=792795&view=diff
==============================================================================
--- maven/maven-2/branches/maven-2.2.x/pom.xml (original)
+++ maven/maven-2/branches/maven-2.2.x/pom.xml Fri Jul 10 04:01:41 2009
@@ -451,6 +451,11 @@
         <version>${wagonVersion}</version>
       </dependency>
       <dependency>
+        <groupId>org.apache.maven.wagon</groupId>
+        <artifactId>wagon-http-lightweight</artifactId>
+        <version>${wagonVersion}</version>
+      </dependency>
+      <dependency>
         <groupId>org.slf4j</groupId>
         <artifactId>slf4j-jdk14</artifactId>
         <version>1.5.6</version>



Re: svn commit: r792795 - in /maven/maven-2/branches/maven-2.2.x: ./ maven-artifact-manager/src/main/java/org/apache/maven/artifact/manager/ maven-core/ maven-core/src/main/java/org/apache/maven/extension/ maven-core/src/main/resources/META-INF/plexus/

Posted by Brett Porter <br...@apache.org>.
On 11/07/2009, at 1:32 AM, John Casey wrote:

>
> Actually, I disagree. First, since the configurator attempts to  
> traverse  the PlexusConfiguration and reflectively assign those  
> values to bean properties (or fields) in the wagon instance, this  
> means that a Wagon.class pass-through will have to be able to  
> somehow accommodate all  possible configuration parameters that any  
> delegate would use. While we might be able to accomplish this by  
> making WagonDelegate implement MapOrientedComponent, we'd still have  
> to take those mapped variables and immediately start over again with  
> a new Configurator instance internally, so we could actually pass on  
> the configuration to the delegate.
>
> Simply resolving the hint and looking up the correct wagon before  
> configuration takes place makes all of this much, much simpler.

Cool, I hadn't thought of that. It is still possible since the config  
is mostly (and should be) the same between the two, but if you've  
sorted out the below it doesn't make it worth it.

>
> As for the POM ordering issue, we can avoid that simply by providing  
> a defaultRoleHints map configuration for the wagon manager and/or  
> the delegator class that looks up the wagon (initially, i had  
> factored out the Wagon-lookup functionality, but folded it back into  
> WagonManager in the end). Since maven-core redefines the lw and  
> httpclient http wagon components using things like 'http-sun', we  
> could configure the wagon manager to default over to 'http-sun' if  
> no sysprop is provided, then if the defaultRoleHints mapping doesn't  
> contain an entry for the protocol in question WagonManager could  
> lookup by the protocol itself.

Cool

>
>> I'd prefer the SysProp be checked in DefaultMaven (via the  
>> execution properties), like maven.artifact.threads, and then a  
>> method called on the wagon manager to set a default wagon impl  
>> (when registerWagons is called).
>
> I don't have any serious issue with moving the sysprop check; I just  
> figured it made more sense architecturally to have all of the  
> configuration logic for wagons in one place (as much as the api  
> separation allows).

While I realise 2.x isn't targeted to being embeddable, I thought it  
seemed good to keep up with the same practices.

Thanks!

- Brett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: svn commit: r792795 - in /maven/maven-2/branches/maven-2.2.x: ./ maven-artifact-manager/src/main/java/org/apache/maven/artifact/manager/ maven-core/ maven-core/src/main/java/org/apache/maven/extension/ maven-core/src/main/resources/META-INF/plexus/

Posted by John Casey <jd...@commonjava.org>.
Hi Brett,

my responses are inline:

Brett Porter wrote:
> 
>> +
>> +    private String getWagonHint( String protocol, String repositoryId )
>> +    {
>> +        // TODO: Implement a better way to get the hint, via 
>> settings.xml or something.
> 
> While this effectively is the same, I think it could be simplified by an 
> actual wagon implementation that takes the http/https definitions and 
> delegates each method call to a selected wagon impl. That way the server 
> config is passed in using normal plexus configuration:
> 
> WagonDelegate implements Wagon, Contextualizable {
> 
>   private String wagonImpl;
> 
>   private Wagon delegate;
> 
>   contextualize() {
>     delegate = container.lookup( ROLE, wagonImpl );
>   }
> 
>  put( ... ) {
>   delegate.put( ... );
>  }
> }
> 
> WDYT?
> 
> This would also avoid the ordering kludge you needed to add to the POM 
> later.

Actually, I disagree. First, since the configurator attempts to traverse 
  the PlexusConfiguration and reflectively assign those values to bean 
properties (or fields) in the wagon instance, this means that a 
Wagon.class pass-through will have to be able to somehow accommodate all 
  possible configuration parameters that any delegate would use. While 
we might be able to accomplish this by making WagonDelegate implement 
MapOrientedComponent, we'd still have to take those mapped variables and 
immediately start over again with a new Configurator instance 
internally, so we could actually pass on the configuration to the delegate.

Simply resolving the hint and looking up the correct wagon before 
configuration takes place makes all of this much, much simpler.

As for the POM ordering issue, we can avoid that simply by providing a 
defaultRoleHints map configuration for the wagon manager and/or the 
delegator class that looks up the wagon (initially, i had factored out 
the Wagon-lookup functionality, but folded it back into WagonManager in 
the end). Since maven-core redefines the lw and httpclient http wagon 
components using things like 'http-sun', we could configure the wagon 
manager to default over to 'http-sun' if no sysprop is provided, then if 
the defaultRoleHints mapping doesn't contain an entry for the protocol 
in question WagonManager could lookup by the protocol itself.

> I'd prefer the SysProp be checked in DefaultMaven (via the execution 
> properties), like maven.artifact.threads, and then a method called on 
> the wagon manager to set a default wagon impl (when registerWagons is 
> called).

I don't have any serious issue with moving the sysprop check; I just 
figured it made more sense architecturally to have all of the 
configuration logic for wagons in one place (as much as the api 
separation allows).

>> +        for ( Iterator<String> i = wagons.iterator(); i.hasNext(); )
> 
> can change to:
> for ( String protocol : wagons ) { availableWagons.put( protocol, 
> extensionContainer ); }

yeah, that got left out while I was trying to debug a problem I was 
having, and I lost track of it.

> Is it possible wagons will be null if the prior use case is encountered, 
> or will it return an empty map?

I'll fix that in my next commit.

-john


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: svn commit: r792795 - in /maven/maven-2/branches/maven-2.2.x: ./ maven-artifact-manager/src/main/java/org/apache/maven/artifact/manager/ maven-core/ maven-core/src/main/java/org/apache/maven/extension/ maven-core/src/main/resources/META-INF/plexus/

Posted by Brett Porter <br...@apache.org>.
Hi John,

Some thoughts on this...

On 10/07/2009, at 2:01 PM, jdcasey@apache.org wrote:

> +
> +    private String getWagonHint( String protocol, String  
> repositoryId )
> +    {
> +        // TODO: Implement a better way to get the hint, via  
> settings.xml or something.

While this effectively is the same, I think it could be simplified by  
an actual wagon implementation that takes the http/https definitions  
and delegates each method call to a selected wagon impl. That way the  
server config is passed in using normal plexus configuration:

WagonDelegate implements Wagon, Contextualizable {

   private String wagonImpl;

   private Wagon delegate;

   contextualize() {
     delegate = container.lookup( ROLE, wagonImpl );
   }

  put( ... ) {
   delegate.put( ... );
  }
}

WDYT?

This would also avoid the ordering kludge you needed to add to the POM  
later.

>
> +        if ( impl == null )
> +        {
> +            impl = System.getProperty( DELEGATE_PROPERTY_BASE +  
> protocol, null );
> +        }

I'd prefer the SysProp be checked in DefaultMaven (via the execution  
properties), like maven.artifact.threads, and then a method called on  
the wagon manager to set a default wagon impl (when registerWagons is  
called).

>
> +    @SuppressWarnings( "unchecked" )
>     public void registerWagons( Collection wagons,
>                                 PlexusContainer extensionContainer )
>     {
> -        for ( Iterator i = wagons.iterator(); i.hasNext(); )
> +        for ( Iterator<String> i = wagons.iterator(); i.hasNext(); )
>         {
>             availableWagons.put( i.next(), extensionContainer );
>         }

can change to:
for ( String protocol : wagons ) { availableWagons.put( protocol,  
extensionContainer ); }

>         {
> -            try
> -            {
> -                Map wagons =  
> extensionContainer.lookupMap( Wagon.ROLE );
> -                getLogger().debug( "Wagons to register: " +  
> wagons.keySet() );
> -                wagonManager.registerWagons( wagons.keySet(),  
> extensionContainer );
> -            }
> -            catch ( ComponentLookupException e )
> -            {
> -                // no wagons found in the extension
> -                getLogger().debug( "No wagons found in the  
> extensions or other internal error: " + e.getMessage(), e );
> -            }
> +            Map wagons =  
> extensionContainer.getComponentDescriptorMap( Wagon.ROLE );
> +            getLogger().debug( "Wagons to register: " +  
> wagons.keySet() );
> +            wagonManager.registerWagons( wagons.keySet(),  
> extensionContainer );

Is it possible wagons will be null if the prior use case is  
encountered, or will it return an empty map?

Cheers,
Brett




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org