From d19f5818ad19aec152ed84e5b102d064a99bd7bf Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Wed, 12 Feb 2025 15:06:22 -0700 Subject: [PATCH 1/3] Make remote polling test work again This essentially reverts 3b7cdffa7. In 2024, git ls-remote on a local repo works and the tests already depend on it in testPolling_CanDoRemotePollingIfOneBranchButMultipleRepositories. Add an assertion in testBasicRemotePoll to ensure that it's really testing remote polling. --- src/test/java/hudson/plugins/git/AbstractGitTestCase.java | 3 ++- src/test/java/hudson/plugins/git/GitSCMTest.java | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/java/hudson/plugins/git/AbstractGitTestCase.java b/src/test/java/hudson/plugins/git/AbstractGitTestCase.java index 5b3cd44207..d5f7e2985f 100644 --- a/src/test/java/hudson/plugins/git/AbstractGitTestCase.java +++ b/src/test/java/hudson/plugins/git/AbstractGitTestCase.java @@ -193,7 +193,8 @@ protected FreeStyleProject setupProject(List branches, boolean autho if (credential != null) { project.getBuildersList().add(new HasCredentialBuilder(credential.getId())); } - scm.getExtensions().add(new DisableRemotePoll()); // don't work on a file:// repository + if (!fastRemotePoll) + scm.getExtensions().add(new DisableRemotePoll()); if (relativeTargetDir!=null) scm.getExtensions().add(new RelativeTargetDirectory(relativeTargetDir)); if (excludedUsers!=null) diff --git a/src/test/java/hudson/plugins/git/GitSCMTest.java b/src/test/java/hudson/plugins/git/GitSCMTest.java index f9cf6fee2e..ac35631817 100644 --- a/src/test/java/hudson/plugins/git/GitSCMTest.java +++ b/src/test/java/hudson/plugins/git/GitSCMTest.java @@ -334,6 +334,8 @@ public void testBasicRemotePoll() throws Exception { assumeTrue("Test class max time " + MAX_SECONDS_FOR_THESE_TESTS + " exceeded", isTimeAvailable()); // FreeStyleProject project = setupProject("master", true, false); FreeStyleProject project = setupProject("master", false, null, null, null, true, null); + assertFalse("scm should not require workspace for polling", project.getScm().requiresWorkspaceForPolling()); + // create initial commit and then run the build against it: final String commitFile1 = "commitFile1"; commit(commitFile1, johnDoe, "Commit number 1"); From ea4629d1eefc46bd15508750a5a5c6f5ebacaab0 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Thu, 13 Feb 2025 13:01:57 -0700 Subject: [PATCH 2/3] Use workspace polling for empty branches When a single empty branch is encountered, getSingleBranch converts it to the ** wildcard spec like BranchSpec. Since that's not null, requiresWorkspaceForPolling says it can be used with remote polling. However, remote polling doesn't work well for wildcard branches since it doesn't have access to the remote history to prune old branches. Detect this special case and require workspace polling for it. I'm not sure why getSingleBranch goes to such effort to reject wildcard branches but then treats allows an empty branch as a wildcard. It seems like it should return null for an empty branch, but it also gets passed to getCandidateRevisions, which may expect those semantics. --- src/main/java/hudson/plugins/git/GitSCM.java | 4 +++- src/test/java/hudson/plugins/git/GitSCMUnitTest.java | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/plugins/git/GitSCM.java b/src/main/java/hudson/plugins/git/GitSCM.java index 5ccac995db..a2061edb04 100644 --- a/src/main/java/hudson/plugins/git/GitSCM.java +++ b/src/main/java/hudson/plugins/git/GitSCM.java @@ -680,7 +680,9 @@ boolean requiresWorkspaceForPolling(EnvVars environment) { for (GitSCMExtension ext : getExtensions()) { if (ext.requiresWorkspaceForPolling()) return true; } - return getSingleBranch(environment) == null; + + final String singleBranch = getSingleBranch(environment); + return singleBranch == null || singleBranch.equals("**"); } @Override diff --git a/src/test/java/hudson/plugins/git/GitSCMUnitTest.java b/src/test/java/hudson/plugins/git/GitSCMUnitTest.java index 96533247c7..659d66e805 100644 --- a/src/test/java/hudson/plugins/git/GitSCMUnitTest.java +++ b/src/test/java/hudson/plugins/git/GitSCMUnitTest.java @@ -275,7 +275,7 @@ public void testRequiresWorkspaceForPollingEmptyBranchName() throws Exception { GitSCM bigGitSCM = new GitSCM(createRepoList(repoURL, null), Collections.singletonList(new BranchSpec("${A}")), null, null, Collections.emptyList()); - assertFalse(bigGitSCM.requiresWorkspaceForPolling(env)); + assertTrue(bigGitSCM.requiresWorkspaceForPolling(env)); } @Test From efd3f8f1c0c68f4eb6b5ba31e474626a757123c2 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Thu, 13 Feb 2025 09:56:47 -0700 Subject: [PATCH 3/3] Allow remote polling to work with multiple simple branches As long as the branch specs can't match multiple remote branches, then remote polling can be used since it can (and already does) match each branch spec against the map returned from `git ls-remote`. Add a separate helper to determine if all branch specs are simple and use that to determine if workspace polling is required. --- src/main/java/hudson/plugins/git/GitSCM.java | 41 ++++++++++++++++--- .../java/hudson/plugins/git/GitSCMTest.java | 24 +++++++++++ .../hudson/plugins/git/GitSCMUnitTest.java | 26 +++++++++++- 3 files changed, 84 insertions(+), 7 deletions(-) diff --git a/src/main/java/hudson/plugins/git/GitSCM.java b/src/main/java/hudson/plugins/git/GitSCM.java index a2061edb04..34cf3d287e 100644 --- a/src/main/java/hudson/plugins/git/GitSCM.java +++ b/src/main/java/hudson/plugins/git/GitSCM.java @@ -664,6 +664,37 @@ private String getSingleBranch(EnvVars env) { return branch; } + /** + * If any branch can match a multiple remote branches, then workspace polling is required. + * + * Returns {@code true} if any branches require workspace polling. + */ + private boolean branchesRequireWorkspaceForPolling(@NonNull EnvVars env, @CheckForNull TaskListener listener) { + for (BranchSpec branchSpec : getBranches()) { + final String branch = branchSpec.getName(); + final String expandedBranch = env.expand(branch); + + // If the branch contains a wildcard anywhere other than a leading */ or is empty (which is intepreted as + // **), it can match multiple remote branches. + final String strippedBranch = expandedBranch.replaceAll("^\\*/", ""); + if (strippedBranch.equals("") || strippedBranch.contains("*")) { + if (listener != null) { + final PrintStream log = listener.getLogger(); + + if (branch.equals(expandedBranch)) { + log.printf("Branch '%s' requires workspace for polling%n", branch); + } else { + log.printf("Branch '%s' (expanded to '%s') requires workspace for polling%n", + branch, expandedBranch); + } + } + return true; + } + } + + return false; + } + @Override public SCMRevisionState calcRevisionsFromBuild(Run abstractBuild, FilePath workspace, Launcher launcher, TaskListener taskListener) throws IOException, InterruptedException { return SCMRevisionState.NONE; @@ -672,17 +703,15 @@ public SCMRevisionState calcRevisionsFromBuild(Run abstractBuild, FilePath @Override public boolean requiresWorkspaceForPolling() { // TODO would need to use hudson.plugins.git.util.GitUtils.getPollEnvironment - return requiresWorkspaceForPolling(new EnvVars()); + return requiresWorkspaceForPolling(new EnvVars(), null); } /* Package protected for test access */ - boolean requiresWorkspaceForPolling(EnvVars environment) { + boolean requiresWorkspaceForPolling(EnvVars environment, TaskListener listener) { for (GitSCMExtension ext : getExtensions()) { if (ext.requiresWorkspaceForPolling()) return true; } - - final String singleBranch = getSingleBranch(environment); - return singleBranch == null || singleBranch.equals("**"); + return branchesRequireWorkspaceForPolling(environment, listener); } @Override @@ -717,7 +746,7 @@ private PollingResult compareRemoteRevisionWithImpl(Job project, Launcher final String singleBranch = getSingleBranch(pollEnv); - if (!requiresWorkspaceForPolling(pollEnv)) { + if (!requiresWorkspaceForPolling(pollEnv, listener)) { final EnvVars environment = project instanceof AbstractProject ap ? GitUtils.getPollEnvironment(ap, workspace, launcher, listener, false) : new EnvVars(); diff --git a/src/test/java/hudson/plugins/git/GitSCMTest.java b/src/test/java/hudson/plugins/git/GitSCMTest.java index ac35631817..c8813cf1be 100644 --- a/src/test/java/hudson/plugins/git/GitSCMTest.java +++ b/src/test/java/hudson/plugins/git/GitSCMTest.java @@ -2630,6 +2630,30 @@ public void testPolling_CanDoRemotePollingIfOneBranchButMultipleRepositories() t assertFalse(pollingResult.hasChanges()); } + @Test + public void testPolling_CanDoRemotePollingIfMultipleBranches() throws Exception { + assumeTrue("Test class max time " + MAX_SECONDS_FOR_THESE_TESTS + " exceeded", isTimeAvailable()); + FreeStyleProject project = createFreeStyleProject(); + List branchSpecs = Arrays.asList( + new BranchSpec("*/master"), + new BranchSpec("*/stable")); + GitSCM scm = new GitSCM(createRemoteRepositories(), + branchSpecs, null, null, + Collections.emptyList()); + project.setScm(scm); + assertFalse("scm should not require workspace for polling", scm.requiresWorkspaceForPolling()); + + commit("commitFile1", johnDoe, "Commit number 1"); + git.branch("stable"); + + FreeStyleBuild first_build = project.scheduleBuild2(0, new Cause.UserIdCause()).get(); + r.assertBuildStatus(Result.SUCCESS, first_build); + + first_build.getWorkspace().deleteContents(); + PollingResult pollingResult = scm.poll(project, null, first_build.getWorkspace(), listener, null); + assertFalse(pollingResult.hasChanges()); + } + @Issue("JENKINS-24467") @Test public void testPolling_environmentValueAsEnvironmentContributingAction() throws Exception { diff --git a/src/test/java/hudson/plugins/git/GitSCMUnitTest.java b/src/test/java/hudson/plugins/git/GitSCMUnitTest.java index 659d66e805..71e5982fc9 100644 --- a/src/test/java/hudson/plugins/git/GitSCMUnitTest.java +++ b/src/test/java/hudson/plugins/git/GitSCMUnitTest.java @@ -261,6 +261,30 @@ public void testRequiresWorkspaceForPollingMultiBranch() throws Exception { List branches = new ArrayList<>(); branches.add(new BranchSpec("master")); branches.add(new BranchSpec("origin/master")); + GitSCM bigGitSCM = new GitSCM(createRepoList(repoURL, null), + branches, + null, null, Collections.emptyList()); + assertFalse(bigGitSCM.requiresWorkspaceForPolling()); + } + + @Test + public void testRequiresWorkspaceForPollingMultiBranchWithWildcardRemoteName() throws Exception { + /* Multi-branch use case */ + List branches = new ArrayList<>(); + branches.add(new BranchSpec("master")); + branches.add(new BranchSpec("*/master")); + GitSCM bigGitSCM = new GitSCM(createRepoList(repoURL, null), + branches, + null, null, Collections.emptyList()); + assertFalse(bigGitSCM.requiresWorkspaceForPolling()); + } + + @Test + public void testRequiresWorkspaceForPollingMultiBranchWithWildcardSuffix() throws Exception { + /* Multi-branch use case */ + List branches = new ArrayList<>(); + branches.add(new BranchSpec("master")); + branches.add(new BranchSpec("*/stable*")); GitSCM bigGitSCM = new GitSCM(createRepoList(repoURL, null), branches, null, null, Collections.emptyList()); @@ -275,7 +299,7 @@ public void testRequiresWorkspaceForPollingEmptyBranchName() throws Exception { GitSCM bigGitSCM = new GitSCM(createRepoList(repoURL, null), Collections.singletonList(new BranchSpec("${A}")), null, null, Collections.emptyList()); - assertTrue(bigGitSCM.requiresWorkspaceForPolling(env)); + assertTrue(bigGitSCM.requiresWorkspaceForPolling(env, null)); } @Test