Tarun4201 commented on code in PR #13254:
URL: https://github.com/apache/maven/pull/13254#discussion_r4085557253
##########
apache-maven/src/assembly/maven/bin/mvn:
##########
@@ -462,6 +462,42 @@ if $cygwin || $mingw ; then
fi
MAVEN_PROJECTBASEDIR="$MAVEN_PROJECTBASEDIR_NATIVE"
+# MNG-8056: Under Cygwin, the JVM is a native Windows process and cannot
resolve
+# Cygwin-style POSIX paths (e.g. /cygdrive/c/...) directly. The internal paths
+# (MAVEN_HOME, CLASSWORLDS_CONF, JAVA_HOME, …) are already converted above, but
+# user-supplied paths passed via -f/--file, -s/--settings,
-gs/--global-settings, -t/--toolchains, -gt/--global-toolchains,
+# -ps/--project-settings, -is/--install-settings, -it/--install-toolchains,
+# -l/--log-file, and -af/--at-file reach the JVM unconverted.
+# The loop below rewrites only those argument values that begin with '/'
(absolute
+# POSIX paths); relative paths and already-Windows paths are left unchanged.
+# MinGW/MSYS2 perform automatic path mangling, so the fix is Cygwin-only.
+if $cygwin ; then
+ _np=false
+ _count=$#
+ while [ $_count -gt 0 ]; do
+ _a="$1"
+ shift
+ if $_np; then
+ case "$_a" in
+ /*) _a=$(cygpath --windows "$_a") ;;
+ esac
+ _np=false
+ else
+ case "$_a" in
+
-f|--file|-s|--settings|-gs|--global-settings|-t|--toolchains|-gt|--global-toolchains|-ps|--project-settings|-is|--install-settings|-it|--install-toolchains|-l|--log-file|-af|--at-file)
+ _np=true ;;
+
--file=/*|--settings=/*|--global-settings=/*|--toolchains=/*|--global-toolchains=/*|--project-settings=/*|--install-settings=/*|--install-toolchains=/*|--log-file=/*|--at-file=/*)
+ _flag="${_a%%=*}"
+ _path="${_a#*=}"
+ _a="${_flag}=$(cygpath --windows "${_path}")"
+ ;;
+ esac
+ fi
+ set -- "$@" "$_a"
+ _count=$(( _count - 1 ))
Review Comment:
Fixed. The branch has been rebased cleanly onto the current `master`
(fetched from upstream). The MNG-8056 Cygwin block no longer appears in the
diff — the PR now shows only 2 files changed (DefaultProfileSelector.java and
DefaultProfileSelectorTest.java).
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileSelector.java:
##########
@@ -88,6 +100,12 @@ public List<Profile> getActiveProfiles(
activeProfiles.addAll(activePomProfilesByDefault);
}
+ // MNG-6787: only include external activeByDefault profiles when no
profile
+ // has been explicitly activated on the command line via -P.
+ if (!anyProfileExplicitlyActivated) {
+ activeProfiles.addAll(activeExternalProfilesByDefault);
Review Comment:
Added. `DefaultProfileSelectorTest` now lives directly in `impl/maven-impl`
under `org.apache.maven.impl.model`, testing `DefaultProfileSelector` (not the
legacy compat one). Four tests are now present:
- `externalActiveByDefaultSuppressedWhenProfileExplicitlyActivated`
- `externalActiveByDefaultSuppressedWhenPomProfileExplicitlyActivated`
*(new, see thread below)*
- `externalActiveByDefaultReturnedWhenNoProfileExplicitlyActivated`
- `pomActiveByDefaultSuppressedWhenOtherPomProfileExplicitlyActivated`
*(regression guard)*
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileSelector.java:
##########
@@ -65,20 +65,32 @@ public List<Profile> getActiveProfiles(
Collection<Profile> profiles, ProfileActivationContext context,
ModelProblemCollector problems) {
List<Profile> activeProfiles = new ArrayList<>(profiles.size());
List<Profile> activePomProfilesByDefault = new ArrayList<>();
+ List<Profile> activeExternalProfilesByDefault = new ArrayList<>();
boolean activatedPomProfileNotByDefault = false;
+ // MNG-6787: track whether any profile was explicitly activated via -P
so that
+ // activeByDefault profiles from *all* sources (POM and external, e.g.
settings.xml)
+ // are correctly suppressed when the user requests a specific profile.
+ boolean anyProfileExplicitlyActivated = false;
for (Profile profile : profiles) {
if (!context.isProfileInactive(profile.getId())) {
if (context.isProfileActive(profile.getId()) ||
isActive(profile, context, problems)) {
activeProfiles.add(profile);
+ if (context.isProfileActive(profile.getId())) {
+ // Profile was explicitly requested via -P
+ anyProfileExplicitlyActivated = true;
Review Comment:
Documented inline with two distinct comments above each flag:
- `activatedPomProfileNotByDefault` — set by *any* POM profile activation
(explicit `-P` or a condition activator such as file/property/OS). Suppresses
POM `activeByDefault` profiles.
- `anyProfileExplicitlyActivated` — set only by strict `-P` /
`--activate-profiles`. Suppresses external (settings.xml) `activeByDefault`
profiles.
The asymmetry is intentional per the MNG-6787 spec and is now explained in
code.
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultProfileSelectorTest.java:
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.
+ */
+package org.apache.maven.impl.model;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.maven.api.model.Activation;
+import org.apache.maven.api.model.Profile;
+import org.apache.maven.api.services.ModelProblemCollector;
+import org.apache.maven.api.services.model.ProfileActivationContext;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests {@link DefaultProfileSelector}.
+ */
+class DefaultProfileSelectorTest {
+
+ private Profile profile(String id, String source) {
+ return Profile.newBuilder().id(id).source(source).build();
+ }
+
+ private Profile activeByDefaultProfile(String id, String source) {
+ Activation activation =
Activation.newBuilder().activeByDefault(true).build();
+ return
Profile.newBuilder().id(id).source(source).activation(activation).build();
+ }
+
+ private ProfileActivationContext contextWithActiveIds(String... activeIds)
{
+ List<String> active = Arrays.asList(activeIds);
+ return new ProfileActivationContext() {
+ public boolean isProfileActive(String profileId) { return
active.contains(profileId); }
+ public boolean isProfileInactive(String profileId) { return false;
}
+ public String getSystemProperty(String key) { return null; }
+ public String getUserProperty(String key) { return null; }
+ public String getModelProperty(String key) { return null; }
+ public String getModelArtifactId() { return null; }
+ public String getModelPackaging() { return null; }
+ public String getModelRootDirectory() { return null; }
+ public String getModelBaseDirectory() { return null; }
+ public String interpolatePath(String path) { return path; }
+ public boolean exists(String path, boolean glob) { return false; }
+ };
+ }
+
+ private ModelProblemCollector noopCollector() {
+ return (severity, version, message, location, cause) -> {};
+ }
+
+ /**
+ * MNG-6787: when -P is used, external activeByDefault profiles
(settings.xml)
+ * must NOT be active.
+ */
+ @Test
+ void externalActiveByDefaultSuppressedWhenProfileExplicitlyActivated() {
+ DefaultProfileSelector selector = new DefaultProfileSelector();
+ Profile explicitProfile = profile("explicit", Profile.SOURCE_SETTINGS);
+ Profile defaultProfile = activeByDefaultProfile("defaults",
Profile.SOURCE_SETTINGS);
+
+ List<Profile> active = selector.getActiveProfiles(
+ Arrays.asList(explicitProfile, defaultProfile),
+ contextWithActiveIds("explicit"),
+ noopCollector());
+
+ assertEquals(1, active.size(), "Only the explicitly activated profile
should be active");
+ assertEquals("explicit", active.get(0).getId());
+ assertFalse(active.stream().anyMatch(p ->
"defaults".equals(p.getId())),
+ "activeByDefault external profile must be suppressed when -P
is used");
+ }
+
+ /**
+ * MNG-6787: when NO -P is used, external activeByDefault profiles must be
active.
+ */
+ @Test
+ void externalActiveByDefaultReturnedWhenNoProfileExplicitlyActivated() {
+ DefaultProfileSelector selector = new DefaultProfileSelector();
+ Profile defaultProfile = activeByDefaultProfile("defaults",
Profile.SOURCE_SETTINGS);
+
+ List<Profile> active = selector.getActiveProfiles(
+ Collections.singletonList(defaultProfile),
+ contextWithActiveIds(),
+ noopCollector());
+
+ assertEquals(1, active.size(), "activeByDefault external profile must
be active when no -P given");
+ assertEquals("defaults", active.get(0).getId());
+ }
+
+ /**
+ * Regression: POM activeByDefault profiles must still be suppressed when
+ * another POM profile is explicitly activated.
+ */
+ @Test
+ void pomActiveByDefaultSuppressedWhenOtherPomProfileExplicitlyActivated() {
+ DefaultProfileSelector selector = new DefaultProfileSelector();
+ Profile explicitPomProfile = profile("explicit", Profile.SOURCE_POM);
+ Profile defaultPomProfile = activeByDefaultProfile("pom-default",
Profile.SOURCE_POM);
+
+ List<Profile> active = selector.getActiveProfiles(
+ Arrays.asList(explicitPomProfile, defaultPomProfile),
+ contextWithActiveIds("explicit"),
+ noopCollector());
+
+ assertTrue(active.stream().anyMatch(p ->
"explicit".equals(p.getId())));
+ assertFalse(active.stream().anyMatch(p ->
"pom-default".equals(p.getId())),
+ "POM activeByDefault must be suppressed when another POM
profile is activated");
+ }
+}
Review Comment:
Added `externalActiveByDefaultSuppressedWhenPomProfileExplicitlyActivated`
as the 4th test. It activates a POM-sourced profile via `-P` and asserts that
an external `activeByDefault` profile is suppressed, validating that
`anyProfileExplicitlyActivated` fires regardless of the activated profile's
source. This prevents silent regressions if the flag is ever scoped.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
