You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "slachiewicz (via GitHub)" <gi...@apache.org> on 2026/08/19 06:11:55 UTC
[PR] [MSITE-1033] Detect inheritance for SCM site URLs [maven-site-plugin]
slachiewicz opened a new pull request, #1297:
URL: https://github.com/apache/maven-site-plugin/pull/1297
Fixes #1159. Same approach as #227, rebased onto current master, which #227 no longer merges cleanly against — credit to @kwin for the diagnosis and for the unwrap-then-compare shape.
`getTopLevelProject` compares `distributionManagement.site.url` through `URIPathDescriptor.sameSite()`, which needs a hierarchical URI. An SCM URL such as `scm:git:https://github.com/org/repo.git` is opaque, so scheme, host and port parse as `scm`, `null` and `-1`: every SCM URL matches every other, and a project inherits a top-level project it shares nothing with.
Two parts, and the second is easy to miss:
1. Unwrap the provider-specific part through `ScmUrlUtils`, converting SCP-like `git@host:path` to `ssh://host/path` and normalising svn `https` to `http`, so the two access schemes of one repository still match.
2. Compare host **and** path. Unwrapping alone leaves two repositories on one forge sharing scheme, host and port, so `sameSite()` still calls them one site. The child path must lie under the parent path, whole segments only, so `/foo` does not contain `/foobar`.
A URL that neither unwrapping nor URI parsing resolves counts as a separate site, which leaves a project deploying its own site rather than merging it into an unrelated one.
Six unit tests, including the sibling-repository case that fails with unwrapping alone.
Verified: with 3.22.1-SNAPSHOT, plexus-xml — a single-module project whose site URL is an SCM URL — stages into `target/staging` with 216 files, where 3.22.0 staged into `target/staging/../plexus-xml.git` with none. That is the downstream report in codehaus-plexus/plexus-pom#333.
*This change was created with AI assistance.*
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MSITE-1033] Detect inheritance for SCM site URLs [maven-site-plugin]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1297:
URL: https://github.com/apache/maven-site-plugin/pull/1297#issuecomment-5561808054
@slachiewicz The PR can't be associated to a milestone, because there are multiple open milestones. Please add the text "branch: master" to the description to the milestone where this PR belongs to.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MSITE-1033] Detect inheritance for SCM site URLs [maven-site-plugin]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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., git@github.com: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., git@github.com: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., git@github.com: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., git@github.com: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., git@github.com: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:git@github.com: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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MSITE-1033] Detect inheritance for SCM site URLs [maven-site-plugin]
Posted by "slachiewicz (via GitHub)" <gi...@apache.org>.
slachiewicz commented on PR #1297:
URL: https://github.com/apache/maven-site-plugin/pull/1297#issuecomment-5471709719
@Copilot-pull-request-reviewer re: your 5 review comments on #1297. 1/ Malformed scm: URL: valid — catch IndexOutOfBoundsException in extractComparableUrl, fallback to original URL so build doesn't crash. 2/ Scheme decision at call site: already addressed in code: scopes strict check to SCM URLs only, non-SCM keeps URIPathDescriptor.sameSite() — no MSITE-600 regression. 3/ Test javadoc: valid — references URIPathDescriptor.sameSite() but SCM URLs compare via isSameRepositoryPath; reword javadoc. 4/ IPv6 bracket: valid — indexOf(':') inside brackets breaks; add bracket guard before SCP conversion or skip IPv6 case. 5/ MSITE-600 regression: invalid — isSameSite() doesn't exist; design deliberately scopes strict check to SCM URLs only; non-SCM URLs retain original semantics. Summary: fixes 1,3,4 worth it; 2 already handled; 5 misunderstanding of design.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MSITE-1033] Detect inheritance for SCM site URLs [maven-site-plugin]
Posted by "slachiewicz (via GitHub)" <gi...@apache.org>.
slachiewicz merged PR #1297:
URL: https://github.com/apache/maven-site-plugin/pull/1297
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org