Skip to content

Commit 55eea5b

Browse files
MarkEWaiteMarkEWaite
authored
Merge pull request #605 from oleg-nenashev/bug/toolInstallerOnMaster
[JENKINS-52754] - GitBranchSource should consult with GitTools node-specific tool installers
2 parents 5ffad06 + 77967db commit 55eea5b

File tree

6 files changed

+128
-29
lines changed

6 files changed

+128
-29
lines changed

‎src/main/java/hudson/plugins/git/GitSCM.java‎

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -909,15 +909,9 @@ private RemoteConfig newRemoteConfig(String name, String refUrl, RefSpec... refS
909909
}
910910
}
911911

912-
@SuppressFBWarnings(value="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE", justification="Jenkins.getInstance() is not null")
912+
@CheckForNull
913913
public GitTool resolveGitTool(TaskListener listener) {
914-
if (gitTool == null) return GitTool.getDefaultInstallation();
915-
GitTool git = Jenkins.getInstance().getDescriptorByType(GitTool.DescriptorImpl.class).getInstallation(gitTool);
916-
if (git == null) {
917-
listener.getLogger().println("Selected Git installation does not exist. Using Default");
918-
git = GitTool.getDefaultInstallation();
919-
}
920-
return git;
914+
return GitUtils.resolveGitTool(gitTool, listener);
921915
}
922916

923917
public String getGitExe(Node builtOn, TaskListener listener) {
@@ -943,16 +937,9 @@ public String getGitExe(Node builtOn, EnvVars env, TaskListener listener) {
943937
}
944938
if (client == GitClientType.JGIT) return JGitTool.MAGIC_EXENAME;
945939

946-
GitTool tool = resolveGitTool(listener);
947-
if (builtOn != null) {
948-
try {
949-
tool = tool.forNode(builtOn, listener);
950-
} catch (IOException | InterruptedException e) {
951-
listener.getLogger().println("Failed to get git executable");
952-
}
953-
}
954-
if (env != null) {
955-
tool = tool.forEnvironment(env);
940+
GitTool tool = GitUtils.resolveGitTool(gitTool, listener);
941+
if (tool == null) {
942+
return null;
956943
}
957944

958945
return tool.getGitExe();

‎src/main/java/hudson/plugins/git/util/GitUtils.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import hudson.plugins.git.BranchSpec;
1111
import hudson.plugins.git.GitException;
1212
import hudson.plugins.git.GitObject;
13+
import hudson.plugins.git.GitTool;
1314
import hudson.plugins.git.Revision;
1415
import hudson.remoting.VirtualChannel;
1516
import hudson.slaves.NodeProperty;
@@ -29,6 +30,7 @@
2930
import java.util.*;
3031
import java.util.logging.Level;
3132
import java.util.logging.Logger;
33+
import javax.annotation.CheckForNull;
3234
import javax.annotation.Nonnull;
3335

3436
public class GitUtils implements Serializable {
@@ -44,6 +46,56 @@ public GitUtils(@Nonnull TaskListener listener, @Nonnull GitClient git) {
4446
this.listener = listener;
4547
}
4648

49+
/**
50+
* Resolves Git Tool by name.
51+
* @param gitTool Tool name. If {@code null}, default tool will be used (if exists)
52+
* @param builtOn Node for which the tool should be resolved
53+
* Can be {@link Jenkins#getInstance()} when running on master
54+
* @param env Additional environment variables
55+
* @param listener Event listener
56+
* @return Tool installation or {@code null} if it cannot be resolved
57+
* @since TODO
58+
*/
59+
@CheckForNull
60+
public static GitTool resolveGitTool(@CheckForNull String gitTool,
61+
@CheckForNull Node builtOn,
62+
@CheckForNull EnvVars env,
63+
@Nonnull TaskListener listener) {
64+
GitTool git = gitTool == null
65+
? GitTool.getDefaultInstallation()
66+
: Jenkins.getActiveInstance().getDescriptorByType(GitTool.DescriptorImpl.class).getInstallation(gitTool);
67+
if (git == null) {
68+
listener.getLogger().println("Selected Git installation does not exist. Using Default");
69+
git = GitTool.getDefaultInstallation();
70+
}
71+
if (git != null) {
72+
if (builtOn != null) {
73+
try {
74+
git = git.forNode(builtOn, listener);
75+
} catch (IOException | InterruptedException e) {
76+
listener.getLogger().println("Failed to get git executable");
77+
}
78+
}
79+
if (env != null) {
80+
git = git.forEnvironment(env);
81+
}
82+
}
83+
return git;
84+
}
85+
86+
/**
87+
* Resolves Git Tool by name in a node-agnostic way.
88+
* Use {@link #resolveGitTool(String, Node, EnvVars, TaskListener)} when the node is known
89+
* @param gitTool Tool name. If {@code null}, default tool will be used (if exists)
90+
* @param listener Event listener
91+
* @return Tool installation or {@code null} if it cannot be resolved
92+
* @since TODO
93+
*/
94+
@CheckForNull
95+
public static GitTool resolveGitTool(@CheckForNull String gitTool, @Nonnull TaskListener listener) {
96+
return resolveGitTool(gitTool, null, null, listener);
97+
}
98+
4799
public static Node workspaceToNode(FilePath workspace) { // TODO https://trello.com/c/doFFMdUm/46-filepath-getcomputer
48100
Jenkins j = Jenkins.getActiveInstance();
49101
if (workspace != null && workspace.isRemote()) {

‎src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import hudson.model.Action;
4141
import hudson.model.Actionable;
4242
import hudson.model.Item;
43+
import hudson.model.Node;
4344
import hudson.model.TaskListener;
4445
import hudson.plugins.git.Branch;
4546
import hudson.plugins.git.GitException;
@@ -54,6 +55,7 @@
5455
import hudson.plugins.git.util.BuildChooserContext;
5556
import hudson.plugins.git.util.BuildChooserDescriptor;
5657
import hudson.plugins.git.util.BuildData;
58+
import hudson.plugins.git.util.GitUtils;
5759
import hudson.scm.SCM;
5860
import hudson.security.ACL;
5961
import java.io.File;
@@ -298,14 +300,17 @@ protected GitTool resolveGitTool() {
298300
* @param gitTool the {@link GitTool#getName()} to resolve.
299301
* @return the {@link GitTool}
300302
* @since 3.4.0
303+
* @deprecated Use {@link #resolveGitTool(String, TaskListener)} instead
301304
*/
302305
@CheckForNull
306+
@Deprecated
303307
protected GitTool resolveGitTool(String gitTool) {
304-
return StringUtils.isBlank(gitTool)
305-
? GitTool.getDefaultInstallation()
306-
: Jenkins.getActiveInstance()
307-
.getDescriptorByType(GitTool.DescriptorImpl.class)
308-
.getInstallation(gitTool);
308+
return resolveGitTool(gitTool, TaskListener.NULL);
309+
}
310+
311+
protected GitTool resolveGitTool(String gitTool, TaskListener listener) {
312+
final Jenkins jenkins = Jenkins.getInstance();
313+
return GitUtils.resolveGitTool(gitTool, jenkins, null, TaskListener.NULL);
309314
}
310315

311316
private interface Retriever<T> {
@@ -324,7 +329,7 @@ private <T, C extends GitSCMSourceContext<C, R>, R extends GitSCMSourceRequest>
324329
try {
325330
File cacheDir = getCacheDir(cacheEntry);
326331
Git git = Git.with(listener, new EnvVars(EnvVars.masterEnvVars)).in(cacheDir);
327-
GitTool tool = resolveGitTool(context.gitTool());
332+
GitTool tool = resolveGitTool(context.gitTool(), listener);
328333
if (tool != null) {
329334
git.using(tool.getGitExe());
330335
}
@@ -794,7 +799,7 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta
794799
// 8. A short/full revision hash that is not the head revision of a branch (we'll need to fetch everything to
795800
// try and resolve the hash from the history of one of the heads)
796801
Git git = Git.with(listener, new EnvVars(EnvVars.masterEnvVars));
797-
GitTool tool = resolveGitTool(context.gitTool());
802+
GitTool tool = resolveGitTool(context.gitTool(), listener);
798803
if (tool != null) {
799804
git.using(tool.getGitExe());
800805
}
@@ -1017,7 +1022,7 @@ protected Set<String> retrieveRevisions(@NonNull final TaskListener listener) th
10171022
return result;
10181023
}
10191024
Git git = Git.with(listener, new EnvVars(EnvVars.masterEnvVars));
1020-
GitTool tool = resolveGitTool(context.gitTool());
1025+
GitTool tool = resolveGitTool(context.gitTool(), listener);
10211026
if (tool != null) {
10221027
git.using(tool.getGitExe());
10231028
}
@@ -1084,7 +1089,7 @@ protected List<Action> retrieveActions(@CheckForNull SCMSourceEvent event, @NonN
10841089
final GitSCMSourceContext context =
10851090
new GitSCMSourceContext<>(null, SCMHeadObserver.none()).withTraits(getTraits());
10861091
Git git = Git.with(listener, new EnvVars(EnvVars.masterEnvVars));
1087-
GitTool tool = resolveGitTool(context.gitTool());
1092+
GitTool tool = resolveGitTool(context.gitTool(), listener);
10881093
if (tool != null) {
10891094
git.using(tool.getGitExe());
10901095
}

‎src/main/java/jenkins/plugins/git/GitSCMFileSystem.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ public SCMFileSystem build(@NonNull SCMSource source, @NonNull SCMHead head, @Ch
371371
try {
372372
File cacheDir = AbstractGitSCMSource.getCacheDir(cacheEntry);
373373
Git git = Git.with(listener, new EnvVars(EnvVars.masterEnvVars)).in(cacheDir);
374-
GitTool tool = gitSCMSource.resolveGitTool(builder.gitTool());
374+
GitTool tool = gitSCMSource.resolveGitTool(builder.gitTool(), listener);
375375
if (tool != null) {
376376
git.using(tool.getGitExe());
377377
}

‎src/test/java/jenkins/plugins/git/AbstractGitSCMSourceRetrieveHeadsTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public void setup() throws Exception {
5656
// Partial mock our AbstractGitSCMSourceImpl
5757
gitSCMSource = PowerMockito.spy(new AbstractGitSCMSourceImpl());
5858
// Always resolve to mocked GitTool
59-
PowerMockito.doReturn(mockedTool).when(gitSCMSource).resolveGitTool(EXPECTED_GIT_EXE);
59+
PowerMockito.doReturn(mockedTool).when(gitSCMSource).resolveGitTool(EXPECTED_GIT_EXE, TaskListener.NULL);
6060
}
6161

6262
/**

‎src/test/java/jenkins/plugins/git/GitSCMSourceTest.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,25 @@
22

33
import com.cloudbees.plugins.credentials.common.StandardCredentials;
44
import edu.umd.cs.findbugs.annotations.NonNull;
5+
import hudson.EnvVars;
6+
import hudson.FilePath;
57
import hudson.model.Action;
68
import hudson.model.Item;
9+
import hudson.model.Node;
710
import hudson.model.TaskListener;
811
import hudson.model.TopLevelItem;
912
import hudson.plugins.git.GitStatus;
13+
import hudson.plugins.git.GitTool;
14+
import hudson.remoting.Launcher;
15+
import hudson.tools.CommandInstaller;
16+
import hudson.tools.InstallSourceProperty;
17+
import hudson.tools.ToolInstallation;
18+
import hudson.tools.ToolInstaller;
1019
import hudson.util.LogTaskListener;
1120
import java.io.ByteArrayInputStream;
1221
import java.io.FileNotFoundException;
1322
import java.io.InputStream;
23+
import java.io.StringWriter;
1424
import java.nio.charset.StandardCharsets;
1525
import java.util.Arrays;
1626
import java.util.List;
@@ -20,6 +30,8 @@
2030
import java.util.concurrent.TimeoutException;
2131
import java.util.logging.Level;
2232
import java.util.logging.Logger;
33+
34+
import hudson.util.StreamTaskListener;
2335
import jenkins.plugins.git.traits.BranchDiscoveryTrait;
2436
import jenkins.plugins.git.traits.TagDiscoveryTrait;
2537
import jenkins.scm.api.SCMEventListener;
@@ -35,6 +47,7 @@
3547
import jenkins.scm.api.metadata.PrimaryInstanceMetadataAction;
3648
import jenkins.scm.api.trait.SCMSourceTrait;
3749
import org.hamcrest.Matchers;
50+
import org.junit.Assume;
3851
import org.junit.Before;
3952
import org.junit.Rule;
4053
import org.junit.Test;
@@ -50,6 +63,7 @@
5063
import static org.hamcrest.Matchers.allOf;
5164
import static org.hamcrest.Matchers.contains;
5265
import static org.hamcrest.Matchers.containsInAnyOrder;
66+
import static org.hamcrest.Matchers.containsString;
5367
import static org.hamcrest.Matchers.hasItem;
5468
import static org.hamcrest.Matchers.hasProperty;
5569
import static org.hamcrest.Matchers.hasSize;
@@ -59,6 +73,7 @@
5973
import static org.hamcrest.Matchers.nullValue;
6074
import static org.junit.Assert.assertNull;
6175
import static org.junit.Assert.assertThat;
76+
import static org.junit.Assert.assertTrue;
6277
import static org.mockito.Matchers.notNull;
6378
import static org.mockito.Mockito.mock;
6479
import static org.mockito.Mockito.times;
@@ -333,6 +348,46 @@ public void telescopeFetchActions() throws Exception {
333348
is(Collections.<Action>emptyList()));
334349
}
335350

351+
352+
@Issue("JENKINS-52754")
353+
@Test
354+
public void gitSCMSourceShouldResolveToolsForMaster() throws Exception {
355+
Assume.assumeTrue("Runs on Unix only", !Launcher.isWindows());
356+
TaskListener log = StreamTaskListener.fromStdout();
357+
HelloToolInstaller inst = new HelloToolInstaller("master", "echo Hello", "git");
358+
GitTool t = new GitTool("myGit", null, Collections.singletonList(
359+
new InstallSourceProperty(Collections.singletonList(inst))));
360+
t.getDescriptor().setInstallations(t);
361+
362+
GitTool defaultTool = GitTool.getDefaultInstallation();
363+
GitTool resolved = (GitTool) defaultTool.translate(jenkins.jenkins, new EnvVars(), TaskListener.NULL);
364+
assertThat(resolved.getGitExe(), org.hamcrest.CoreMatchers.containsString("git"));
365+
366+
GitSCMSource instance = new GitSCMSource("http://git.test/telescope.git");
367+
instance.retrieveRevisions(log);
368+
assertTrue("Installer should be invoked", inst.isInvoked());
369+
}
370+
371+
private static class HelloToolInstaller extends CommandInstaller {
372+
373+
private boolean invoked;
374+
375+
public HelloToolInstaller(String label, String command, String toolHome) {
376+
super(label, command, toolHome);
377+
}
378+
379+
public boolean isInvoked() {
380+
return invoked;
381+
}
382+
383+
@Override
384+
public FilePath performInstallation(ToolInstallation toolInstallation, Node node, TaskListener taskListener) throws IOException, InterruptedException {
385+
taskListener.error("Hello, world!");
386+
invoked = true;
387+
return super.performInstallation(toolInstallation, node, taskListener);
388+
}
389+
}
390+
336391
@TestExtension
337392
public static class MyGitSCMTelescope extends GitSCMTelescope {
338393
@Override

0 commit comments

Comments
 (0)