You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Arnaud Héritier <ah...@gmail.com> on 2016/02/26 12:04:02 UTC
Please review Re: maven git commit: [MNG-5984] Maven core extension
resolution ignores repositories from activeByDefault profiles in settings.xml
Hi Christian,
Isn't it possible to add some tests here ?
Also something I'm not sure to understand in the case is if the user has
another profile activated
AFAIR activeByDefault activates a profile only if no profile is manually
activated (with activeProfiles or -P).
There is often a big misunderstanding with this option.
Cheers
On Thu, Feb 25, 2016 at 7:59 PM, <sc...@apache.org> wrote:
> Repository: maven
> Updated Branches:
> refs/heads/master 0a5351340 -> 425289c5e
>
>
> [MNG-5984] Maven core extension resolution ignores repositories from
> activeByDefault profiles in settings.xml
>
>
> Project: http://git-wip-us.apache.org/repos/asf/maven/repo
> Commit: http://git-wip-us.apache.org/repos/asf/maven/commit/425289c5
> Tree: http://git-wip-us.apache.org/repos/asf/maven/tree/425289c5
> Diff: http://git-wip-us.apache.org/repos/asf/maven/diff/425289c5
>
> Branch: refs/heads/master
> Commit: 425289c5e21a822dc150e75fc03abcaf61473900
> Parents: 0a53513
> Author: Christian Schulte <sc...@apache.org>
> Authored: Thu Feb 25 19:46:13 2016 +0100
> Committer: Christian Schulte <sc...@apache.org>
> Committed: Thu Feb 25 19:51:13 2016 +0100
>
> ----------------------------------------------------------------------
> .../SettingsXmlConfigurationProcessor.java | 157 ++++++++++++++-----
> 1 file changed, 114 insertions(+), 43 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/maven/blob/425289c5/maven-embedder/src/main/java/org/apache/maven/cli/configuration/SettingsXmlConfigurationProcessor.java
> ----------------------------------------------------------------------
> diff --git
> a/maven-embedder/src/main/java/org/apache/maven/cli/configuration/SettingsXmlConfigurationProcessor.java
> b/maven-embedder/src/main/java/org/apache/maven/cli/configuration/SettingsXmlConfigurationProcessor.java
> index 791a226..1a57baa 100644
> ---
> a/maven-embedder/src/main/java/org/apache/maven/cli/configuration/SettingsXmlConfigurationProcessor.java
> +++
> b/maven-embedder/src/main/java/org/apache/maven/cli/configuration/SettingsXmlConfigurationProcessor.java
> @@ -21,6 +21,7 @@ package org.apache.maven.cli.configuration;
>
> import java.io.File;
> import java.io.FileNotFoundException;
> +import java.util.ArrayList;
> import java.util.List;
>
> import org.apache.commons.cli.CommandLine;
> @@ -31,9 +32,16 @@ import org.apache.maven.cli.CLIManager;
> import org.apache.maven.cli.CliRequest;
> import org.apache.maven.execution.MavenExecutionRequest;
> import
> org.apache.maven.execution.MavenExecutionRequestPopulationException;
> +import org.apache.maven.model.Profile;
> +import org.apache.maven.model.Repository;
> +import org.apache.maven.model.building.DefaultModelProblem;
> +import org.apache.maven.model.building.ModelProblem;
> +import org.apache.maven.model.building.ModelProblemCollector;
> +import org.apache.maven.model.building.ModelProblemCollectorRequest;
> +import org.apache.maven.model.profile.DefaultProfileActivationContext;
> +import org.apache.maven.model.profile.ProfileSelector;
> import org.apache.maven.settings.Mirror;
> import org.apache.maven.settings.Proxy;
> -import org.apache.maven.settings.Repository;
> import org.apache.maven.settings.Server;
> import org.apache.maven.settings.Settings;
> import org.apache.maven.settings.SettingsUtils;
> @@ -51,6 +59,7 @@ import org.slf4j.Logger;
> public class SettingsXmlConfigurationProcessor
> implements ConfigurationProcessor
> {
> +
> public static final String HINT = "settings";
>
> public static final String USER_HOME = System.getProperty(
> "user.home" );
> @@ -59,8 +68,8 @@ public class SettingsXmlConfigurationProcessor
>
> public static final File DEFAULT_USER_SETTINGS_FILE = new File(
> USER_MAVEN_CONFIGURATION_HOME, "settings.xml" );
>
> - public static final File DEFAULT_GLOBAL_SETTINGS_FILE = new File(
> System.getProperty( "maven.home", System
> - .getProperty( "user.dir", "" ) ), "conf/settings.xml" );
> + public static final File DEFAULT_GLOBAL_SETTINGS_FILE =
> + new File( System.getProperty( "maven.home", System.getProperty(
> "user.dir", "" ) ), "conf/settings.xml" );
>
> @Requirement
> private Logger logger;
> @@ -71,6 +80,9 @@ public class SettingsXmlConfigurationProcessor
> @Requirement
> private SettingsDecrypter settingsDecrypter;
>
> + @Requirement
> + private ProfileSelector profileSelector;
> +
> @Override
> public void process( CliRequest cliRequest )
> throws Exception
> @@ -88,8 +100,9 @@ public class SettingsXmlConfigurationProcessor
>
> if ( !userSettingsFile.isFile() )
> {
> - throw new FileNotFoundException( "The specified user
> settings file does not exist: "
> - + userSettingsFile );
> + throw new FileNotFoundException( String.format( "The
> specified user settings file does not exist: %s",
> +
> userSettingsFile ) );
> +
> }
> }
> else
> @@ -106,8 +119,9 @@ public class SettingsXmlConfigurationProcessor
>
> if ( !globalSettingsFile.isFile() )
> {
> - throw new FileNotFoundException( "The specified global
> settings file does not exist: "
> - + globalSettingsFile );
> + throw new FileNotFoundException( String.format( "The
> specified global settings file does not exist: %s",
> +
> globalSettingsFile ) );
> +
> }
> }
> else
> @@ -129,10 +143,13 @@ public class SettingsXmlConfigurationProcessor
> request.getEventSpyDispatcher().onEvent( settingsRequest );
> }
>
> - logger.debug( "Reading global settings from "
> - + getLocation( settingsRequest.getGlobalSettingsSource(),
> settingsRequest.getGlobalSettingsFile() ) );
> - logger.debug( "Reading user settings from "
> - + getLocation( settingsRequest.getUserSettingsSource(),
> settingsRequest.getUserSettingsFile() ) );
> + logger.debug( String.format( "Reading global settings from %s",
> + getLocation(
> settingsRequest.getGlobalSettingsSource(),
> +
> settingsRequest.getGlobalSettingsFile() ) ) );
> +
> + logger.debug( String.format( "Reading user settings from %s",
> + getLocation(
> settingsRequest.getUserSettingsSource(),
> +
> settingsRequest.getUserSettingsFile() ) ) );
>
> SettingsBuildingResult settingsResult = settingsBuilder.build(
> settingsRequest );
>
> @@ -154,6 +171,89 @@ public class SettingsXmlConfigurationProcessor
> }
> logger.warn( "" );
> }
> +
> + // profile activation
> + final DefaultProfileActivationContext profileActivationContext =
> new DefaultProfileActivationContext();
> + profileActivationContext.setActiveProfileIds(
> request.getActiveProfiles() );
> + profileActivationContext.setInactiveProfileIds(
> request.getInactiveProfiles() );
> + profileActivationContext.setSystemProperties(
> request.getSystemProperties() );
> + profileActivationContext.setUserProperties(
> request.getUserProperties() );
> + profileActivationContext.setProjectDirectory( ( request.getPom()
> != null )
> + ?
> request.getPom().getParentFile()
> + : null );
> +
> + final List<ModelProblem> modelProblems = new ArrayList<>();
> + final List<Profile> activeProfiles =
> + this.profileSelector.getActiveProfiles(
> request.getProfiles(), profileActivationContext,
> + new
> ModelProblemCollector()
> + {
> +
> + @Override
> + public void add(
> final ModelProblemCollectorRequest req )
> + {
> +
> modelProblems.add( new DefaultModelProblem(
> +
> req.getMessage(), req.getSeverity(),
> +
> req.getVersion(), "settings.xml", -1, -1,
> + null,
> req.getException() ) );
> +
> + }
> +
> + } );
> +
> + if ( !modelProblems.isEmpty() )
> + {
> + logger.warn( "" );
> + logger.warn( "Some problems were encountered while processing
> profiles" );
> +
> + for ( final ModelProblem problem : modelProblems )
> + {
> + logger.warn( problem.getMessage(), problem.getException()
> );
> + }
> +
> + logger.warn( "" );
> + }
> +
> + if ( !activeProfiles.isEmpty() )
> + {
> + for ( final Profile profile : activeProfiles )
> + {
> + final List<Repository> remoteRepositories =
> profile.getRepositories();
> +
> + for ( final Repository remoteRepository :
> remoteRepositories )
> + {
> + try
> + {
> + request.addRemoteRepository(
> +
> MavenRepositorySystem.buildArtifactRepository( remoteRepository ) );
> +
> + }
> + catch ( final InvalidRepositoryException e )
> + {
> + logger.warn( String.format( "Failure adding
> repository '%s' from profile '%s'.",
> +
> remoteRepository.getId(), profile.getId() ), e );
> +
> + }
> + }
> +
> + final List<Repository> pluginRepositories =
> profile.getPluginRepositories();
> +
> + for ( final Repository pluginRepository :
> pluginRepositories )
> + {
> + try
> + {
> + request.addPluginArtifactRepository(
> +
> MavenRepositorySystem.buildArtifactRepository( pluginRepository ) );
> +
> + }
> + catch ( InvalidRepositoryException e )
> + {
> + logger.warn( String.format( "Failure adding
> plugin repository '%s' from profile '%s'.",
> +
> pluginRepository.getId(), profile.getId() ), e );
> +
> + }
> + }
> + }
> + }
> }
>
> private MavenExecutionRequest populateFromSettings(
> MavenExecutionRequest request, Settings settings )
> @@ -218,43 +318,13 @@ public class SettingsXmlConfigurationProcessor
> request.addMirror( mirror );
> }
>
> - request.setActiveProfiles( settings.getActiveProfiles() );
> + request.addActiveProfiles( settings.getActiveProfiles() );
>
> for ( org.apache.maven.settings.Profile rawProfile :
> settings.getProfiles() )
> {
> request.addProfile( SettingsUtils.convertFromSettingsProfile(
> rawProfile ) );
> -
> - if ( settings.getActiveProfiles().contains(
> rawProfile.getId() ) )
> - {
> - List<Repository> remoteRepositories =
> rawProfile.getRepositories();
> - for ( Repository remoteRepository : remoteRepositories )
> - {
> - try
> - {
> - request.addRemoteRepository(
> -
> MavenRepositorySystem.buildArtifactRepository( remoteRepository ) );
> - }
> - catch ( InvalidRepositoryException e )
> - {
> - // do nothing for now
> - }
> - }
> -
> - List<Repository> pluginRepositories =
> rawProfile.getPluginRepositories();
> - for ( Repository pluginRepository : pluginRepositories )
> - {
> - try
> - {
> - request.addPluginArtifactRepository(
> -
> MavenRepositorySystem.buildArtifactRepository( pluginRepository ) );
> - }
> - catch ( InvalidRepositoryException e )
> - {
> - // do nothing for now
> - }
> - }
> - }
> }
> +
> return request;
> }
>
> @@ -287,4 +357,5 @@ public class SettingsXmlConfigurationProcessor
> return new File( workingDirectory, file.getPath()
> ).getAbsoluteFile();
> }
> }
> +
> }
>
>
--
-----
Arnaud Héritier
http://aheritier.net
Mail/GTalk: aheritier AT gmail DOT com
Twitter/Skype : aheritier
Re: Please review Re: maven git commit: [MNG-5984] Maven core
extension resolution ignores repositories from activeByDefault profiles in
settings.xml
Posted by Christian Schulte <cs...@schulte.it>.
Am 02/26/16 um 12:04 schrieb Arnaud Héritier:
> Hi Christian,
>
> Isn't it possible to add some tests here ?
> Also something I'm not sure to understand in the case is if the user has
> another profile activated
> AFAIR activeByDefault activates a profile only if no profile is manually
> activated (with activeProfiles or -P).
> There is often a big misunderstanding with this option.
I asked the reporter to test the snapshot. See the last comment on
MNG-5984. Will add tests when everything is sorted out, of course.
> activeByDefault activates a profile only if no profile is
> manually activated (with activeProfiles or -P)
Looking at DefaultProfileSelector [1], that only applies to profiles
declared in the pom "Profile.SOURCE_POM.equals( profile.getSource() )".
Profiles from settings are handled differently. Currently the settings
are processed before any command line options (like -P, for example). I
think that needs to be changed so that the -P command line option also
works for profiles from settings. The commit is not only about handling
the "activeByDefault" case. The "<activations>" were ignored before as well.
[1]
<https://git-wip-us.apache.org/repos/asf?p=maven.git;a=blob;f=maven-model-builder/src/main/java/org/apache/maven/model/profile/DefaultProfileSelector.java;h=457ab2a3c1ebb350ae44efdab91b62bbc9512859;hb=HEAD>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org