Uploaded image for project: 'Maven'
  1. Maven
  2. MNG-7417

Several classes do not set properties properly for building requests

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    Description

      A sample case for this bug is DefaultArtifactDescriptorReader:

      When Resolver needs to resolve a transitive dependency it consults Maven Resolver Provider to create data. Here the properties of the repository system session are not properly passed with their respective scopes down to the model building request. When a POM now needs to be interpolated, the interpolation can fail.
      For long time Maven, unfortunately, promoted user properties to system properties making the available everywhere (expect this to be cleaned up in Maven 4). A prime example where this is broken: DefaultModelVersionProcessor uses for some strange reason system properties although CI Friendly Versions are user properties only. Fixing this component with:

      --- a/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/DefaultModelVersionProcessor.java
      +++ b/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/DefaultModelVersionProcessor.java
      @@ -52,17 +52,17 @@ public boolean isValidProperty( String property )
           @Override
           public void overwriteModelProperties( Properties modelProperties, ModelBuildingRequest request )
           {
      -        if ( request.getSystemProperties().containsKey( REVISION_PROPERTY ) )
      +        if ( request.getUserProperties().containsKey( REVISION_PROPERTY ) )
               {
      -            modelProperties.put( REVISION_PROPERTY, request.getSystemProperties().get( REVISION_PROPERTY ) );
      +            modelProperties.put( REVISION_PROPERTY, request.getUserProperties().get( REVISION_PROPERTY ) );
               }
      -        if ( request.getSystemProperties().containsKey( CHANGELIST_PROPERTY ) )
      +        if ( request.getUserProperties().containsKey( CHANGELIST_PROPERTY ) )
               {
      -            modelProperties.put( CHANGELIST_PROPERTY, request.getSystemProperties().get( CHANGELIST_PROPERTY ) );
      +            modelProperties.put( CHANGELIST_PROPERTY, request.getUserProperties().get( CHANGELIST_PROPERTY ) );
               }
      -        if ( request.getSystemProperties().containsKey( SHA1_PROPERTY ) )
      +        if ( request.getUserProperties().containsKey( SHA1_PROPERTY ) )
               {
      -            modelProperties.put( SHA1_PROPERTY, request.getSystemProperties().get( SHA1_PROPERTY ) );
      +            modelProperties.put( SHA1_PROPERTY, request.getUserProperties().get( SHA1_PROPERTY ) );
               }
      
           }
      

      and running ITs makes several of them fail:

      ...
      mng6090 CIFriendly.itShouldResolveTheDependenciesWithBuildConsumer() FAILURE (10.4 s)
      mng6090 CIFriendly.itShouldResolveTheDependenciesWithoutBuildConsumer() FAILURE (1.2 s)
      ...
      mng5895 CIFriendlyUsageWithProperty.itShouldResolveTheDependenciesWithBuildConsumer() FAILURE (0.5 s)
      mng5895 CIFriendlyUsageWithProperty.itShouldResolveTheDependenciesWithoutBuildConsumer() FAILURE (0.5 s)
      ...
      

      The reason is simple:

                      modelRequest.setSystemProperties( toProperties( session.getUserProperties(),
                                                                      session.getSystemProperties() ) );
      

      Properties from user are not available. The are likely other usecases affected by this bug.

      Yet another problem is that plugins which resolve dependencies, e.g., MASSEMBLY will also fail even if this bug is fixed since they rely on an old version of the Maven Resolver Provider and don't use provided scope to use the fixed one from Maven Core. This is a separate problem to be solved.

      Attachments

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            slachiewicz Sylwester Lachiewicz
            michael-o Michael Osipov
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment