Skip to content

Commit 6d2b42c

Browse files
MarkEWaiteMarkEWaite
committed
Fix [JENKINS-55827] - local tool definition ignored on agent
Revert "[JENKINS-52754] - GitBranchSource should consult with GitTools node-specific tool installers" This reverts commit 77967db. This is an intentionally short term fix that reverts the fix for JENKINS-52754 (GitBranchSource should consult with GitTools node-specific tool installers) in order to resolve the more serious problem of JENKINS-55827 (local tool definition ignored on agent).
1 parent 8c2f799 commit 6d2b42c

File tree

6 files changed

+29
-128
lines changed

6 files changed

+29
-128
lines changed

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

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -915,9 +915,15 @@ private RemoteConfig newRemoteConfig(String name, String refUrl, RefSpec... refS
915915
}
916916
}
917917

918-
@CheckForNull
918+
@SuppressFBWarnings(value="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE", justification="Jenkins.getInstance() is not null")
919919
public GitTool resolveGitTool(TaskListener listener) {
920-
return GitUtils.resolveGitTool(gitTool, listener);
920+
if (gitTool == null) return GitTool.getDefaultInstallation();
921+
GitTool git = Jenkins.getInstance().getDescriptorByType(GitTool.DescriptorImpl.class).getInstallation(gitTool);
922+
if (git == null) {
923+
listener.getLogger().println("Selected Git installation does not exist. Using Default");
924+
git = GitTool.getDefaultInstallation();
925+
}
926+
return git;
921927
}
922928

923929
public String getGitExe(Node builtOn, TaskListener listener) {
@@ -943,9 +949,16 @@ public String getGitExe(Node builtOn, EnvVars env, TaskListener listener) {
943949
}
944950
if (client == GitClientType.JGIT) return JGitTool.MAGIC_EXENAME;
945951

946-
GitTool tool = GitUtils.resolveGitTool(gitTool, listener);
947-
if (tool == null) {
948-
return null;
952+
GitTool tool = resolveGitTool(listener);
953+
if (builtOn != null) {
954+
try {
955+
tool = tool.forNode(builtOn, listener);
956+
} catch (IOException | InterruptedException e) {
957+
listener.getLogger().println("Failed to get git executable");
958+
}
959+
}
960+
if (env != null) {
961+
tool = tool.forEnvironment(env);
949962
}
950963

951964
return tool.getGitExe();

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

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import hudson.plugins.git.BranchSpec;
1111
import hudson.plugins.git.GitException;
1212
import hudson.plugins.git.GitObject;
13-
import hudson.plugins.git.GitTool;
1413
import hudson.plugins.git.Revision;
1514
import hudson.remoting.VirtualChannel;
1615
import hudson.slaves.NodeProperty;
@@ -30,7 +29,6 @@
3029
import java.util.*;
3130
import java.util.logging.Level;
3231
import java.util.logging.Logger;
33-
import javax.annotation.CheckForNull;
3432
import javax.annotation.Nonnull;
3533

3634
public class GitUtils implements Serializable {
@@ -46,56 +44,6 @@ public GitUtils(@Nonnull TaskListener listener, @Nonnull GitClient git) {
4644
this.listener = listener;
4745
}
4846

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-
9947
public static Node workspaceToNode(FilePath workspace) { // TODO https://trello.com/c/doFFMdUm/46-filepath-getcomputer
10048
Jenkins j = Jenkins.getActiveInstance();
10149
if (workspace != null && workspace.isRemote()) {

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

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import hudson.model.Action;
4141
import hudson.model.Actionable;
4242
import hudson.model.Item;
43-
import hudson.model.Node;
4443
import hudson.model.TaskListener;
4544
import hudson.plugins.git.Branch;
4645
import hudson.plugins.git.GitException;
@@ -55,7 +54,6 @@
5554
import hudson.plugins.git.util.BuildChooserContext;
5655
import hudson.plugins.git.util.BuildChooserDescriptor;
5756
import hudson.plugins.git.util.BuildData;
58-
import hudson.plugins.git.util.GitUtils;
5957
import hudson.scm.SCM;
6058
import hudson.security.ACL;
6159
import java.io.File;
@@ -300,17 +298,14 @@ protected GitTool resolveGitTool() {
300298
* @param gitTool the {@link GitTool#getName()} to resolve.
301299
* @return the {@link GitTool}
302300
* @since 3.4.0
303-
* @deprecated Use {@link #resolveGitTool(String, TaskListener)} instead
304301
*/
305302
@CheckForNull
306-
@Deprecated
307303
protected GitTool resolveGitTool(String 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);
304+
return StringUtils.isBlank(gitTool)
305+
? GitTool.getDefaultInstallation()
306+
: Jenkins.getActiveInstance()
307+
.getDescriptorByType(GitTool.DescriptorImpl.class)
308+
.getInstallation(gitTool);
314309
}
315310

316311
private interface Retriever<T> {
@@ -329,7 +324,7 @@ private <T, C extends GitSCMSourceContext<C, R>, R extends GitSCMSourceRequest>
329324
try {
330325
File cacheDir = getCacheDir(cacheEntry);
331326
Git git = Git.with(listener, new EnvVars(EnvVars.masterEnvVars)).in(cacheDir);
332-
GitTool tool = resolveGitTool(context.gitTool(), listener);
327+
GitTool tool = resolveGitTool(context.gitTool());
333328
if (tool != null) {
334329
git.using(tool.getGitExe());
335330
}
@@ -799,7 +794,7 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta
799794
// 8. A short/full revision hash that is not the head revision of a branch (we'll need to fetch everything to
800795
// try and resolve the hash from the history of one of the heads)
801796
Git git = Git.with(listener, new EnvVars(EnvVars.masterEnvVars));
802-
GitTool tool = resolveGitTool(context.gitTool(), listener);
797+
GitTool tool = resolveGitTool(context.gitTool());
803798
if (tool != null) {
804799
git.using(tool.getGitExe());
805800
}
@@ -1022,7 +1017,7 @@ protected Set<String> retrieveRevisions(@NonNull final TaskListener listener) th
10221017
return result;
10231018
}
10241019
Git git = Git.with(listener, new EnvVars(EnvVars.masterEnvVars));
1025-
GitTool tool = resolveGitTool(context.gitTool(), listener);
1020+
GitTool tool = resolveGitTool(context.gitTool());
10261021
if (tool != null) {
10271022
git.using(tool.getGitExe());
10281023
}
@@ -1089,7 +1084,7 @@ protected List<Action> retrieveActions(@CheckForNull SCMSourceEvent event, @NonN
10891084
final GitSCMSourceContext context =
10901085
new GitSCMSourceContext<>(null, SCMHeadObserver.none()).withTraits(getTraits());
10911086
Git git = Git.with(listener, new EnvVars(EnvVars.masterEnvVars));
1092-
GitTool tool = resolveGitTool(context.gitTool(), listener);
1087+
GitTool tool = resolveGitTool(context.gitTool());
10931088
if (tool != null) {
10941089
git.using(tool.getGitExe());
10951090
}

‎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(), listener);
374+
GitTool tool = gitSCMSource.resolveGitTool(builder.gitTool());
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, TaskListener.NULL);
59+
PowerMockito.doReturn(mockedTool).when(gitSCMSource).resolveGitTool(EXPECTED_GIT_EXE);
6060
}
6161

6262
/**

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

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,15 @@
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;
75
import hudson.model.Action;
86
import hudson.model.Item;
9-
import hudson.model.Node;
107
import hudson.model.TaskListener;
118
import hudson.model.TopLevelItem;
129
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;
1910
import hudson.util.LogTaskListener;
2011
import java.io.ByteArrayInputStream;
2112
import java.io.FileNotFoundException;
2213
import java.io.InputStream;
23-
import java.io.StringWriter;
2414
import java.nio.charset.StandardCharsets;
2515
import java.util.Arrays;
2616
import java.util.List;
@@ -30,8 +20,6 @@
3020
import java.util.concurrent.TimeoutException;
3121
import java.util.logging.Level;
3222
import java.util.logging.Logger;
33-
34-
import hudson.util.StreamTaskListener;
3523
import jenkins.plugins.git.traits.BranchDiscoveryTrait;
3624
import jenkins.plugins.git.traits.TagDiscoveryTrait;
3725
import jenkins.scm.api.SCMEventListener;
@@ -47,7 +35,6 @@
4735
import jenkins.scm.api.metadata.PrimaryInstanceMetadataAction;
4836
import jenkins.scm.api.trait.SCMSourceTrait;
4937
import org.hamcrest.Matchers;
50-
import org.junit.Assume;
5138
import org.junit.Before;
5239
import org.junit.Rule;
5340
import org.junit.Test;
@@ -63,7 +50,6 @@
6350
import static org.hamcrest.Matchers.allOf;
6451
import static org.hamcrest.Matchers.contains;
6552
import static org.hamcrest.Matchers.containsInAnyOrder;
66-
import static org.hamcrest.Matchers.containsString;
6753
import static org.hamcrest.Matchers.hasItem;
6854
import static org.hamcrest.Matchers.hasProperty;
6955
import static org.hamcrest.Matchers.hasSize;
@@ -73,7 +59,6 @@
7359
import static org.hamcrest.Matchers.nullValue;
7460
import static org.junit.Assert.assertNull;
7561
import static org.junit.Assert.assertThat;
76-
import static org.junit.Assert.assertTrue;
7762
import static org.mockito.Matchers.notNull;
7863
import static org.mockito.Mockito.mock;
7964
import static org.mockito.Mockito.times;
@@ -348,46 +333,6 @@ public void telescopeFetchActions() throws Exception {
348333
is(Collections.<Action>emptyList()));
349334
}
350335

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-
391336
@TestExtension
392337
public static class MyGitSCMTelescope extends GitSCMTelescope {
393338
@Override

0 commit comments

Comments
 (0)