Skip to content

[JENKINS-48431] - Support both lightweight checkout AND build parameter #898

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jetersen wants to merge 14 commits into from
Closed

[JENKINS-48431] - Support both lightweight checkout AND build parameter #898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
5bd9173
JENKINS-48431 Support both lightweight checkout AND build parameters
Oct 14, 2019
bf791e2
Bugfix for tests
Oct 14, 2019
96f458d
Trim whitespaces
Oct 19, 2019
cb53320
Bugfix spotbugs
Nov 28, 2019
efe53fd
Merge branch 'master' into JENKINS-48431
MarkEWaite Mar 10, 2020
4e4d5fd
Merge branch 'master' into JENKINS-48431
MarkEWaite Apr 5, 2020
2243149
Merge branch 'master' into JENKINS-48431
jetersen May 25, 2020
5825807
deduplicate and implement both GitSCMTelescope and GitSCMFileSystem
jetersen May 25, 2020
274e66b
remove null check for build
jetersen May 25, 2020
3cc1567
apply suggestions from @tgatinea
jetersen Jun 6, 2020
c424a69
remote can potentially be null
jetersen Jun 7, 2020
49dee32
Merge branch 'master' into JENKINS-48431
MarkEWaite Dec 19, 2020
4efe4e4
Merge branch 'master' into JENKINS-48431
MarkEWaite Apr 10, 2021
a1ebe5e
Merge branch 'master' into JENKINS-48431
MarkEWaite Oct 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>scm-api</artifactId>
<version>${scm-api-plugin.version}</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/hudson/plugins/git/util/GitUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,10 @@ public static String[] fixupNames(String[] names, String[] urls) {
return returnNames;
}

public static String expand(String str, EnvVars env) {
return env == null ? str : env.expand(str);
}

private static final Logger LOGGER = Logger.getLogger(GitUtils.class.getName());

private static final long serialVersionUID = 1L;
Expand Down
49 changes: 38 additions & 11 deletions src/main/java/jenkins/plugins/git/GitSCMFileSystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.EnvVars;
import hudson.Extension;
import hudson.model.Run;
import hudson.model.Item;
import hudson.model.TaskListener;
import hudson.plugins.git.BranchSpec;
Expand All @@ -53,6 +54,7 @@
import java.io.Writer;
import java.net.URISyntaxException;
import java.util.Arrays;
import java.util.Collections;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Lock;
Expand All @@ -76,6 +78,8 @@
import org.jenkinsci.plugins.gitclient.Git;
import org.jenkinsci.plugins.gitclient.GitClient;

import static hudson.plugins.git.util.GitUtils.expand;

/**
* Base implementation of {@link SCMFileSystem}.
*
Expand Down Expand Up @@ -286,21 +290,35 @@ public boolean supportsDescriptor(SCMSourceDescriptor descriptor) {
@Override
public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull SCMRevision rev)
throws IOException, InterruptedException {
return getScmFileSystem(scm, rev, owner, null);
}

@Override
public SCMFileSystem build(@NonNull Run<?, ?> build, @NonNull SCM scm, SCMRevision rev)
throws IOException, InterruptedException {
Item owner = build.getParent();
return getScmFileSystem(scm, rev, owner, build);
}

private SCMFileSystem getScmFileSystem(@NonNull SCM scm,
SCMRevision rev, Item owner, @CheckForNull Run<?, ?> build) throws IOException, InterruptedException {
if (rev != null && !(rev instanceof AbstractGitSCMSource.SCMRevisionImpl)) {
return null;
}
if (!(scm instanceof GitSCM)) {
return null; // Spotbugs warns about unchecked cast without this check
}
GitSCM gitSCM = (GitSCM) scm;
TaskListener listener = new LogTaskListener(LOGGER, Level.FINE);
UserRemoteConfig config = gitSCM.getUserRemoteConfigs().get(0);
BranchSpec branchSpec = gitSCM.getBranches().get(0);
String remote = config.getUrl();
TaskListener listener = new LogTaskListener(LOGGER, Level.FINE);
EnvVars env = build == null ? null : build.getEnvironment(listener);
String remote = expand(config.getUrl(), env);
if (remote == null) {
listener.getLogger().println("Git remote url is null");
return null;
}
remote = remote.trim();
String cacheEntry = AbstractGitSCMSource.getCacheEntry(remote);
Lock cacheLock = AbstractGitSCMSource.getCacheLock(cacheEntry);
cacheLock.lock();
Expand Down Expand Up @@ -334,7 +352,7 @@ public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull
listener.getLogger().println("Creating git repository in " + cacheDir);
client.init();
}
String remoteName = StringUtils.defaultIfBlank(config.getName(), Constants.DEFAULT_REMOTE_NAME);
String remoteName = StringUtils.defaultIfBlank(expand(config.getName(), env), Constants.DEFAULT_REMOTE_NAME).trim();
listener.getLogger().println("Setting " + remoteName + " to " + remote);
client.setRemoteUrl(remoteName, remote);
listener.getLogger().println("Fetching & pruning " + remoteName + "...");
Expand All @@ -344,28 +362,37 @@ public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull
} catch (URISyntaxException ex) {
listener.getLogger().println("URI syntax exception for '" + remoteName + "' " + ex);
}
String prefix = Constants.R_HEADS;
String prefix = Constants.R_HEADS;
if(branchSpec.getName().startsWith(Constants.R_TAGS)){
prefix = Constants.R_TAGS;
prefix = Constants.R_TAGS;
}
String headName;
if (rev != null) {
headName = rev.getHead().getName();
} else {
if (branchSpec.getName().startsWith(prefix)){
headName = branchSpec.getName().substring(prefix.length());
headName = branchSpec.getName().substring(prefix.length());
} else if (branchSpec.getName().startsWith("*/")) {
headName = branchSpec.getName().substring(2);
} else {
headName = branchSpec.getName();
}
}
client.fetch_().prune(true).from(remoteURI, Arrays
.asList(new RefSpec(
"+" + prefix + headName + ":" + Constants.R_REMOTES + remoteName + "/"
+ headName))).execute();
headName = expand(headName, env).trim();
Copy link

@gnin-slack gnin-slack Jul 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I discovered that this doesn't behave in quite the same way for statically provided strings vs environment variables in that by expanding the variable here rather than earlier we're forgoing some of the input normalization being done above.

example:
*/master will become refs/remotes/origin/master (probably) whereas

${SOME_ENV_VAR} whose value is */master will become refs/remotes/origin/*/master which will NPE a little later on in the code.

Obviously it's easy enough to just not include */ in the variable but it may also be worthwhile for variables and static strings to go through the same process of string normalization. 🤷

Copy link

@akostadinov akostadinov Oct 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*/master vs master does not make any difference when expansion is not used? I wonder why is it supported in non-expansion mode in this case. Perhaps keep expansion as implemented here and consider changing non-expansion version later?


String refspec = expand(config.getRefspec(), env);
String head = headName;
if (refspec == null) {
refspec = "+" + Constants.R_HEADS + headName + ":" + Constants.R_REMOTES + remoteName + "/" + headName;
head = Constants.R_REMOTES + remoteName + "/" +headName;
}
else {
refspec = refspec.trim();
}
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 + "/" +headName, (AbstractGitSCMSource.SCMRevisionImpl) rev);
return new GitSCMFileSystem(client, remote, head, (AbstractGitSCMSource.SCMRevisionImpl) rev);
} finally {
cacheLock.unlock();
}
Expand Down
28 changes: 24 additions & 4 deletions src/main/java/jenkins/plugins/git/GitSCMTelescope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@
import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;
import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.EnvVars;
import hudson.ExtensionList;
import hudson.model.Item;
import hudson.model.Queue;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.model.queue.Tasks;
import hudson.plugins.git.BranchSpec;
import hudson.plugins.git.GitSCM;
Expand All @@ -54,6 +57,8 @@
import org.eclipse.jgit.lib.Constants;
import org.jenkinsci.plugins.gitclient.GitClient;

import static hudson.plugins.git.util.GitUtils.expand;

/**
* An implementation of this extension point allows {@link AbstractGitSCMSource} to examine a repository from a distance
* without requiring a local checkout.
Expand Down Expand Up @@ -181,14 +186,27 @@ public final SCMFileSystem build(@NonNull SCMSource source, @NonNull SCMHead hea
@Override
public final SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, SCMRevision rev)
throws IOException, InterruptedException {
return build(scm, rev, owner, null);
}

@Override
public SCMFileSystem build(@NonNull Run<?, ?> build, @NonNull SCM scm, SCMRevision rev)
throws IOException, InterruptedException {
Item owner = build.getParent();
return build(scm, rev, owner, build);
}

private SCMFileSystem build(@NonNull SCM scm, SCMRevision rev, Item owner, @CheckForNull Run<?, ?> build)
throws IOException, InterruptedException {
if (scm instanceof GitSCM) {
// we only support the GitSCM if the branch is completely unambiguous
GitSCM git = (GitSCM) scm;
List<UserRemoteConfig> configs = git.getUserRemoteConfigs();
List<BranchSpec> branches = git.getBranches();
EnvVars env = build == null ? null : build.getEnvironment(TaskListener.NULL);
if (configs.size() == 1) {
UserRemoteConfig config = configs.get(0);
String remote = config.getUrl();
String remote = expand(config.getUrl(), env);
if (remote != null && supports(remote)
&& branches.size() == 1 && !branches.get(0).getName().contains("*")) {
StandardCredentials credentials;
Expand All @@ -210,7 +228,7 @@ public final SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, SCMRevis
validate(remote, credentials);
SCMHead head;
if (rev == null) {
String name = branches.get(0).getName();
String name = expand(branches.get(0).getName(), env);
if (name.startsWith(Constants.R_TAGS)) {
head = new GitTagSCMHead(
name.substring(Constants.R_TAGS.length()),
Expand All @@ -219,8 +237,10 @@ public final SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, SCMRevis
} else if (name.startsWith(Constants.R_HEADS)) {
head = new GitBranchSCMHead(name.substring(Constants.R_HEADS.length()));
} else {
if (name.startsWith(config.getName() + "/")) {
head = new GitBranchSCMHead(name.substring(config.getName().length() + 1));
String remoteName = expand(config.getName(), env);
if (name.startsWith(remoteName + "/")) {
head = new GitBranchSCMHead(name.substring(
remoteName.length() + 1));
} else {
head = new GitBranchSCMHead(name);
}
Expand Down