You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/01/24 22:06:46 UTC
[GitHub] [maven] michael-o commented on a change in pull request #429: [MNG-7051] - Optionally skip non-existing profiles and break on missing required profiles
michael-o commented on a change in pull request #429:
URL: https://github.com/apache/maven/pull/429#discussion_r563362703
##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -493,25 +508,88 @@ private void validatePrerequisitesForNonMavenPluginProjects( List<MavenProject>
}
}
- private void validateActivatedProfiles( List<MavenProject> projects, List<String> activeProfileIds )
+ /**
+ * Get all profiles that are detected in either the projects, any parent of the projects or the settings.
+ * @param session The Maven session
+ * @return A {@link Set} of profile identifiers, never {@code null}.
+ */
+ private Set<String> getAllProfiles( MavenSession session )
{
- Collection<String> notActivatedProfileIds = new LinkedHashSet<>( activeProfileIds );
-
- for ( MavenProject project : projects )
+ final Set<MavenProject> projectsIncludingParents = new HashSet<>();
+ for ( MavenProject project : session.getProjects() )
{
- for ( List<String> profileIds : project.getInjectedProfileIds().values() )
+ boolean isAdded = projectsIncludingParents.add( project );
+ MavenProject parent = project.getParent();
+ while ( isAdded && parent != null )
{
- notActivatedProfileIds.removeAll( profileIds );
+ isAdded = projectsIncludingParents.add( parent );
+ parent = parent.getParent();
}
}
- for ( String notActivatedProfileId : notActivatedProfileIds )
+ final Stream<String> projectProfiles = projectsIncludingParents.stream()
+ .map( MavenProject::getModel )
+ .map( Model::getProfiles )
+ .flatMap( Collection::stream )
+ .map( Profile::getId );
+ final Stream<String> settingsProfiles = session.getSettings().getProfiles().stream()
+ .map( IdentifiableBase::getId );
Review comment:
Why not call `Profile::getId`?
##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -493,25 +508,88 @@ private void validatePrerequisitesForNonMavenPluginProjects( List<MavenProject>
}
}
- private void validateActivatedProfiles( List<MavenProject> projects, List<String> activeProfileIds )
+ /**
+ * Get all profiles that are detected in either the projects, any parent of the projects or the settings.
+ * @param session The Maven session
+ * @return A {@link Set} of profile identifiers, never {@code null}.
+ */
+ private Set<String> getAllProfiles( MavenSession session )
{
- Collection<String> notActivatedProfileIds = new LinkedHashSet<>( activeProfileIds );
-
- for ( MavenProject project : projects )
+ final Set<MavenProject> projectsIncludingParents = new HashSet<>();
+ for ( MavenProject project : session.getProjects() )
{
- for ( List<String> profileIds : project.getInjectedProfileIds().values() )
+ boolean isAdded = projectsIncludingParents.add( project );
+ MavenProject parent = project.getParent();
+ while ( isAdded && parent != null )
{
- notActivatedProfileIds.removeAll( profileIds );
+ isAdded = projectsIncludingParents.add( parent );
+ parent = parent.getParent();
}
}
- for ( String notActivatedProfileId : notActivatedProfileIds )
+ final Stream<String> projectProfiles = projectsIncludingParents.stream()
+ .map( MavenProject::getModel )
+ .map( Model::getProfiles )
+ .flatMap( Collection::stream )
+ .map( Profile::getId );
+ final Stream<String> settingsProfiles = session.getSettings().getProfiles().stream()
+ .map( IdentifiableBase::getId );
+
+ return Stream.concat( projectProfiles, settingsProfiles ).collect( toSet() );
+ }
+
+ /**
+ * Check whether the required profiles were found in any of the projects we're building or the Maven settings.
+ * @param session the Maven session.
+ * @param profileActivation the requested optional and required profiles.
+ */
+ private void validateRequiredProfiles( MavenSession session, ProfileActivation profileActivation )
+ {
+ final Set<String> allDetectedProfiles = getAllProfiles( session );
Review comment:
I think `allAvailableProfiles` suits better here.
##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -493,25 +508,88 @@ private void validatePrerequisitesForNonMavenPluginProjects( List<MavenProject>
}
}
- private void validateActivatedProfiles( List<MavenProject> projects, List<String> activeProfileIds )
+ /**
+ * Get all profiles that are detected in either the projects, any parent of the projects or the settings.
+ * @param session The Maven session
+ * @return A {@link Set} of profile identifiers, never {@code null}.
+ */
+ private Set<String> getAllProfiles( MavenSession session )
{
- Collection<String> notActivatedProfileIds = new LinkedHashSet<>( activeProfileIds );
-
- for ( MavenProject project : projects )
+ final Set<MavenProject> projectsIncludingParents = new HashSet<>();
+ for ( MavenProject project : session.getProjects() )
{
- for ( List<String> profileIds : project.getInjectedProfileIds().values() )
+ boolean isAdded = projectsIncludingParents.add( project );
+ MavenProject parent = project.getParent();
+ while ( isAdded && parent != null )
{
- notActivatedProfileIds.removeAll( profileIds );
+ isAdded = projectsIncludingParents.add( parent );
+ parent = parent.getParent();
}
}
- for ( String notActivatedProfileId : notActivatedProfileIds )
+ final Stream<String> projectProfiles = projectsIncludingParents.stream()
Review comment:
Very nice and concise code! I did this almost 20 years ago in Haskell, happy this seeing in Java now.
##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -493,25 +508,88 @@ private void validatePrerequisitesForNonMavenPluginProjects( List<MavenProject>
}
}
- private void validateActivatedProfiles( List<MavenProject> projects, List<String> activeProfileIds )
+ /**
+ * Get all profiles that are detected in either the projects, any parent of the projects or the settings.
+ * @param session The Maven session
+ * @return A {@link Set} of profile identifiers, never {@code null}.
+ */
+ private Set<String> getAllProfiles( MavenSession session )
{
- Collection<String> notActivatedProfileIds = new LinkedHashSet<>( activeProfileIds );
-
- for ( MavenProject project : projects )
+ final Set<MavenProject> projectsIncludingParents = new HashSet<>();
+ for ( MavenProject project : session.getProjects() )
{
- for ( List<String> profileIds : project.getInjectedProfileIds().values() )
+ boolean isAdded = projectsIncludingParents.add( project );
+ MavenProject parent = project.getParent();
+ while ( isAdded && parent != null )
{
- notActivatedProfileIds.removeAll( profileIds );
+ isAdded = projectsIncludingParents.add( parent );
+ parent = parent.getParent();
}
}
- for ( String notActivatedProfileId : notActivatedProfileIds )
+ final Stream<String> projectProfiles = projectsIncludingParents.stream()
+ .map( MavenProject::getModel )
+ .map( Model::getProfiles )
+ .flatMap( Collection::stream )
+ .map( Profile::getId );
+ final Stream<String> settingsProfiles = session.getSettings().getProfiles().stream()
+ .map( IdentifiableBase::getId );
+
+ return Stream.concat( projectProfiles, settingsProfiles ).collect( toSet() );
+ }
+
+ /**
+ * Check whether the required profiles were found in any of the projects we're building or the Maven settings.
+ * @param session the Maven session.
+ * @param profileActivation the requested optional and required profiles.
+ */
+ private void validateRequiredProfiles( MavenSession session, ProfileActivation profileActivation )
+ {
+ final Set<String> allDetectedProfiles = getAllProfiles( session );
+
+ final Set<String> requiredProfiles = new HashSet<>( );
+ requiredProfiles.addAll( profileActivation.getRequiredActiveProfileIds() );
+ requiredProfiles.addAll( profileActivation.getRequiredInactiveProfileIds() );
+
+ // Check whether the required profiles were found in any of the projects we're building.
+ final Set<String> notFoundRequiredProfiles = requiredProfiles.stream()
+ .filter( rap -> !allDetectedProfiles.contains( rap ) )
+ .collect( toSet() );
+
+ if ( !notFoundRequiredProfiles.isEmpty() )
{
- logger.warn( "The requested profile \"" + notActivatedProfileId
- + "\" could not be activated because it does not exist." );
+ final String profileIds = notFoundRequiredProfiles.stream()
+ .map( p -> String.format( "\"%s\"", p ) )
+ .collect( joining( ", " ) );
+ final String message = String.format(
+ "The requested profile(s) %s could not be activated or de-activated because they do not exist.",
Review comment:
I think this can be improved. Do you remember agreeing on common single quote for scalar values and brackets for collections. I n think case just a join with `[` and `]` will do and since your are emitting a collection you can always say "requested profiles ..."
##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -493,25 +508,88 @@ private void validatePrerequisitesForNonMavenPluginProjects( List<MavenProject>
}
}
- private void validateActivatedProfiles( List<MavenProject> projects, List<String> activeProfileIds )
+ /**
+ * Get all profiles that are detected in either the projects, any parent of the projects or the settings.
+ * @param session The Maven session
+ * @return A {@link Set} of profile identifiers, never {@code null}.
+ */
+ private Set<String> getAllProfiles( MavenSession session )
{
- Collection<String> notActivatedProfileIds = new LinkedHashSet<>( activeProfileIds );
-
- for ( MavenProject project : projects )
+ final Set<MavenProject> projectsIncludingParents = new HashSet<>();
+ for ( MavenProject project : session.getProjects() )
{
- for ( List<String> profileIds : project.getInjectedProfileIds().values() )
+ boolean isAdded = projectsIncludingParents.add( project );
+ MavenProject parent = project.getParent();
+ while ( isAdded && parent != null )
{
- notActivatedProfileIds.removeAll( profileIds );
+ isAdded = projectsIncludingParents.add( parent );
+ parent = parent.getParent();
}
}
- for ( String notActivatedProfileId : notActivatedProfileIds )
+ final Stream<String> projectProfiles = projectsIncludingParents.stream()
+ .map( MavenProject::getModel )
+ .map( Model::getProfiles )
+ .flatMap( Collection::stream )
+ .map( Profile::getId );
+ final Stream<String> settingsProfiles = session.getSettings().getProfiles().stream()
+ .map( IdentifiableBase::getId );
+
+ return Stream.concat( projectProfiles, settingsProfiles ).collect( toSet() );
+ }
+
+ /**
+ * Check whether the required profiles were found in any of the projects we're building or the Maven settings.
+ * @param session the Maven session.
+ * @param profileActivation the requested optional and required profiles.
+ */
+ private void validateRequiredProfiles( MavenSession session, ProfileActivation profileActivation )
+ {
+ final Set<String> allDetectedProfiles = getAllProfiles( session );
+
+ final Set<String> requiredProfiles = new HashSet<>( );
+ requiredProfiles.addAll( profileActivation.getRequiredActiveProfileIds() );
+ requiredProfiles.addAll( profileActivation.getRequiredInactiveProfileIds() );
+
+ // Check whether the required profiles were found in any of the projects we're building.
+ final Set<String> notFoundRequiredProfiles = requiredProfiles.stream()
+ .filter( rap -> !allDetectedProfiles.contains( rap ) )
+ .collect( toSet() );
+
+ if ( !notFoundRequiredProfiles.isEmpty() )
{
- logger.warn( "The requested profile \"" + notActivatedProfileId
- + "\" could not be activated because it does not exist." );
+ final String profileIds = notFoundRequiredProfiles.stream()
+ .map( p -> String.format( "\"%s\"", p ) )
+ .collect( joining( ", " ) );
+ final String message = String.format(
+ "The requested profile(s) %s could not be activated or de-activated because they do not exist.",
+ profileIds
+ );
+ addExceptionToResult( session.getResult(), new MissingProfileException( message ) );
}
}
+ /**
+ * Check whether any of the requested optional profiles were not activated or deactivated.
+ * @param session the Maven session.
+ * @param profileActivation the requested optional and required profiles.
+ */
+ private void validateOptionalProfiles( MavenSession session, ProfileActivation profileActivation )
+ {
+ final Set<String> allDetectedProfiles = getAllProfiles( session );
+
+ final Set<String> optionalProfiles = new HashSet<>( );
+ optionalProfiles.addAll( profileActivation.getOptionalActiveProfileIds() );
+ optionalProfiles.addAll( profileActivation.getOptionalInactiveProfileIds() );
+
+ final Set<String> notFoundOptionalProfiles = optionalProfiles.stream()
+ .filter( rap -> !allDetectedProfiles.contains( rap ) )
+ .collect( toSet() );
+
+ notFoundOptionalProfiles.forEach( profile -> logger.warn( "The requested profile \"" + profile
Review comment:
Please remember the single quote here too. Interestingly enough, for required profiles we have one exception with a collection, but for optional ones one statement per profile. Would you prefer to make this consistent anyhow?
##########
File path: maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java
##########
@@ -277,22 +277,62 @@
MavenExecutionRequest setProfiles( List<Profile> profiles );
+ /**
+ * @deprecated Use {@link #getProfileActivationRequest()}.
+ */
+ @Deprecated
MavenExecutionRequest addActiveProfile( String profile );
+ /**
+ * @deprecated Use {@link #getProfileActivationRequest()}.
+ */
+ @Deprecated
MavenExecutionRequest addActiveProfiles( List<String> profiles );
+ /**
+ * @deprecated Use {@link #getProfileActivationRequest()}.
+ */
+ @Deprecated
MavenExecutionRequest setActiveProfiles( List<String> profiles );
+ /**
+ * @return The list of profiles that the user wants to activate.
+ * @deprecated Use {@link #getProfileActivationRequest()}.
+ */
+ @Deprecated
List<String> getActiveProfiles();
+ /**
+ * @deprecated Use {@link #getProfileActivationRequest()}.
+ */
+ @Deprecated
MavenExecutionRequest addInactiveProfile( String profile );
+ /**
+ * @deprecated Use {@link #getProfileActivationRequest()}.
+ */
+ @Deprecated
MavenExecutionRequest addInactiveProfiles( List<String> profiles );
+ /**
+ * @deprecated Use {@link #getProfileActivationRequest()}.
+ */
+ @Deprecated
MavenExecutionRequest setInactiveProfiles( List<String> profiles );
+ /**
+ * @return The list of profiles that the user wants to de-activate.
+ * @deprecated Use {@link #getProfileActivationRequest()}.
+ */
+ @Deprecated
List<String> getInactiveProfiles();
+ /**
+ * Return the requested activation(s) of profile(s) in this execution.
+ * @return requested (de-)activation(s) of profile(s) in this execution. Never {@code null}.
+ */
+ ProfileActivation getProfileActivationRequest();
Review comment:
See comment on the implementation.
##########
File path: maven-core/src/main/java/org/apache/maven/MissingProfileException.java
##########
@@ -0,0 +1,32 @@
+package org.apache.maven;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Signals that the user referenced a Maven profile that could not be located in either the project or the settings.
+ */
+public class MissingProfileException
Review comment:
The exception name contradicts the passed message because it implies that one exception instance maps to one profile, yet you collect profiles into one exception.
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -1507,41 +1506,41 @@ else if ( projectAction.startsWith( "+" ) )
}
// Visible for testing
- static ProfileActivation determineProfileActivation( final CommandLine commandLine )
+ static void performProfileActivation( final CommandLine commandLine,
+ final ProfileActivation profileActivation )
{
- final ProfileActivation result = new ProfileActivation();
-
if ( commandLine.hasOption( CLIManager.ACTIVATE_PROFILES ) )
{
- String[] profileOptionValues = commandLine.getOptionValues( CLIManager.ACTIVATE_PROFILES );
- if ( profileOptionValues != null )
+ final String[] optionValues = commandLine.getOptionValues( CLIManager.ACTIVATE_PROFILES );
+
+ if ( optionValues == null )
Review comment:
Can this array also be empty instead of null?
##########
File path: maven-core/src/main/java/org/apache/maven/MissingProfileException.java
##########
@@ -0,0 +1,32 @@
+package org.apache.maven;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Signals that the user referenced a Maven profile that could not be located in either the project or the settings.
+ */
+public class MissingProfileException
+ extends Exception
Review comment:
should that rather be `IllegalStateException`?
##########
File path: maven-core/src/main/java/org/apache/maven/execution/ProfileActivation.java
##########
@@ -0,0 +1,180 @@
+package org.apache.maven.execution;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Predicate;
+
+import static java.util.stream.Collectors.toSet;
+
+/**
+ * Container for storing the request from the user to activate or de-activate certain profiles and optionally fail the
+ * build if those profiles do not exist.
+ */
+public class ProfileActivation
+{
+
+ private static class ActivationSettings
+ {
+ /** Should the profile be active? */
+ final boolean active;
+ /** Should the build continue if the profile is not present? */
+ final boolean optional;
+
+ ActivationSettings( final boolean active, final boolean optional )
+ {
+ this.active = active;
+ this.optional = optional;
+ }
+ }
+
+ private final Map<String, ActivationSettings> activations = new HashMap<>();
+
+ /**
+ * Mimics the pre-Maven 4 "active profiles" list.
+ */
+ @Deprecated
+ public List<String> getActiveProfiles()
+ {
+ return new ArrayList<>( getProfileIds( pa -> pa.active ) );
+ }
+
+ /**
+ * Mimics the pre-Maven 4 "inactive profiles" list.
+ */
+ @Deprecated
+ public List<String> getInactiveProfiles()
+ {
+ return new ArrayList<>( getProfileIds( pa -> !pa.active ) );
+ }
+
+ /**
+ * Overwrites the active profiles based on a pre-Maven 4 "active profiles" list.
+ * @param activeProfileIds A {@link List} of profile IDs that must be activated.
+ */
+ @Deprecated
+ public void overwriteActiveProfiles( List<String> activeProfileIds )
+ {
+ getActiveProfiles().forEach( this.activations::remove );
+ activeProfileIds.forEach( this::activateOptionalProfile );
+ }
+
+ /**
+ * Overwrites the inactive profiles based on a pre-Maven 4 "inactive profiles" list.
+ * @param inactiveProfileIds A {@link List} of profile IDs that must be deactivated.
+ */
+ @Deprecated
+ public void overwriteInactiveProfiles( List<String> inactiveProfileIds )
Review comment:
You introduce a totally new class, but instantly deprecate methods without documenting the replacement. Why?
##########
File path: maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java
##########
@@ -358,16 +352,18 @@ public MavenExecutionRequest setInactiveProfiles( List<String> inactiveProfiles
{
if ( inactiveProfiles != null )
{
- this.inactiveProfiles = new ArrayList<>( inactiveProfiles );
- }
- else
- {
- this.inactiveProfiles = null;
+ this.profileActivation.overwriteInactiveProfiles( inactiveProfiles );
}
return this;
}
+ @Override
+ public ProfileActivation getProfileActivationRequest()
Review comment:
Why request? The class name does not say anything about request.
##########
File path: maven-core/src/main/java/org/apache/maven/execution/ProfileActivation.java
##########
@@ -0,0 +1,180 @@
+package org.apache.maven.execution;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Predicate;
+
+import static java.util.stream.Collectors.toSet;
+
+/**
+ * Container for storing the request from the user to activate or de-activate certain profiles and optionally fail the
+ * build if those profiles do not exist.
+ */
+public class ProfileActivation
Review comment:
How is your stance in returning a list vs a set? Does it matter from a client's POV? Shouldn't it be a `Collection` rather? WDYT?
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -1507,41 +1506,41 @@ else if ( projectAction.startsWith( "+" ) )
}
// Visible for testing
- static ProfileActivation determineProfileActivation( final CommandLine commandLine )
+ static void performProfileActivation( final CommandLine commandLine,
+ final ProfileActivation profileActivation )
{
- final ProfileActivation result = new ProfileActivation();
-
if ( commandLine.hasOption( CLIManager.ACTIVATE_PROFILES ) )
{
- String[] profileOptionValues = commandLine.getOptionValues( CLIManager.ACTIVATE_PROFILES );
- if ( profileOptionValues != null )
+ final String[] optionValues = commandLine.getOptionValues( CLIManager.ACTIVATE_PROFILES );
+
+ if ( optionValues == null )
{
- for ( String profileOptionValue : profileOptionValues )
- {
- StringTokenizer profileTokens = new StringTokenizer( profileOptionValue, "," );
+ return;
+ }
- while ( profileTokens.hasMoreTokens() )
+ for ( final String optionValue : optionValues )
+ {
+ for ( String token : optionValue.split( "," ) )
+ {
+ String profileId = token.trim();
+ boolean active = true;
+ if ( profileId.charAt( 0 ) == '-' || profileId.charAt( 0 ) == '!' )
Review comment:
Side question: Can the hyphen cause ambiguous input during parsing?
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -1507,41 +1506,41 @@ else if ( projectAction.startsWith( "+" ) )
}
// Visible for testing
- static ProfileActivation determineProfileActivation( final CommandLine commandLine )
+ static void performProfileActivation( final CommandLine commandLine,
+ final ProfileActivation profileActivation )
{
- final ProfileActivation result = new ProfileActivation();
-
if ( commandLine.hasOption( CLIManager.ACTIVATE_PROFILES ) )
{
- String[] profileOptionValues = commandLine.getOptionValues( CLIManager.ACTIVATE_PROFILES );
- if ( profileOptionValues != null )
+ final String[] optionValues = commandLine.getOptionValues( CLIManager.ACTIVATE_PROFILES );
+
+ if ( optionValues == null )
{
- for ( String profileOptionValue : profileOptionValues )
- {
- StringTokenizer profileTokens = new StringTokenizer( profileOptionValue, "," );
+ return;
+ }
- while ( profileTokens.hasMoreTokens() )
+ for ( final String optionValue : optionValues )
+ {
+ for ( String token : optionValue.split( "," ) )
+ {
+ String profileId = token.trim();
+ boolean active = true;
+ if ( profileId.charAt( 0 ) == '-' || profileId.charAt( 0 ) == '!' )
{
- String profileAction = profileTokens.nextToken().trim();
-
- if ( profileAction.startsWith( "-" ) || profileAction.startsWith( "!" ) )
- {
- result.deactivate( profileAction.substring( 1 ) );
- }
- else if ( profileAction.startsWith( "+" ) )
- {
- result.activate( profileAction.substring( 1 ) );
- }
- else
- {
- result.activate( profileAction );
- }
+ active = false;
+ profileId = profileId.substring( 1 );
+ }
+ else if ( token.charAt( 0 ) == '+' )
Review comment:
Wow, did you find the plus being documented somewhere?
##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
##########
@@ -190,6 +190,8 @@ public void validateFileModel( Model m, ModelBuildingRequest request, ModelProbl
{
String prefix = "profiles.profile[" + profile.getId() + "].";
+ validateId( prefix, "id", problems, Severity.ERROR, Version.V37, profile.getId(), null, m );
Review comment:
It has to be `Version.V40`.
Please also note that:
```
private boolean isValidIdCharacter( char c )
{
return c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '-' || c == '_' || c == '.';
}
```
this needs to be tightened in the future.
##########
File path: maven-core/src/main/java/org/apache/maven/execution/ProfileActivation.java
##########
@@ -0,0 +1,180 @@
+package org.apache.maven.execution;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Predicate;
+
+import static java.util.stream.Collectors.toSet;
+
+/**
+ * Container for storing the request from the user to activate or de-activate certain profiles and optionally fail the
+ * build if those profiles do not exist.
+ */
+public class ProfileActivation
+{
+
+ private static class ActivationSettings
+ {
+ /** Should the profile be active? */
+ final boolean active;
+ /** Should the build continue if the profile is not present? */
+ final boolean optional;
+
+ ActivationSettings( final boolean active, final boolean optional )
+ {
+ this.active = active;
+ this.optional = optional;
+ }
+ }
+
+ private final Map<String, ActivationSettings> activations = new HashMap<>();
+
+ /**
+ * Mimics the pre-Maven 4 "active profiles" list.
+ */
+ @Deprecated
+ public List<String> getActiveProfiles()
+ {
+ return new ArrayList<>( getProfileIds( pa -> pa.active ) );
+ }
+
+ /**
+ * Mimics the pre-Maven 4 "inactive profiles" list.
+ */
+ @Deprecated
+ public List<String> getInactiveProfiles()
+ {
+ return new ArrayList<>( getProfileIds( pa -> !pa.active ) );
+ }
+
+ /**
+ * Overwrites the active profiles based on a pre-Maven 4 "active profiles" list.
+ * @param activeProfileIds A {@link List} of profile IDs that must be activated.
+ */
+ @Deprecated
+ public void overwriteActiveProfiles( List<String> activeProfileIds )
+ {
+ getActiveProfiles().forEach( this.activations::remove );
+ activeProfileIds.forEach( this::activateOptionalProfile );
+ }
+
+ /**
+ * Overwrites the inactive profiles based on a pre-Maven 4 "inactive profiles" list.
+ * @param inactiveProfileIds A {@link List} of profile IDs that must be deactivated.
+ */
+ @Deprecated
+ public void overwriteInactiveProfiles( List<String> inactiveProfileIds )
+ {
+ getInactiveProfiles().forEach( this.activations::remove );
+ inactiveProfileIds.forEach( this::deactivateOptionalProfile );
+ }
+
+ /**
+ * Mark a profile as required and activated.
+ * @param id The identifier of the profile.
+ */
+ public void activateRequiredProfile( String id )
+ {
+ this.activations.put( id, new ActivationSettings( true, false ) );
Review comment:
Since `ActivationSettings` is immutable after instantation would it make sense introduce pre-instantioned objects for them to avoid allocation?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org