From 2a91c9512b1e76d3e5558d6393cce7a41ec76a80 Mon Sep 17 00:00:00 2001 From: kuc2hi Date: Mon, 19 Jun 2023 09:06:22 +0200 Subject: [PATCH 01/17] Support custom refspec in lightweight checkout Let user specify a custom refspec to fetch. Add support for branch names FETCH_HEAD or a commit hash. This allows to use lightweight checkout for: * Fetch the pipeline from a Gerrit change (refspec: refs/changes/*/change/patchset and branch: FETCH_HEAD). * Fetch the pipeline from a fixed commit of a branch, instead of head (refspec: branch and branch: commit hash). --- .../jenkins/plugins/git/GitSCMFileSystem.java | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index a8998fb229..139e884c08 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -60,6 +60,9 @@ import java.util.concurrent.locks.Lock; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import jenkins.scm.api.SCMFile; import jenkins.scm.api.SCMFileSystem; import jenkins.scm.api.SCMHead; @@ -400,13 +403,36 @@ public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull } HeadNameResult headNameResult = HeadNameResult.calculate(branchSpec, rev, env); + String head = Constants.R_REMOTES + remoteName + "/" + headNameResult.headName; + + if (branchSpec.getName().equals("FETCH_HEAD")) { + head = "FETCH_HEAD"; + } else { + // check for a commit hash in branchSpec + final String regex = "^[a-fA-F0-9]{40}$"; + final Pattern pattern = Pattern.compile(regex, Pattern.MULTILINE); + final Matcher matcher = pattern.matcher(branchSpec.getName()); + + if (matcher.find()) { + head = branchSpec.getName(); + rev = new AbstractGitSCMSource.SCMRevisionImpl(new SCMHead(headNameResult.headName), head); + } + } + + String refspec = config.getRefspec(); + if (refspec != null) { + if (env != null) { + refspec = env.expand(refspec); + } + } else { + refspec = "+" + headNameResult.prefix + headNameResult.headName + ":" + Constants.R_REMOTES + remoteName + "/" + + headNameResult.headName; + } - client.fetch_().prune(true).from(remoteURI, Collections.singletonList(new RefSpec( - "+" + headNameResult.prefix + headNameResult.headName + ":" + Constants.R_REMOTES + remoteName + "/" - + headNameResult.headName))).execute(); + client.fetch_().prune(true).from(remoteURI, Collections.singletonList(new RefSpec(refspec))).execute(); listener.getLogger().println("Done."); - return new GitSCMFileSystem(client, remote, Constants.R_REMOTES + remoteName + "/" + headNameResult.headName, (AbstractGitSCMSource.SCMRevisionImpl) rev); + return new GitSCMFileSystem(client, remote, head, (AbstractGitSCMSource.SCMRevisionImpl) rev); } finally { cacheLock.unlock(); } From d20c7160fc77f57b5e836b0319b85b3690eff2e1 Mon Sep 17 00:00:00 2001 From: kuc2hi Date: Wed, 21 Jun 2023 10:00:38 +0200 Subject: [PATCH 02/17] Move logic inside HeadNameResult.calculate() --- .../jenkins/plugins/git/GitSCMFileSystem.java | 87 +++++---- .../plugins/git/GitSCMFileSystemTest.java | 182 ++++++++++++++---- 2 files changed, 190 insertions(+), 79 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index 139e884c08..da1ae2e52c 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -290,39 +290,76 @@ public boolean supportsDescriptor(SCMSourceDescriptor descriptor) { static class HeadNameResult { final String headName; - final String prefix; + final String remoteHeadName; + final String refspec; + final SCMRevision rev; - private HeadNameResult(String headName, String prefix) { + private HeadNameResult(String headName, String remoteHeadName, String refspec, SCMRevision rev) { this.headName = headName; - this.prefix = prefix; + this.remoteHeadName = remoteHeadName; + this.refspec = refspec; + this.rev = rev; } static HeadNameResult calculate(@NonNull BranchSpec branchSpec, @CheckForNull SCMRevision rev, - @CheckForNull EnvVars env) { + @NonNull String refSpec, + @CheckForNull EnvVars env, + @CheckForNull String remoteName) { + String branchSpecExpandedName = branchSpec.getName(); if (env != null) { branchSpecExpandedName = env.expand(branchSpecExpandedName); } + String refspecExpandedName = refSpec; + if (env != null) { + refspecExpandedName = env.expand(refspecExpandedName); + } + // default to a branch (refs/heads) String prefix = Constants.R_HEADS; + // check for a tag if (branchSpecExpandedName.startsWith(Constants.R_TAGS)) { prefix = Constants.R_TAGS; + } else { + // check for FETCH_HEAD + if (branchSpecExpandedName.equals("FETCH_HEAD") && !refspecExpandedName.equals("")) { + prefix = null; + } else { + // check for commit-id + final String regex = "^[a-fA-F0-9]{40}$"; + final Pattern pattern = Pattern.compile(regex, Pattern.MULTILINE); + final Matcher matcher = pattern.matcher(branchSpecExpandedName); + + if (matcher.find()) { + // commit-id + prefix = null; + rev = new AbstractGitSCMSource.SCMRevisionImpl(new SCMHead(branchSpecExpandedName), branchSpecExpandedName); + } + } } - String headName; + String headName = branchSpecExpandedName; if (rev != null && env != null) { headName = env.expand(rev.getHead().getName()); } else { - if (branchSpecExpandedName.startsWith(prefix)) { + if (prefix != null && branchSpecExpandedName.startsWith(prefix)) { headName = branchSpecExpandedName.substring(prefix.length()); } else if (branchSpecExpandedName.startsWith("*/")) { headName = branchSpecExpandedName.substring(2); - } else { - headName = branchSpecExpandedName; } } - return new HeadNameResult(headName, prefix); + + if (refspecExpandedName == null || refspecExpandedName.equals("")) { + refspecExpandedName = "+" + prefix + headName + ":" + Constants.R_REMOTES + remoteName + "/" + headName; + } + + String remoteHead = headName; + if (prefix == Constants.R_HEADS || prefix == Constants.R_TAGS) { + remoteHead = Constants.R_REMOTES + remoteName + "/" + headName; + } + + return new HeadNameResult(headName, remoteHead, refspecExpandedName, rev); } } @@ -402,37 +439,11 @@ public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull listener.getLogger().println("URI syntax exception for '" + remoteName + "' " + ex); } - HeadNameResult headNameResult = HeadNameResult.calculate(branchSpec, rev, env); - String head = Constants.R_REMOTES + remoteName + "/" + headNameResult.headName; - - if (branchSpec.getName().equals("FETCH_HEAD")) { - head = "FETCH_HEAD"; - } else { - // check for a commit hash in branchSpec - final String regex = "^[a-fA-F0-9]{40}$"; - final Pattern pattern = Pattern.compile(regex, Pattern.MULTILINE); - final Matcher matcher = pattern.matcher(branchSpec.getName()); - - if (matcher.find()) { - head = branchSpec.getName(); - rev = new AbstractGitSCMSource.SCMRevisionImpl(new SCMHead(headNameResult.headName), head); - } - } - - String refspec = config.getRefspec(); - if (refspec != null) { - if (env != null) { - refspec = env.expand(refspec); - } - } else { - refspec = "+" + headNameResult.prefix + headNameResult.headName + ":" + Constants.R_REMOTES + remoteName + "/" - + headNameResult.headName; - } - - client.fetch_().prune(true).from(remoteURI, Collections.singletonList(new RefSpec(refspec))).execute(); + HeadNameResult headNameResult = HeadNameResult.calculate(branchSpec, rev, config.getRefspec(), env, remoteName); + client.fetch_().prune(true).from(remoteURI, Collections.singletonList(new RefSpec(headNameResult.refspec))).execute(); listener.getLogger().println("Done."); - return new GitSCMFileSystem(client, remote, head, (AbstractGitSCMSource.SCMRevisionImpl) rev); + return new GitSCMFileSystem(client, remote, headNameResult.remoteHeadName, (AbstractGitSCMSource.SCMRevisionImpl) headNameResult.rev); } finally { cacheLock.unlock(); } diff --git a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java index 64eb3718d5..c6eb4fa171 100644 --- a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java +++ b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java @@ -32,10 +32,15 @@ import hudson.plugins.git.GitException; import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; +import java.util.List; import java.util.Set; import java.util.TreeSet; + +import hudson.plugins.git.UserRemoteConfig; import jenkins.scm.api.SCMFile; import jenkins.scm.api.SCMFileSystem; import jenkins.scm.api.SCMHead; @@ -396,29 +401,72 @@ public void create_SCMFileSystem_from_tag() throws Exception { sampleRepo.git("commit", "--all", "--message=dev"); sampleRepo.git("tag", "v1.0"); SCMFileSystem fs = SCMFileSystem.of(r.createFreeStyleProject(), new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), Collections.singletonList(new BranchSpec("refs/tags/v1.0")), null, null, Collections.emptyList())); + assertEquals("modified", getFileContent(fs, "dir/subdir/file", "modified")); + } + + public static List createRepoListWithRefspec(String url, String refspec) { + List repoList = new ArrayList<>(); + repoList.add(new UserRemoteConfig(url, null, refspec, null)); + return repoList; + } + + public String getFileContent(SCMFileSystem fs, String path, String expectedContent) throws IOException, InterruptedException { assertThat(fs, notNullValue()); assertThat(fs.getRoot(), notNullValue()); - Iterable children = fs.getRoot().children(); - Iterator iterator = children.iterator(); - assertThat(iterator.hasNext(), is(true)); - SCMFile dir = iterator.next(); - assertThat(iterator.hasNext(), is(false)); - assertThat(dir.getName(), is("dir")); - assertThat(dir.getType(), is(SCMFile.Type.DIRECTORY)); - children = dir.children(); - iterator = children.iterator(); - assertThat(iterator.hasNext(), is(true)); - SCMFile subdir = iterator.next(); - assertThat(iterator.hasNext(), is(false)); - assertThat(subdir.getName(), is("subdir")); - assertThat(subdir.getType(), is(SCMFile.Type.DIRECTORY)); - children = subdir.children(); - iterator = children.iterator(); - assertThat(iterator.hasNext(), is(true)); - SCMFile file = iterator.next(); - assertThat(iterator.hasNext(), is(false)); - assertThat(file.getName(), is("file")); - assertThat(file.contentAsString(), is("modified")); + String[] segments = path.split("/"); + + SCMFile parent = fs.getRoot(); + SCMFile file = null; + for (String segment: segments) { + Iterable children = parent.children(); + Iterator iterator = children.iterator(); + if (segment == segments[segments.length - 1]) { + // file + assertThat(iterator.hasNext(), is(true)); + SCMFile t = iterator.next(); + assertThat(iterator.hasNext(), is(false)); + assertThat(t.getName(), is(segment)); + assertThat(t.getType(), is(SCMFile.Type.REGULAR_FILE)); + file = t; + } else { + // dir + assertThat(iterator.hasNext(), is(true)); + SCMFile dir = iterator.next(); + assertThat(iterator.hasNext(), is(false)); + assertThat(dir.getName(), is(segment)); + assertThat(dir.getType(), is(SCMFile.Type.DIRECTORY)); + parent = dir; + } + } + return file.contentAsString(); + } + + @Test + public void create_SCMFileSystem_from_commit() throws Exception { + sampleRepo.init(); + sampleRepo.git("checkout", "-b", "dev"); + sampleRepo.mkdirs("dir/subdir"); + sampleRepo.git("mv", "file", "dir/subdir/file"); + sampleRepo.write("dir/subdir/file", "modified"); + sampleRepo.git("commit", "--all", "--message=dev"); + String modifiedCommit = sampleRepo.head(); + sampleRepo.write("dir/subdir/file", "modified again"); + sampleRepo.git("commit", "--all", "--message=dev"); + SCMFileSystem fs = SCMFileSystem.of(r.createFreeStyleProject(), new GitSCM(createRepoListWithRefspec(sampleRepo.toString(), "dev"), Collections.singletonList(new BranchSpec(modifiedCommit)), null, null, Collections.emptyList())); + assertEquals(modifiedCommit, fs.getRevision().toString()); + assertEquals("modified", getFileContent(fs, "dir/subdir/file", "modified")); + } + + @Test + public void create_SCMFileSystem_from_FETCH_HEAD() throws Exception { + sampleRepo.init(); + sampleRepo.git("checkout", "-b", "dev"); + sampleRepo.mkdirs("dir/subdir"); + sampleRepo.git("mv", "file", "dir/subdir/file"); + sampleRepo.write("dir/subdir/file", "modified"); + sampleRepo.git("commit", "--all", "--message=dev"); + SCMFileSystem fs = SCMFileSystem.of(r.createFreeStyleProject(), new GitSCM(createRepoListWithRefspec(sampleRepo.toString(), "dev"), Collections.singletonList(new BranchSpec("FETCH_HEAD")), null, null, Collections.emptyList())); + assertEquals("modified", getFileContent(fs, "dir/subdir/file", "modified")); } @Issue("JENKINS-52964") @@ -431,35 +479,87 @@ public void filesystem_supports_descriptor() throws Exception { @Issue("JENKINS-42971") @Test public void calculate_head_name_with_env() throws Exception { - GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, - new EnvVars("BRANCH", "master-a")); + String remote = "origin"; + GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, null, + new EnvVars("BRANCH", "master-a"), remote); assertEquals("master-a", result1.headName); - assertEquals(Constants.R_HEADS, result1.prefix); + assertTrue(result1.refspec.startsWith("+" + Constants.R_HEADS)); - GitSCMFileSystem.BuilderImpl.HeadNameResult result2 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, - new EnvVars("BRANCH", "refs/heads/master-b")); + GitSCMFileSystem.BuilderImpl.HeadNameResult result2 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, null, + new EnvVars("BRANCH", "refs/heads/master-b"), remote); assertEquals("master-b", result2.headName); - assertEquals(Constants.R_HEADS, result2.prefix); + assertTrue(result2.refspec.startsWith("+" + Constants.R_HEADS)); - GitSCMFileSystem.BuilderImpl.HeadNameResult result3 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("refs/heads/${BRANCH}"), null, - new EnvVars("BRANCH", "master-c")); + GitSCMFileSystem.BuilderImpl.HeadNameResult result3 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("refs/heads/${BRANCH}"), null,null, + new EnvVars("BRANCH", "master-c"), remote); assertEquals("master-c", result3.headName); - assertEquals(Constants.R_HEADS, result3.prefix); + assertTrue(result3.refspec.startsWith("+" + Constants.R_HEADS)); - GitSCMFileSystem.BuilderImpl.HeadNameResult result4 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, - null); + GitSCMFileSystem.BuilderImpl.HeadNameResult result4 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, null, + null, remote); assertEquals("${BRANCH}", result4.headName); - assertEquals(Constants.R_HEADS, result4.prefix); + assertTrue(result4.refspec.startsWith("+" + Constants.R_HEADS)); - GitSCMFileSystem.BuilderImpl.HeadNameResult result5 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("*/${BRANCH}"), null, - new EnvVars("BRANCH", "master-d")); + GitSCMFileSystem.BuilderImpl.HeadNameResult result5 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("*/${BRANCH}"), null, null, + new EnvVars("BRANCH", "master-d"), remote); assertEquals("master-d", result5.headName); - assertEquals(Constants.R_HEADS, result5.prefix); + assertTrue(result5.refspec.startsWith("+" + Constants.R_HEADS)); - GitSCMFileSystem.BuilderImpl.HeadNameResult result6 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("*/master-e"), null, - new EnvVars("BRANCH", "dummy")); + GitSCMFileSystem.BuilderImpl.HeadNameResult result6 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("*/master-e"), null, null, + new EnvVars("BRANCH", "dummy"), remote); assertEquals("master-e", result6.headName); - assertEquals(Constants.R_HEADS, result6.prefix); + assertTrue(result6.refspec.startsWith("+" + Constants.R_HEADS)); + } + + @Test + public void calculate_head_name() throws Exception { + String remote = "origin"; + GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("branch"), null, null, null, remote); + assertEquals("branch", result1.headName); + assertEquals("refs/remotes/origin/branch", result1.remoteHeadName); + assertEquals("+refs/heads/branch:refs/remotes/origin/branch", result1.refspec); + + GitSCMFileSystem.BuilderImpl.HeadNameResult result2 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("refs/heads/branch"), null, null, null, remote); + assertEquals("branch", result2.headName); + assertEquals("refs/remotes/origin/branch", result2.remoteHeadName); + assertEquals("+refs/heads/branch:refs/remotes/origin/branch", result2.refspec); + + GitSCMFileSystem.BuilderImpl.HeadNameResult result3 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("refs/tags/my-tag"), null, null, null, remote); + assertEquals("my-tag", result3.headName); + assertEquals("refs/remotes/origin/my-tag", result3.remoteHeadName); + assertEquals("+refs/tags/my-tag:refs/remotes/origin/my-tag", result3.refspec); + } + + @Test + public void calculate_head_name_with_refspec_commit() throws Exception { + String remote = "origin"; + String commit = "0123456789" + "0123456789" + "0123456789" + "0123456789"; + String branch = "branch"; + GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec(commit), null, branch, null, remote); + assertEquals(commit, result1.headName); + assertEquals(commit, result1.remoteHeadName); + assertEquals(branch, result1.refspec); + + GitSCMFileSystem.BuilderImpl.HeadNameResult result2 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, "${REFSPEC}", + new EnvVars("BRANCH", commit, "REFSPEC", branch), remote); + assertEquals(commit, result2.headName); + assertEquals(commit, result2.remoteHeadName); + assertEquals(branch, result2.refspec); + } + + @Test + public void calculate_head_name_with_refspec_FETCH_HEAD() throws Exception { + String remote = "origin"; + GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("FETCH_HEAD"), null, "refs/changes/1/2/3", null, remote); + assertEquals("FETCH_HEAD", result1.headName); + assertEquals("FETCH_HEAD", result1.remoteHeadName); + assertEquals("refs/changes/1/2/3", result1.refspec); + + GitSCMFileSystem.BuilderImpl.HeadNameResult result2 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, "${REFSPEC}", + new EnvVars("BRANCH", "FETCH_HEAD", "REFSPEC", "refs/changes/1/2/3"), remote); + assertEquals("FETCH_HEAD", result2.headName); + assertEquals("FETCH_HEAD", result2.remoteHeadName); + assertEquals("refs/changes/1/2/3", result2.refspec); } /* GitSCMFileSystem in git plugin 4.14.0 reported a null pointer @@ -472,9 +572,9 @@ public void null_pointer_exception() throws Exception { ObjectId git260 = client.revParse(GIT_2_6_0_TAG); AbstractGitSCMSource.SCMRevisionImpl rev260 = new AbstractGitSCMSource.SCMRevisionImpl(new SCMHead("origin"), git260.getName()); - GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("master-f"), rev260, null); + GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("master-f"), rev260, null, null, "origin"); assertEquals("master-f", result1.headName); - assertEquals(Constants.R_HEADS, result1.prefix); + assertTrue(result1.refspec.startsWith("+" + Constants.R_HEADS)); } /** inline ${@link hudson.Functions#isWindows()} to prevent a transient remote classloader issue */ From 1fde396e06a0e5c6e4c1f0eef3ff43f3e01275a6 Mon Sep 17 00:00:00 2001 From: kuc2hi Date: Wed, 21 Jun 2023 13:03:55 +0200 Subject: [PATCH 03/17] [Jenkins-71506] Remove spotbugs finding --- src/main/java/jenkins/plugins/git/GitSCMFileSystem.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index da1ae2e52c..f648fe8c1f 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -355,7 +355,7 @@ static HeadNameResult calculate(@NonNull BranchSpec branchSpec, } String remoteHead = headName; - if (prefix == Constants.R_HEADS || prefix == Constants.R_TAGS) { + if (prefix.equals(Constants.R_HEADS) || prefix.equals(Constants.R_TAGS)) { remoteHead = Constants.R_REMOTES + remoteName + "/" + headName; } From efed3af4b281cc3c31c7d23cdeb7d3d8daf15e8f Mon Sep 17 00:00:00 2001 From: kuc2hi Date: Wed, 21 Jun 2023 15:00:25 +0200 Subject: [PATCH 04/17] Fix NPE --- src/main/java/jenkins/plugins/git/GitSCMFileSystem.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index f648fe8c1f..df67d30671 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -355,7 +355,7 @@ static HeadNameResult calculate(@NonNull BranchSpec branchSpec, } String remoteHead = headName; - if (prefix.equals(Constants.R_HEADS) || prefix.equals(Constants.R_TAGS)) { + if (prefix != null && (prefix.equals(Constants.R_HEADS) || prefix.equals(Constants.R_TAGS))) { remoteHead = Constants.R_REMOTES + remoteName + "/" + headName; } From c64fd027816d9fd27e7522f86c628c5d660c2380 Mon Sep 17 00:00:00 2001 From: kuc2hi Date: Fri, 23 Jun 2023 16:15:28 +0200 Subject: [PATCH 05/17] Make use of Constants.FETCH_HEAD --- .../java/jenkins/plugins/git/GitSCMFileSystem.java | 2 +- .../jenkins/plugins/git/GitSCMFileSystemTest.java | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index df67d30671..35ea53e4f5 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -323,7 +323,7 @@ static HeadNameResult calculate(@NonNull BranchSpec branchSpec, prefix = Constants.R_TAGS; } else { // check for FETCH_HEAD - if (branchSpecExpandedName.equals("FETCH_HEAD") && !refspecExpandedName.equals("")) { + if (branchSpecExpandedName.equals(Constants.FETCH_HEAD) && !refspecExpandedName.equals("")) { prefix = null; } else { // check for commit-id diff --git a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java index c6eb4fa171..13a049bec5 100644 --- a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java +++ b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java @@ -465,7 +465,7 @@ public void create_SCMFileSystem_from_FETCH_HEAD() throws Exception { sampleRepo.git("mv", "file", "dir/subdir/file"); sampleRepo.write("dir/subdir/file", "modified"); sampleRepo.git("commit", "--all", "--message=dev"); - SCMFileSystem fs = SCMFileSystem.of(r.createFreeStyleProject(), new GitSCM(createRepoListWithRefspec(sampleRepo.toString(), "dev"), Collections.singletonList(new BranchSpec("FETCH_HEAD")), null, null, Collections.emptyList())); + SCMFileSystem fs = SCMFileSystem.of(r.createFreeStyleProject(), new GitSCM(createRepoListWithRefspec(sampleRepo.toString(), "dev"), Collections.singletonList(new BranchSpec(Constants.FETCH_HEAD)), null, null, Collections.emptyList())); assertEquals("modified", getFileContent(fs, "dir/subdir/file", "modified")); } @@ -550,15 +550,15 @@ public void calculate_head_name_with_refspec_commit() throws Exception { @Test public void calculate_head_name_with_refspec_FETCH_HEAD() throws Exception { String remote = "origin"; - GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("FETCH_HEAD"), null, "refs/changes/1/2/3", null, remote); - assertEquals("FETCH_HEAD", result1.headName); - assertEquals("FETCH_HEAD", result1.remoteHeadName); + GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec(Constants.FETCH_HEAD), null, "refs/changes/1/2/3", null, remote); + assertEquals(Constants.FETCH_HEAD, result1.headName); + assertEquals(Constants.FETCH_HEAD, result1.remoteHeadName); assertEquals("refs/changes/1/2/3", result1.refspec); GitSCMFileSystem.BuilderImpl.HeadNameResult result2 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, "${REFSPEC}", - new EnvVars("BRANCH", "FETCH_HEAD", "REFSPEC", "refs/changes/1/2/3"), remote); - assertEquals("FETCH_HEAD", result2.headName); - assertEquals("FETCH_HEAD", result2.remoteHeadName); + new EnvVars("BRANCH", Constants.FETCH_HEAD, "REFSPEC", "refs/changes/1/2/3"), remote); + assertEquals(Constants.FETCH_HEAD, result2.headName); + assertEquals(Constants.FETCH_HEAD, result2.remoteHeadName); assertEquals("refs/changes/1/2/3", result2.refspec); } From 080cfb8796d4693948f401564f155e8c4c97851a Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 1 Jul 2023 11:39:06 -0600 Subject: [PATCH 06/17] Accept 64 character SHA-256 as well --- src/main/java/jenkins/plugins/git/GitSCMFileSystem.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index 35ea53e4f5..5a854b5628 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -327,7 +327,7 @@ static HeadNameResult calculate(@NonNull BranchSpec branchSpec, prefix = null; } else { // check for commit-id - final String regex = "^[a-fA-F0-9]{40}$"; + final String regex = "^([a-fA-F0-9]{40}|[a-fA-F0-9]{64})$"; final Pattern pattern = Pattern.compile(regex, Pattern.MULTILINE); final Matcher matcher = pattern.matcher(branchSpecExpandedName); From fd64d806eb9a012d930852b28e9922c4eb128f68 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 1 Jul 2023 12:06:59 -0600 Subject: [PATCH 07/17] Reduce diff for easier code review --- .../plugins/git/GitSCMFileSystemTest.java | 57 +++++++++---------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java index 13a049bec5..011f42e583 100644 --- a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java +++ b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java @@ -404,43 +404,38 @@ public void create_SCMFileSystem_from_tag() throws Exception { assertEquals("modified", getFileContent(fs, "dir/subdir/file", "modified")); } - public static List createRepoListWithRefspec(String url, String refspec) { - List repoList = new ArrayList<>(); - repoList.add(new UserRemoteConfig(url, null, refspec, null)); - return repoList; - } - public String getFileContent(SCMFileSystem fs, String path, String expectedContent) throws IOException, InterruptedException { assertThat(fs, notNullValue()); assertThat(fs.getRoot(), notNullValue()); - String[] segments = path.split("/"); - - SCMFile parent = fs.getRoot(); - SCMFile file = null; - for (String segment: segments) { - Iterable children = parent.children(); - Iterator iterator = children.iterator(); - if (segment == segments[segments.length - 1]) { - // file - assertThat(iterator.hasNext(), is(true)); - SCMFile t = iterator.next(); - assertThat(iterator.hasNext(), is(false)); - assertThat(t.getName(), is(segment)); - assertThat(t.getType(), is(SCMFile.Type.REGULAR_FILE)); - file = t; - } else { - // dir - assertThat(iterator.hasNext(), is(true)); - SCMFile dir = iterator.next(); - assertThat(iterator.hasNext(), is(false)); - assertThat(dir.getName(), is(segment)); - assertThat(dir.getType(), is(SCMFile.Type.DIRECTORY)); - parent = dir; - } - } + Iterable children = fs.getRoot().children(); + Iterator iterator = children.iterator(); + assertThat(iterator.hasNext(), is(true)); + SCMFile dir = iterator.next(); + assertThat(iterator.hasNext(), is(false)); + assertThat(dir.getName(), is("dir")); + assertThat(dir.getType(), is(SCMFile.Type.DIRECTORY)); + children = dir.children(); + iterator = children.iterator(); + assertThat(iterator.hasNext(), is(true)); + SCMFile subdir = iterator.next(); + assertThat(iterator.hasNext(), is(false)); + assertThat(subdir.getName(), is("subdir")); + assertThat(subdir.getType(), is(SCMFile.Type.DIRECTORY)); + children = subdir.children(); + iterator = children.iterator(); + assertThat(iterator.hasNext(), is(true)); + SCMFile file = iterator.next(); + assertThat(iterator.hasNext(), is(false)); + assertThat(file.getName(), is("file")); return file.contentAsString(); } + public static List createRepoListWithRefspec(String url, String refspec) { + List repoList = new ArrayList<>(); + repoList.add(new UserRemoteConfig(url, null, refspec, null)); + return repoList; + } + @Test public void create_SCMFileSystem_from_commit() throws Exception { sampleRepo.init(); From e6e957f82a92990c678c427a1749d1bb64979662 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 1 Jul 2023 12:24:22 -0600 Subject: [PATCH 08/17] Revert "Accept 64 character SHA-256 as well" No need to apply that technique in one location when there are many others that have the same issue. This reverts commit 080cfb8796d4693948f401564f155e8c4c97851a. --- src/main/java/jenkins/plugins/git/GitSCMFileSystem.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index 5a854b5628..35ea53e4f5 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -327,7 +327,7 @@ static HeadNameResult calculate(@NonNull BranchSpec branchSpec, prefix = null; } else { // check for commit-id - final String regex = "^([a-fA-F0-9]{40}|[a-fA-F0-9]{64})$"; + final String regex = "^[a-fA-F0-9]{40}$"; final Pattern pattern = Pattern.compile(regex, Pattern.MULTILINE); final Matcher matcher = pattern.matcher(branchSpecExpandedName); From fb0afc90685a59cd7ad3c2cf2c62d26aceb3dfd0 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 1 Jul 2023 16:07:01 -0600 Subject: [PATCH 09/17] Use an import to simplify reading the test --- .../plugins/git/GitSCMFileSystemTest.java | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java index 011f42e583..6873c90ee6 100644 --- a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java +++ b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java @@ -72,6 +72,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertTrue; +import jenkins.plugins.git.GitSCMFileSystem.BuilderImpl.HeadNameResult; + /** * Tests for {@link AbstractGitSCMSource} */ @@ -475,33 +477,27 @@ public void filesystem_supports_descriptor() throws Exception { @Test public void calculate_head_name_with_env() throws Exception { String remote = "origin"; - GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, null, - new EnvVars("BRANCH", "master-a"), remote); + HeadNameResult result1 = HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, null, new EnvVars("BRANCH", "master-a"), remote); assertEquals("master-a", result1.headName); assertTrue(result1.refspec.startsWith("+" + Constants.R_HEADS)); - GitSCMFileSystem.BuilderImpl.HeadNameResult result2 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, null, - new EnvVars("BRANCH", "refs/heads/master-b"), remote); + HeadNameResult result2 = HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, null, new EnvVars("BRANCH", "refs/heads/master-b"), remote); assertEquals("master-b", result2.headName); assertTrue(result2.refspec.startsWith("+" + Constants.R_HEADS)); - GitSCMFileSystem.BuilderImpl.HeadNameResult result3 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("refs/heads/${BRANCH}"), null,null, - new EnvVars("BRANCH", "master-c"), remote); + HeadNameResult result3 = HeadNameResult.calculate(new BranchSpec("refs/heads/${BRANCH}"), null, null, new EnvVars("BRANCH", "master-c"), remote); assertEquals("master-c", result3.headName); assertTrue(result3.refspec.startsWith("+" + Constants.R_HEADS)); - GitSCMFileSystem.BuilderImpl.HeadNameResult result4 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, null, - null, remote); + HeadNameResult result4 = HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, null, null, remote); assertEquals("${BRANCH}", result4.headName); assertTrue(result4.refspec.startsWith("+" + Constants.R_HEADS)); - GitSCMFileSystem.BuilderImpl.HeadNameResult result5 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("*/${BRANCH}"), null, null, - new EnvVars("BRANCH", "master-d"), remote); + HeadNameResult result5 = HeadNameResult.calculate(new BranchSpec("*/${BRANCH}"), null, null, new EnvVars("BRANCH", "master-d"), remote); assertEquals("master-d", result5.headName); assertTrue(result5.refspec.startsWith("+" + Constants.R_HEADS)); - GitSCMFileSystem.BuilderImpl.HeadNameResult result6 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("*/master-e"), null, null, - new EnvVars("BRANCH", "dummy"), remote); + HeadNameResult result6 = HeadNameResult.calculate(new BranchSpec("*/master-e"), null, null, new EnvVars("BRANCH", "dummy"), remote); assertEquals("master-e", result6.headName); assertTrue(result6.refspec.startsWith("+" + Constants.R_HEADS)); } From d21149d0380c70d7a2f3ae22895066fe9b49ff3f Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 1 Jul 2023 16:09:18 -0600 Subject: [PATCH 10/17] Suppress a spotbugs warning as technical debt Not clear to me how to resolve the spotbugs warning --- src/spotbugs/excludesFilter.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/spotbugs/excludesFilter.xml b/src/spotbugs/excludesFilter.xml index bf1e7ef977..27bf5a1b76 100644 --- a/src/spotbugs/excludesFilter.xml +++ b/src/spotbugs/excludesFilter.xml @@ -74,4 +74,9 @@ + + + + + From 343d3f10d224a3dd6bbe580e47cb199e8420e743 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 1 Jul 2023 16:16:58 -0600 Subject: [PATCH 11/17] Rename local headName as calculatedHeadName for clarity Using the same string for the local as is used in the HeadNameResult class was confusing me. Rather than risk being confused when I read it again later, let's rename it now. --- .../jenkins/plugins/git/GitSCMFileSystem.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index 35ea53e4f5..7a48e251b0 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -339,27 +339,27 @@ static HeadNameResult calculate(@NonNull BranchSpec branchSpec, } } - String headName = branchSpecExpandedName; + String calculatedHeadName = branchSpecExpandedName; if (rev != null && env != null) { - headName = env.expand(rev.getHead().getName()); + calculatedHeadName = env.expand(rev.getHead().getName()); } else { if (prefix != null && branchSpecExpandedName.startsWith(prefix)) { - headName = branchSpecExpandedName.substring(prefix.length()); + calculatedHeadName = branchSpecExpandedName.substring(prefix.length()); } else if (branchSpecExpandedName.startsWith("*/")) { - headName = branchSpecExpandedName.substring(2); + calculatedHeadName = branchSpecExpandedName.substring(2); } } if (refspecExpandedName == null || refspecExpandedName.equals("")) { - refspecExpandedName = "+" + prefix + headName + ":" + Constants.R_REMOTES + remoteName + "/" + headName; + refspecExpandedName = "+" + prefix + calculatedHeadName + ":" + Constants.R_REMOTES + remoteName + "/" + calculatedHeadName; } - String remoteHead = headName; + String remoteHead = calculatedHeadName; if (prefix != null && (prefix.equals(Constants.R_HEADS) || prefix.equals(Constants.R_TAGS))) { - remoteHead = Constants.R_REMOTES + remoteName + "/" + headName; + remoteHead = Constants.R_REMOTES + remoteName + "/" + calculatedHeadName; } - return new HeadNameResult(headName, remoteHead, refspecExpandedName, rev); + return new HeadNameResult(calculatedHeadName, remoteHead, refspecExpandedName, rev); } } From 244ba1f8ffb77016b55a8a38c873e737a080ecc6 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 1 Jul 2023 16:18:56 -0600 Subject: [PATCH 12/17] Report value of headNameResult.headName Silences a spotbugs warning and helps my testing --- src/main/java/jenkins/plugins/git/GitSCMFileSystem.java | 2 +- src/spotbugs/excludesFilter.xml | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index 7a48e251b0..9f515505af 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -442,7 +442,7 @@ public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull HeadNameResult headNameResult = HeadNameResult.calculate(branchSpec, rev, config.getRefspec(), env, remoteName); client.fetch_().prune(true).from(remoteURI, Collections.singletonList(new RefSpec(headNameResult.refspec))).execute(); - listener.getLogger().println("Done."); + listener.getLogger().println("Done with " + remoteName + " using " + headNameResult.headName + "."); return new GitSCMFileSystem(client, remote, headNameResult.remoteHeadName, (AbstractGitSCMSource.SCMRevisionImpl) headNameResult.rev); } finally { cacheLock.unlock(); diff --git a/src/spotbugs/excludesFilter.xml b/src/spotbugs/excludesFilter.xml index 27bf5a1b76..bf1e7ef977 100644 --- a/src/spotbugs/excludesFilter.xml +++ b/src/spotbugs/excludesFilter.xml @@ -74,9 +74,4 @@ - - - - - From 1b881a68c5be62a22f1f45624f163a2e6b0e8f6f Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 1 Jul 2023 16:21:39 -0600 Subject: [PATCH 13/17] Declare refSpec can be null in calculate The tests are passing null and have encountered no error with the null argument --- src/main/java/jenkins/plugins/git/GitSCMFileSystem.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index 9f515505af..85a068c774 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -303,7 +303,7 @@ private HeadNameResult(String headName, String remoteHeadName, String refspec, S static HeadNameResult calculate(@NonNull BranchSpec branchSpec, @CheckForNull SCMRevision rev, - @NonNull String refSpec, + @CheckForNull String refSpec, @CheckForNull EnvVars env, @CheckForNull String remoteName) { From e66e798ab17556f5fafc2b85bd9ce2994a679e3a Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sun, 2 Jul 2023 07:02:53 -0600 Subject: [PATCH 14/17] Simplify GitSCMFileSystemTest with import --- .../plugins/git/GitSCMFileSystemTest.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java index 6873c90ee6..ec7a3ba24c 100644 --- a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java +++ b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java @@ -505,17 +505,17 @@ public void calculate_head_name_with_env() throws Exception { @Test public void calculate_head_name() throws Exception { String remote = "origin"; - GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("branch"), null, null, null, remote); + HeadNameResult result1 = HeadNameResult.calculate(new BranchSpec("branch"), null, null, null, remote); assertEquals("branch", result1.headName); assertEquals("refs/remotes/origin/branch", result1.remoteHeadName); assertEquals("+refs/heads/branch:refs/remotes/origin/branch", result1.refspec); - GitSCMFileSystem.BuilderImpl.HeadNameResult result2 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("refs/heads/branch"), null, null, null, remote); + HeadNameResult result2 = HeadNameResult.calculate(new BranchSpec("refs/heads/branch"), null, null, null, remote); assertEquals("branch", result2.headName); assertEquals("refs/remotes/origin/branch", result2.remoteHeadName); assertEquals("+refs/heads/branch:refs/remotes/origin/branch", result2.refspec); - GitSCMFileSystem.BuilderImpl.HeadNameResult result3 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("refs/tags/my-tag"), null, null, null, remote); + HeadNameResult result3 = HeadNameResult.calculate(new BranchSpec("refs/tags/my-tag"), null, null, null, remote); assertEquals("my-tag", result3.headName); assertEquals("refs/remotes/origin/my-tag", result3.remoteHeadName); assertEquals("+refs/tags/my-tag:refs/remotes/origin/my-tag", result3.refspec); @@ -526,12 +526,12 @@ public void calculate_head_name_with_refspec_commit() throws Exception { String remote = "origin"; String commit = "0123456789" + "0123456789" + "0123456789" + "0123456789"; String branch = "branch"; - GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec(commit), null, branch, null, remote); + HeadNameResult result1 = HeadNameResult.calculate(new BranchSpec(commit), null, branch, null, remote); assertEquals(commit, result1.headName); assertEquals(commit, result1.remoteHeadName); assertEquals(branch, result1.refspec); - GitSCMFileSystem.BuilderImpl.HeadNameResult result2 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, "${REFSPEC}", + HeadNameResult result2 = HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, "${REFSPEC}", new EnvVars("BRANCH", commit, "REFSPEC", branch), remote); assertEquals(commit, result2.headName); assertEquals(commit, result2.remoteHeadName); @@ -541,12 +541,12 @@ public void calculate_head_name_with_refspec_commit() throws Exception { @Test public void calculate_head_name_with_refspec_FETCH_HEAD() throws Exception { String remote = "origin"; - GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec(Constants.FETCH_HEAD), null, "refs/changes/1/2/3", null, remote); + HeadNameResult result1 = HeadNameResult.calculate(new BranchSpec(Constants.FETCH_HEAD), null, "refs/changes/1/2/3", null, remote); assertEquals(Constants.FETCH_HEAD, result1.headName); assertEquals(Constants.FETCH_HEAD, result1.remoteHeadName); assertEquals("refs/changes/1/2/3", result1.refspec); - GitSCMFileSystem.BuilderImpl.HeadNameResult result2 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, "${REFSPEC}", + HeadNameResult result2 = HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, "${REFSPEC}", new EnvVars("BRANCH", Constants.FETCH_HEAD, "REFSPEC", "refs/changes/1/2/3"), remote); assertEquals(Constants.FETCH_HEAD, result2.headName); assertEquals(Constants.FETCH_HEAD, result2.remoteHeadName); @@ -563,7 +563,7 @@ public void null_pointer_exception() throws Exception { ObjectId git260 = client.revParse(GIT_2_6_0_TAG); AbstractGitSCMSource.SCMRevisionImpl rev260 = new AbstractGitSCMSource.SCMRevisionImpl(new SCMHead("origin"), git260.getName()); - GitSCMFileSystem.BuilderImpl.HeadNameResult result1 = GitSCMFileSystem.BuilderImpl.HeadNameResult.calculate(new BranchSpec("master-f"), rev260, null, null, "origin"); + HeadNameResult result1 = HeadNameResult.calculate(new BranchSpec("master-f"), rev260, null, null, "origin"); assertEquals("master-f", result1.headName); assertTrue(result1.refspec.startsWith("+" + Constants.R_HEADS)); } From c40b7475a8c872d8c01a19f0eaba5af4b52adaef Mon Sep 17 00:00:00 2001 From: kuc2hi Date: Fri, 7 Jul 2023 16:40:04 +0200 Subject: [PATCH 15/17] Fix refspec calculation for tags --- .../java/jenkins/plugins/git/GitSCMFileSystem.java | 13 +++++++++---- .../jenkins/plugins/git/GitSCMFileSystemTest.java | 4 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index 85a068c774..3d19d2c54c 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -312,7 +312,7 @@ static HeadNameResult calculate(@NonNull BranchSpec branchSpec, branchSpecExpandedName = env.expand(branchSpecExpandedName); } String refspecExpandedName = refSpec; - if (env != null) { + if (env != null && refspecExpandedName != null) { refspecExpandedName = env.expand(refspecExpandedName); } @@ -323,7 +323,8 @@ static HeadNameResult calculate(@NonNull BranchSpec branchSpec, prefix = Constants.R_TAGS; } else { // check for FETCH_HEAD - if (branchSpecExpandedName.equals(Constants.FETCH_HEAD) && !refspecExpandedName.equals("")) { + if (branchSpecExpandedName.equals(Constants.FETCH_HEAD) && refspecExpandedName != null && + !refspecExpandedName.equals("")) { prefix = null; } else { // check for commit-id @@ -351,11 +352,15 @@ static HeadNameResult calculate(@NonNull BranchSpec branchSpec, } if (refspecExpandedName == null || refspecExpandedName.equals("")) { - refspecExpandedName = "+" + prefix + calculatedHeadName + ":" + Constants.R_REMOTES + remoteName + "/" + calculatedHeadName; + if (prefix.equals(Constants.R_TAGS)) { + refspecExpandedName = "+" + prefix + calculatedHeadName + ":" + prefix + calculatedHeadName; + } else { + refspecExpandedName = "+" + prefix + calculatedHeadName + ":" + Constants.R_REMOTES + remoteName + "/" + calculatedHeadName; + } } String remoteHead = calculatedHeadName; - if (prefix != null && (prefix.equals(Constants.R_HEADS) || prefix.equals(Constants.R_TAGS))) { + if (prefix != null && prefix.equals(Constants.R_HEADS)) { remoteHead = Constants.R_REMOTES + remoteName + "/" + calculatedHeadName; } diff --git a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java index ec7a3ba24c..b73afd4c51 100644 --- a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java +++ b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java @@ -517,8 +517,8 @@ public void calculate_head_name() throws Exception { HeadNameResult result3 = HeadNameResult.calculate(new BranchSpec("refs/tags/my-tag"), null, null, null, remote); assertEquals("my-tag", result3.headName); - assertEquals("refs/remotes/origin/my-tag", result3.remoteHeadName); - assertEquals("+refs/tags/my-tag:refs/remotes/origin/my-tag", result3.refspec); + assertEquals("my-tag", result3.remoteHeadName); + assertEquals("+refs/tags/my-tag:refs/tags/my-tag", result3.refspec); } @Test From 8878bbbbdc88b5b045121c319a0c2dc8f74fb25c Mon Sep 17 00:00:00 2001 From: kuc2hi Date: Fri, 7 Jul 2023 16:42:54 +0200 Subject: [PATCH 16/17] Remove headName field in HeadNameResult The field was only needed to calculte remoteHeadName, which is now done inside the calculate() method. --- .../jenkins/plugins/git/GitSCMFileSystem.java | 8 +++---- .../plugins/git/GitSCMFileSystemTest.java | 21 +++++++------------ 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index 3d19d2c54c..bebd7bd6f2 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -289,13 +289,11 @@ public boolean supportsDescriptor(SCMSourceDescriptor descriptor) { } static class HeadNameResult { - final String headName; final String remoteHeadName; final String refspec; final SCMRevision rev; - private HeadNameResult(String headName, String remoteHeadName, String refspec, SCMRevision rev) { - this.headName = headName; + private HeadNameResult(String remoteHeadName, String refspec, SCMRevision rev) { this.remoteHeadName = remoteHeadName; this.refspec = refspec; this.rev = rev; @@ -364,7 +362,7 @@ static HeadNameResult calculate(@NonNull BranchSpec branchSpec, remoteHead = Constants.R_REMOTES + remoteName + "/" + calculatedHeadName; } - return new HeadNameResult(calculatedHeadName, remoteHead, refspecExpandedName, rev); + return new HeadNameResult(remoteHead, refspecExpandedName, rev); } } @@ -447,7 +445,7 @@ public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull HeadNameResult headNameResult = HeadNameResult.calculate(branchSpec, rev, config.getRefspec(), env, remoteName); client.fetch_().prune(true).from(remoteURI, Collections.singletonList(new RefSpec(headNameResult.refspec))).execute(); - listener.getLogger().println("Done with " + remoteName + " using " + headNameResult.headName + "."); + listener.getLogger().println("Done with " + remoteName + " using " + headNameResult.remoteHeadName + "."); return new GitSCMFileSystem(client, remote, headNameResult.remoteHeadName, (AbstractGitSCMSource.SCMRevisionImpl) headNameResult.rev); } finally { cacheLock.unlock(); diff --git a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java index b73afd4c51..5b7acaccdb 100644 --- a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java +++ b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java @@ -478,27 +478,27 @@ public void filesystem_supports_descriptor() throws Exception { public void calculate_head_name_with_env() throws Exception { String remote = "origin"; HeadNameResult result1 = HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, null, new EnvVars("BRANCH", "master-a"), remote); - assertEquals("master-a", result1.headName); + assertEquals("refs/remotes/origin/master-a", result1.remoteHeadName); assertTrue(result1.refspec.startsWith("+" + Constants.R_HEADS)); HeadNameResult result2 = HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, null, new EnvVars("BRANCH", "refs/heads/master-b"), remote); - assertEquals("master-b", result2.headName); + assertEquals("refs/remotes/origin/master-b", result2.remoteHeadName); assertTrue(result2.refspec.startsWith("+" + Constants.R_HEADS)); HeadNameResult result3 = HeadNameResult.calculate(new BranchSpec("refs/heads/${BRANCH}"), null, null, new EnvVars("BRANCH", "master-c"), remote); - assertEquals("master-c", result3.headName); + assertEquals("refs/remotes/origin/master-c", result3.remoteHeadName); assertTrue(result3.refspec.startsWith("+" + Constants.R_HEADS)); HeadNameResult result4 = HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, null, null, remote); - assertEquals("${BRANCH}", result4.headName); + assertEquals("refs/remotes/origin/${BRANCH}", result4.remoteHeadName); assertTrue(result4.refspec.startsWith("+" + Constants.R_HEADS)); HeadNameResult result5 = HeadNameResult.calculate(new BranchSpec("*/${BRANCH}"), null, null, new EnvVars("BRANCH", "master-d"), remote); - assertEquals("master-d", result5.headName); + assertEquals("refs/remotes/origin/master-d", result5.remoteHeadName); assertTrue(result5.refspec.startsWith("+" + Constants.R_HEADS)); HeadNameResult result6 = HeadNameResult.calculate(new BranchSpec("*/master-e"), null, null, new EnvVars("BRANCH", "dummy"), remote); - assertEquals("master-e", result6.headName); + assertEquals("refs/remotes/origin/master-e", result6.remoteHeadName); assertTrue(result6.refspec.startsWith("+" + Constants.R_HEADS)); } @@ -506,17 +506,14 @@ public void calculate_head_name_with_env() throws Exception { public void calculate_head_name() throws Exception { String remote = "origin"; HeadNameResult result1 = HeadNameResult.calculate(new BranchSpec("branch"), null, null, null, remote); - assertEquals("branch", result1.headName); assertEquals("refs/remotes/origin/branch", result1.remoteHeadName); assertEquals("+refs/heads/branch:refs/remotes/origin/branch", result1.refspec); HeadNameResult result2 = HeadNameResult.calculate(new BranchSpec("refs/heads/branch"), null, null, null, remote); - assertEquals("branch", result2.headName); assertEquals("refs/remotes/origin/branch", result2.remoteHeadName); assertEquals("+refs/heads/branch:refs/remotes/origin/branch", result2.refspec); HeadNameResult result3 = HeadNameResult.calculate(new BranchSpec("refs/tags/my-tag"), null, null, null, remote); - assertEquals("my-tag", result3.headName); assertEquals("my-tag", result3.remoteHeadName); assertEquals("+refs/tags/my-tag:refs/tags/my-tag", result3.refspec); } @@ -527,13 +524,11 @@ public void calculate_head_name_with_refspec_commit() throws Exception { String commit = "0123456789" + "0123456789" + "0123456789" + "0123456789"; String branch = "branch"; HeadNameResult result1 = HeadNameResult.calculate(new BranchSpec(commit), null, branch, null, remote); - assertEquals(commit, result1.headName); assertEquals(commit, result1.remoteHeadName); assertEquals(branch, result1.refspec); HeadNameResult result2 = HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, "${REFSPEC}", new EnvVars("BRANCH", commit, "REFSPEC", branch), remote); - assertEquals(commit, result2.headName); assertEquals(commit, result2.remoteHeadName); assertEquals(branch, result2.refspec); } @@ -542,13 +537,11 @@ public void calculate_head_name_with_refspec_commit() throws Exception { public void calculate_head_name_with_refspec_FETCH_HEAD() throws Exception { String remote = "origin"; HeadNameResult result1 = HeadNameResult.calculate(new BranchSpec(Constants.FETCH_HEAD), null, "refs/changes/1/2/3", null, remote); - assertEquals(Constants.FETCH_HEAD, result1.headName); assertEquals(Constants.FETCH_HEAD, result1.remoteHeadName); assertEquals("refs/changes/1/2/3", result1.refspec); HeadNameResult result2 = HeadNameResult.calculate(new BranchSpec("${BRANCH}"), null, "${REFSPEC}", new EnvVars("BRANCH", Constants.FETCH_HEAD, "REFSPEC", "refs/changes/1/2/3"), remote); - assertEquals(Constants.FETCH_HEAD, result2.headName); assertEquals(Constants.FETCH_HEAD, result2.remoteHeadName); assertEquals("refs/changes/1/2/3", result2.refspec); } @@ -564,7 +557,7 @@ public void null_pointer_exception() throws Exception { AbstractGitSCMSource.SCMRevisionImpl rev260 = new AbstractGitSCMSource.SCMRevisionImpl(new SCMHead("origin"), git260.getName()); HeadNameResult result1 = HeadNameResult.calculate(new BranchSpec("master-f"), rev260, null, null, "origin"); - assertEquals("master-f", result1.headName); + assertEquals("refs/remotes/origin/master-f", result1.remoteHeadName); assertTrue(result1.refspec.startsWith("+" + Constants.R_HEADS)); } From 66acacc4a3cead019ab3a3c7f0fc15e544af82a4 Mon Sep 17 00:00:00 2001 From: Victor Balakine Date: Wed, 14 Feb 2024 11:45:35 +0200 Subject: [PATCH 17/17] Some light refactoring --- .../jenkins/plugins/git/GitSCMFileSystem.java | 43 ++++++------------- 1 file changed, 14 insertions(+), 29 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index 0c2ed3b46c..c8f588c831 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -60,9 +60,6 @@ import java.util.concurrent.locks.Lock; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - import jenkins.scm.api.SCMFile; import jenkins.scm.api.SCMFileSystem; import jenkins.scm.api.SCMHead; @@ -310,7 +307,7 @@ static HeadNameResult calculate(@NonNull BranchSpec branchSpec, branchSpecExpandedName = env.expand(branchSpecExpandedName); } String refspecExpandedName = refSpec; - if (env != null && refspecExpandedName != null) { + if (env != null) { refspecExpandedName = env.expand(refspecExpandedName); } @@ -319,39 +316,27 @@ static HeadNameResult calculate(@NonNull BranchSpec branchSpec, // check for a tag if (branchSpecExpandedName.startsWith(Constants.R_TAGS)) { prefix = Constants.R_TAGS; - } else { + } else if (branchSpecExpandedName.equals(Constants.FETCH_HEAD) && StringUtils.isNotBlank(refspecExpandedName)) { // check for FETCH_HEAD - if (branchSpecExpandedName.equals(Constants.FETCH_HEAD) && refspecExpandedName != null && - !refspecExpandedName.equals("")) { - prefix = null; - } else { - // check for commit-id - final String regex = "^[a-fA-F0-9]{40}$"; - final Pattern pattern = Pattern.compile(regex, Pattern.MULTILINE); - final Matcher matcher = pattern.matcher(branchSpecExpandedName); - - if (matcher.find()) { - // commit-id - prefix = null; - rev = new AbstractGitSCMSource.SCMRevisionImpl(new SCMHead(branchSpecExpandedName), branchSpecExpandedName); - } - } + prefix = null; + } else if (branchSpecExpandedName.matches("[0-9a-f]{6,40}")) { + // commit-id + prefix = null; + rev = new AbstractGitSCMSource.SCMRevisionImpl(new SCMHead(branchSpecExpandedName), branchSpecExpandedName); } String calculatedHeadName = branchSpecExpandedName; if (rev != null && env != null) { calculatedHeadName = env.expand(rev.getHead().getName()); - } else { - if (prefix != null && branchSpecExpandedName.startsWith(prefix)) { - calculatedHeadName = branchSpecExpandedName.substring(prefix.length()); - } else if (branchSpecExpandedName.startsWith("*/")) { - calculatedHeadName = branchSpecExpandedName.substring(2); - } + } else if (prefix != null && branchSpecExpandedName.startsWith(prefix)) { + calculatedHeadName = branchSpecExpandedName.substring(prefix.length()); + } else if (branchSpecExpandedName.startsWith("*/")) { + calculatedHeadName = branchSpecExpandedName.substring(2); } - if (refspecExpandedName == null || refspecExpandedName.equals("")) { - if (prefix.equals(Constants.R_TAGS)) { - refspecExpandedName = "+" + prefix + calculatedHeadName + ":" + prefix + calculatedHeadName; + if (StringUtils.isBlank(refspecExpandedName)) { + if (prefix != null && prefix.equals(Constants.R_TAGS)) { + refspecExpandedName = "+" + prefix + calculatedHeadName + ":" + prefix + calculatedHeadName; } else { refspecExpandedName = "+" + prefix + calculatedHeadName + ":" + Constants.R_REMOTES + remoteName + "/" + calculatedHeadName; }