Copilot commented on code in PR #1297:
URL:
https://github.com/apache/maven-site-plugin/pull/1297#discussion_r3810560267
##########
src/main/java/org/apache/maven/plugins/site/deploy/AbstractDeployMojo.java:
##########
@@ -519,6 +522,101 @@ private static String getFullName(MavenProject project) {
+ project.getVersion() + ')';
}
+ /**
+ * Extracts the provider-specific URL from an SCM URL for comparison
purposes.
+ * For non-SCM URLs, returns the original URL.
+ * For SCM URLs with SCP-like syntax (e.g., [email protected]:user/repo.git),
+ * converts them to a comparable format.
+ * For hierarchical SCM systems like SVN, normalizes the scheme to enable
+ * proper comparison of URLs that differ only in http vs https.
+ *
+ * @param url the URL to process
+ * @return the provider-specific URL for SCM URLs, or the original URL
otherwise
+ */
+ static String extractComparableUrl(String url) {
+ if (url != null && url.startsWith("scm:")) {
+ // Extract the SCM provider (e.g., "git", "svn")
+ String provider = ScmUrlUtils.getProvider(url);
+
+ // Extract the provider-specific part of the SCM URL
+ // For example: "scm:git:https://github.com/user/repo.git"; ->
"https://github.com/user/repo.git";
+ String providerSpecificPart =
ScmUrlUtils.getProviderSpecificPart(url);
Review Comment:
`ScmUrlUtils.getProvider(..)` / `getProviderSpecificPart(..)` can throw
`IllegalArgumentException` for malformed `scm:` URLs. In that case
`getTopLevelProject` would fail the build instead of treating the URL as a
distinct site (as described in the PR rationale). Consider catching and falling
back to the original URL so a malformed SCM URL doesn’t crash site deployment.
##########
src/main/java/org/apache/maven/plugins/site/deploy/AbstractDeployMojo.java:
##########
@@ -519,6 +522,101 @@ private static String getFullName(MavenProject project) {
+ project.getVersion() + ')';
}
+ /**
+ * Extracts the provider-specific URL from an SCM URL for comparison
purposes.
+ * For non-SCM URLs, returns the original URL.
+ * For SCM URLs with SCP-like syntax (e.g., [email protected]:user/repo.git),
+ * converts them to a comparable format.
+ * For hierarchical SCM systems like SVN, normalizes the scheme to enable
+ * proper comparison of URLs that differ only in http vs https.
+ *
+ * @param url the URL to process
+ * @return the provider-specific URL for SCM URLs, or the original URL
otherwise
+ */
+ static String extractComparableUrl(String url) {
+ if (url != null && url.startsWith("scm:")) {
+ // Extract the SCM provider (e.g., "git", "svn")
+ String provider = ScmUrlUtils.getProvider(url);
+
+ // Extract the provider-specific part of the SCM URL
+ // For example: "scm:git:https://github.com/user/repo.git"; ->
"https://github.com/user/repo.git";
+ String providerSpecificPart =
ScmUrlUtils.getProviderSpecificPart(url);
+ if (providerSpecificPart != null &&
!providerSpecificPart.isEmpty()) {
+ // Handle SCP-like Git syntax (e.g.,
[email protected]:user/repo.git or user@host:path)
+ // Convert it to a more standard format for comparison
+ // Note: This is a heuristic check - we look for the pattern
of user@host:path
+ // where the colon comes after the @ symbol and is followed by
a path
+ if (providerSpecificPart.contains("@")
+ && !providerSpecificPart.startsWith("http://";)
+ && !providerSpecificPart.startsWith("https://";)
+ && !providerSpecificPart.startsWith("ssh://")) {
+ // Find the @ symbol and look for the first : after it
that's not part of a URL scheme
+ int atIndex = providerSpecificPart.lastIndexOf('@');
+ int colonIndex = providerSpecificPart.indexOf(':',
atIndex);
+
+ // Verify this looks like SCP syntax: user@host:path
+ // The colon should come after @ and before the end
+ if (atIndex >= 0 && colonIndex > atIndex + 1 && colonIndex
< providerSpecificPart.length() - 1) {
+ String host = providerSpecificPart.substring(atIndex +
1, colonIndex);
+ String path =
providerSpecificPart.substring(colonIndex + 1);
+ // Convert to a pseudo-URL format for comparison
+ // Note: IPv6 addresses in brackets are handled by
this approach
+ // as the brackets will be preserved in the host part
+ return "ssh://" + host + "/" + path;
+ }
+ }
+
+ // For hierarchical VCS systems like SVN, normalize the scheme
to allow
+ // comparison of URLs that differ only in http vs https
+ // SVN repositories can be accessed via both protocols and
should be considered the same
+ if ("svn".equalsIgnoreCase(provider) &&
providerSpecificPart.startsWith("https://";)) {
+ // Normalize https to http for SVN URLs to enable proper
comparison
+ return "http" + providerSpecificPart.substring(5);
+ }
+
+ // Return the provider-specific part as-is for standard URLs or
+ // if SCP syntax conversion is not applicable
+ return providerSpecificPart;
+ }
+ }
+ return url;
+ }
+
+ /**
+ * Returns whether a child site lies within a parent site, which is what
makes them one site to deploy.
+ * The host must match and the child path must be the parent path or below
it.
+ * <p>
+ * Comparing host, scheme and port alone is not enough: two repositories
on the same forge, such as
+ * {@code github.com/org/parent.git} and {@code github.com/org/child.git},
share all three and are still
+ * unrelated sites.
+ *
+ * @param parentUri the site URI of the parent project
+ * @param childUri the site URI of the child project
+ * @return {@code true} if both URIs describe the same site
+ */
+ static boolean isSameSite(URI parentUri, URI childUri) {
+ if (!Objects.equals(parentUri.getHost(), childUri.getHost())) {
+ return false;
+ }
+
+ String parentPath = stripTrailingSlash(parentUri.getPath());
+ String childPath = stripTrailingSlash(childUri.getPath());
+ if (parentPath == null || childPath == null) {
+ // an opaque URI neither the SCM unwrapping nor URI parsing could
resolve; treat the sites as separate
+ return false;
+ }
+
+ // compare whole segments, so that /foo does not contain /foobar
+ return childPath.equals(parentPath) || childPath.startsWith(parentPath
+ "/");
+ }
Review Comment:
`isSameSite(..)` no longer checks scheme/port at all, whereas the previous
`URIPathDescriptor.sameSite(..)` logic did. This can cause unrelated
distribution URLs on the same host/path but different transport (e.g.
`dav:http` vs `scp`, or different explicit ports) to be treated as the same
site, potentially changing MSITE-600 behavior for non-SCM URLs. Consider making
scheme comparison conditional (skip only for SCM-derived URLs), while still
keeping a port check.
##########
src/main/java/org/apache/maven/plugins/site/deploy/AbstractDeployMojo.java:
##########
@@ -519,6 +522,101 @@ private static String getFullName(MavenProject project) {
+ project.getVersion() + ')';
}
+ /**
+ * Extracts the provider-specific URL from an SCM URL for comparison
purposes.
+ * For non-SCM URLs, returns the original URL.
+ * For SCM URLs with SCP-like syntax (e.g., [email protected]:user/repo.git),
+ * converts them to a comparable format.
+ * For hierarchical SCM systems like SVN, normalizes the scheme to enable
+ * proper comparison of URLs that differ only in http vs https.
+ *
+ * @param url the URL to process
+ * @return the provider-specific URL for SCM URLs, or the original URL
otherwise
+ */
+ static String extractComparableUrl(String url) {
+ if (url != null && url.startsWith("scm:")) {
+ // Extract the SCM provider (e.g., "git", "svn")
+ String provider = ScmUrlUtils.getProvider(url);
+
+ // Extract the provider-specific part of the SCM URL
+ // For example: "scm:git:https://github.com/user/repo.git"; ->
"https://github.com/user/repo.git";
+ String providerSpecificPart =
ScmUrlUtils.getProviderSpecificPart(url);
+ if (providerSpecificPart != null &&
!providerSpecificPart.isEmpty()) {
+ // Handle SCP-like Git syntax (e.g.,
[email protected]:user/repo.git or user@host:path)
+ // Convert it to a more standard format for comparison
+ // Note: This is a heuristic check - we look for the pattern
of user@host:path
+ // where the colon comes after the @ symbol and is followed by
a path
+ if (providerSpecificPart.contains("@")
+ && !providerSpecificPart.startsWith("http://";)
+ && !providerSpecificPart.startsWith("https://";)
+ && !providerSpecificPart.startsWith("ssh://")) {
+ // Find the @ symbol and look for the first : after it
that's not part of a URL scheme
+ int atIndex = providerSpecificPart.lastIndexOf('@');
+ int colonIndex = providerSpecificPart.indexOf(':',
atIndex);
+
Review Comment:
The SCP-like conversion logic claims to handle bracketed IPv6 hosts, but
`indexOf(':', atIndex)` will hit the first ':' inside the IPv6 literal (e.g.
`git@[2001:db8::1]:repo`), producing an incorrect host/path split. Either
handle bracketed IPv6 explicitly or drop the IPv6 guarantee.
##########
src/main/java/org/apache/maven/plugins/site/deploy/AbstractDeployMojo.java:
##########
@@ -550,10 +648,14 @@ protected MavenProject getTopLevelProject(MavenProject
project) throws MojoExecu
}
// MSITE-600
- URIPathDescriptor siteURI = new
URIPathDescriptor(URIEncoder.encodeURI(site.getUrl()), "");
- URIPathDescriptor oldSiteURI = new
URIPathDescriptor(URIEncoder.encodeURI(oldSite.getUrl()), "");
+ // MSITE-1033: For SCM URLs, extract the provider-specific part
for comparison
+ String siteUrlToCompare = extractComparableUrl(site.getUrl());
+ String oldSiteUrlToCompare =
extractComparableUrl(oldSite.getUrl());
+
+ URIPathDescriptor siteURI = new
URIPathDescriptor(URIEncoder.encodeURI(siteUrlToCompare), "");
+ URIPathDescriptor oldSiteURI = new
URIPathDescriptor(URIEncoder.encodeURI(oldSiteUrlToCompare), "");
- if (!siteURI.sameSite(oldSiteURI.getBaseURI())) {
+ if (!isSameSite(siteURI.getBaseURI(), oldSiteURI.getBaseURI())) {
return oldProject;
Review Comment:
After changing `isSameSite(..)` to compare scheme conditionally, the
`getTopLevelProject(..)` call site needs to decide when to ignore scheme (SCM
URLs) vs when to keep scheme comparison (regular distributionManagement URLs).
##########
src/test/java/org/apache/maven/plugins/site/deploy/AbstractDeployMojoTest.java:
##########
@@ -0,0 +1,231 @@
+/*
+ * 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.plugins.site.deploy;
+
+import org.apache.maven.model.DistributionManagement;
+import org.apache.maven.model.Site;
+import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.project.MavenProject;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * Tests for AbstractDeployMojo.
+ */
+public class AbstractDeployMojoTest {
+
+ /**
+ * Test that getTopLevelProject correctly handles SCM URLs with different
repositories.
+ * This is the test case for MSITE-1033.
+ */
+ @Test
+ public void testGetTopLevelProjectWithDifferentScmUrls() throws Exception {
+ // Create a mock deploy mojo
+ TestDeployMojo mojo = new TestDeployMojo();
+
+ // Create child project with SCM URL
+ MavenProject childProject =
+ createProjectWithSite("child",
"scm:git:[email protected]:codehaus-plexus/plexus-sec-dispatcher.git/");
+
+ // Create parent project with different SCM URL
+ MavenProject parentProject =
+ createProjectWithSite("parent",
"scm:git:https://github.com/codehaus-plexus/plexus-pom.git/";);
+
+ // Set up the parent-child relationship
+ childProject.setParent(parentProject);
+
+ // Call getTopLevelProject - it should return childProject, not
parentProject
+ // because the SCM URLs point to different repositories
+ MavenProject topProject = mojo.getTopLevelProject(childProject);
+
+ // The top project should be the child project itself since the parent
has a different site
+ assertEquals(childProject, topProject, "Top project should be child
project due to different SCM URLs");
+ }
+
+ /**
+ * Test that getTopLevelProject correctly handles SCM URLs with the same
repository.
+ */
+ @Test
+ public void testGetTopLevelProjectWithSameScmUrls() throws Exception {
+ // Create a mock deploy mojo
+ TestDeployMojo mojo = new TestDeployMojo();
+
+ // Create child project with SCM URL
+ MavenProject childProject =
+ createProjectWithSite("child",
"scm:git:https://github.com/codehaus-plexus/plexus-pom.git/child";);
+
+ // Create parent project with same base SCM URL
+ MavenProject parentProject =
+ createProjectWithSite("parent",
"scm:git:https://github.com/codehaus-plexus/plexus-pom.git/";);
+
+ // Set up the parent-child relationship
+ childProject.setParent(parentProject);
+
+ // Call getTopLevelProject - it should return parentProject
+ // because the SCM URLs point to the same repository
+ MavenProject topProject = mojo.getTopLevelProject(childProject);
+
+ // The top project should be the parent project since they share the
same site
+ assertEquals(parentProject, topProject, "Top project should be parent
project due to same SCM base URL");
+ }
+
+ /**
+ * Test that getTopLevelProject correctly handles non-SCM URLs.
+ */
+ @Test
+ public void testGetTopLevelProjectWithNonScmUrls() throws Exception {
+ // Create a mock deploy mojo
+ TestDeployMojo mojo = new TestDeployMojo();
+
+ // Create child project with regular URL
+ MavenProject childProject = createProjectWithSite("child",
"https://example.com/site/child";);
+
+ // Create parent project with same base URL
+ MavenProject parentProject = createProjectWithSite("parent",
"https://example.com/site/";);
+
+ // Set up the parent-child relationship
+ childProject.setParent(parentProject);
+
+ // Call getTopLevelProject - it should return parentProject
+ MavenProject topProject = mojo.getTopLevelProject(childProject);
+
+ // The top project should be the parent project since they share the
same site
+ assertEquals(parentProject, topProject, "Top project should be parent
project for regular URLs");
+ }
+
+ /**
+ * Test that getTopLevelProject correctly handles SCM URLs with standard
https format.
+ */
+ @Test
+ public void testGetTopLevelProjectWithHttpsScmUrls() throws Exception {
+ // Create a mock deploy mojo
+ TestDeployMojo mojo = new TestDeployMojo();
+
+ // Create child project with https SCM URL
+ MavenProject childProject = createProjectWithSite("child",
"scm:git:https://github.com/user/repo1.git/";);
+
+ // Create parent project with different https SCM URL (different
domain)
+ MavenProject parentProject = createProjectWithSite("parent",
"scm:git:https://gitlab.com/user/repo2.git/";);
+
+ // Set up the parent-child relationship
+ childProject.setParent(parentProject);
+
+ // Call getTopLevelProject - it should return childProject
+ // because the SCM URLs point to different repositories
+ MavenProject topProject = mojo.getTopLevelProject(childProject);
+
+ // The top project should be the child project itself since the parent
has a different site
+ assertEquals(childProject, topProject, "Top project should be child
project due to different https SCM URLs");
+ }
+
+ /**
+ * The reported case: sibling repositories on the same host. plexus-xml
inherits from plexus-pom,
+ * they share github.com, and their sites are unrelated.
+ */
+ @Test
+ public void testGetTopLevelProjectWithSiblingRepositoriesOnSameHost()
throws Exception {
+ TestDeployMojo mojo = new TestDeployMojo();
+
+ MavenProject childProject =
+ createProjectWithSite("child",
"scm:git:https://github.com/codehaus-plexus/plexus-xml.git";);
+ MavenProject parentProject =
+ createProjectWithSite("parent",
"scm:git:https://github.com/codehaus-plexus/plexus-pom.git";);
+
+ childProject.setParent(parentProject);
+
+ MavenProject topProject = mojo.getTopLevelProject(childProject);
+
+ assertEquals(childProject, topProject, "Sibling repositories do not
share a site");
+ }
+
+ /**
+ * Test that extractComparableUrl properly handles SVN URLs with different
schemes but same host.
+ * For SVN (hierarchical VCS), URLs with different schemes (http vs https)
should be normalized
+ * to the same scheme to allow URIPathDescriptor.sameSite() to recognize
them as the same site.
+ * Note: The paths may differ (one being a subpath of another), but as
long as scheme, host, and port
+ * are the same, URIPathDescriptor.sameSite() will correctly identify them
as the same site.
+ */
Review Comment:
This test’s Javadoc still refers to `URIPathDescriptor.sameSite()` even
though the production code now uses `AbstractDeployMojo.isSameSite(..)`.
Updating the comment would avoid misleading future readers about what behavior
is being verified.
--
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]
