Skip to content
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

[JENKINS-63519] Create a better mechanism to detect node-specific git implementations #949

Merged
merged 13 commits into from
Sep 12, 2020
Merged

[JENKINS-63519] Create a better mechanism to detect node-specific git implementations #949

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f921410
Resolve JENKINS-63519
rishabhBudhouliya Sep 1, 2020
719f284
Introducing recommendGitToolOnAgent
rishabhBudhouliya Sep 4, 2020
1ceed4d
Add better assertion for unit test
rishabhBudhouliya Sep 4, 2020
26eb95e
Add more tests to describe the behavior of GitToolChooser if user's c…
rishabhBudhouliya Sep 4, 2020
193b7e5
Remove assertion of NONE to jgit for testGitToolWithOnlyJGit
rishabhBudhouliya Sep 5, 2020
34093c3
Fix another test assertion statement
rishabhBudhouliya Sep 5, 2020
25239d1
Remove unreachable check condition in case of command line git is rec…
rishabhBudhouliya Sep 5, 2020
43c3094
Add missing params for javadoc
rishabhBudhouliya Sep 5, 2020
7bc2b84
Fix Windows specific tests for git tool, use isWindows()
rishabhBudhouliya Sep 7, 2020
cfa0661
Fix Windows specific paths
rishabhBudhouliya Sep 7, 2020
2f985e3
Add javadoc for getResolvedGitTool
rishabhBudhouliya Sep 7, 2020
c82a734
Merge branch 'master' into JENKINS-63519
rishabhBudhouliya Sep 8, 2020
34a146d
Ignore platform specific test cases on Windows
rishabhBudhouliya Sep 8, 2020
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
9 changes: 8 additions & 1 deletion src/main/java/hudson/plugins/git/GitSCM.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,8 @@ public GitClient createClient(TaskListener listener, EnvVars environment, Run<?,

String gitExe = getGitExe(n, listener);

GitTool gitTool = getGitTool(n, null, listener);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added to access the GitTool chosen by the system instead of the git executable path for GitToolChooser.


if (!isDisableGitToolChooser()) {
UnsupportedCommand unsupportedCommand = new UnsupportedCommand();
for (GitSCMExtension ext : extensions) {
Expand All @@ -847,7 +849,7 @@ public GitClient createClient(TaskListener listener, EnvVars environment, Run<?,
for (UserRemoteConfig uc : getUserRemoteConfigs()) {
String ucCredentialsId = uc.getCredentialsId();
String url = getParameterString(uc.getUrl(), environment);
chooser = new GitToolChooser(url, project, ucCredentialsId, gitExe, unsupportedCommand.determineSupportForJGit());
chooser = new GitToolChooser(url, project, ucCredentialsId, gitTool, n, listener,unsupportedCommand.determineSupportForJGit());
}
listener.getLogger().println("The recommended git tool is: " + chooser.getGitTool());
String updatedGitExe = chooser.getGitTool();
Expand Down Expand Up @@ -1000,6 +1002,11 @@ public String getGitExe(Node builtOn, EnvVars env, TaskListener listener) {
return tool.getGitExe();
}

public GitTool getGitTool(Node builtOn, EnvVars env, TaskListener listener) {
GitTool tool = GitUtils.resolveGitTool(gitTool, builtOn, env, listener);
return tool;
}

/*package*/ static class BuildChooserContextImpl implements BuildChooserContext, Serializable {
@SuppressFBWarnings(value="SE_BAD_FIELD", justification="known non-serializable field")
final Job project;
Expand Down
104 changes: 71 additions & 33 deletions src/main/java/jenkins/plugins/git/GitToolChooser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import hudson.ExtensionList;
import hudson.ExtensionPoint;
import hudson.model.Item;
import hudson.model.Node;
import hudson.model.TaskListener;
import hudson.plugins.git.GitTool;
import hudson.plugins.git.util.GitUtils;
Expand All @@ -16,6 +17,7 @@

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.logging.Level;
Expand All @@ -31,6 +33,8 @@ public class GitToolChooser {
private long sizeOfRepo = 0L;
private String implementation;
private String gitTool;
private TaskListener listener;
private Node currentNode;
/**
* Size to switch implementation in KiB
*/
Expand All @@ -43,17 +47,21 @@ public class GitToolChooser {
* @param remoteName the repository url
* @param projectContext the context where repository size is being estimated
* @param credentialsId credential used to access the repository or null if no credential is required
* @param gitExe name of the git tool ('git', 'jgit', 'jgitapache') to be used as the default tool
* @param gitExe Git tool ('git', 'jgit', 'jgitapache') to be used as the default tool
* @param n A Jenkins agent used to check validity of git installation
* @param listener TaskListener required by GitUtils.resolveGitTool()
* @param useJGit if true the JGit is allowed as an implementation
* @throws IOException on error
* @throws InterruptedException on error
*/
public GitToolChooser(String remoteName, Item projectContext, String credentialsId, String gitExe, Boolean useJGit) throws IOException, InterruptedException {
public GitToolChooser(String remoteName, Item projectContext, String credentialsId,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add javadoc comments for the new parameters. That will resolve the javadoc warnings below and keep the javadoc clean

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

GitTool gitExe, Node n, TaskListener listener, Boolean useJGit) throws IOException, InterruptedException {
boolean useCache = false;
if (useJGit != null) {
JGIT_SUPPORTED = useJGit;
}

currentNode = n;
this.listener = listener;
implementation = "NONE";
useCache = decideAndUseCache(remoteName);

Expand All @@ -62,7 +70,11 @@ public GitToolChooser(String remoteName, Item projectContext, String credentials
} else {
decideAndUseAPI(remoteName, projectContext, credentialsId, gitExe);
}
determineGitTool(implementation, gitExe);
gitTool = implementation;
}

public void addListener(TaskListener listener) {
this.listener = listener;
}

/**
Expand All @@ -88,7 +100,7 @@ private boolean decideAndUseCache(String remoteName) throws IOException, Interru
return useCache;
}

private void decideAndUseAPI(String remoteName, Item context, String credentialsId, String gitExe) {
private void decideAndUseAPI(String remoteName, Item context, String credentialsId, GitTool gitExe) {
if (setSizeFromAPI(remoteName, context, credentialsId)) {
implementation = determineSwitchOnSize(sizeOfRepo, gitExe);
}
Expand Down Expand Up @@ -126,35 +138,73 @@ private boolean setSizeFromAPI(String repoUrl, Item context, String credentialsI
* @param sizeOfRepo: Size of a repository (in KiBs)
* @return a git implementation, "git" or "jgit"
*/
String determineSwitchOnSize(Long sizeOfRepo, String gitExe) {
String determineSwitchOnSize(Long sizeOfRepo, GitTool tool) {
if (sizeOfRepo != 0L) {
if (sizeOfRepo < SIZE_TO_SWITCH) {
if (!JGIT_SUPPORTED) {
return "NONE";
}
return determineToolName(gitExe, JGitTool.MAGIC_EXENAME);
GitTool rTool = resolveGitToolForRecommendation(tool, JGitTool.MAGIC_EXENAME);
if (rTool == null) {
return "NONE";
}
return rTool.getGitExe();
} else {
return determineToolName(gitExe, "git");
GitTool rTool = resolveGitToolForRecommendation(tool, "git");
return rTool.getGitExe();
}
}
return "NONE";
}

/**
* For a given recommended git implementation, validate if the installation exists and provide no suggestion if
* implementation doesn't exist.
* @param gitImplementation: The recommended git implementation, "git" or "jgit" on the basis of the heuristics.
*/
private void determineGitTool(String gitImplementation, String gitExe) {
if (gitImplementation.equals("NONE")) {
gitTool = "NONE";
return; // Recommend nothing (GitToolRecommendation = NONE)
private GitTool resolveGitToolForRecommendation(GitTool userChoice, String recommendation) {
GitTool tool;
if (recommendation.equals(JGitTool.MAGIC_EXENAME)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For repository size < 5MiB, the recommendation should be jgit/jgitapache. This branch of logic will try to recommend jgit or jgitapache and confirm with the system if it is enabled by the user. It would return a null if it is not enabled by the user.

if (userChoice.getGitExe().equals(JGitApacheTool.MAGIC_EXENAME)) {
recommendation = JGitApacheTool.MAGIC_EXENAME;
}
// check if jgit or jgitapache is enabled
tool = getResolvedGitTool(recommendation);
if (tool.getName().equals(recommendation)) {
return tool;
} else {
return null;
}
} else {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This branch of logic is designed for repo size > 5M.
Here two things can happen:

  • if user's choice is not jgit or jgitapache, it would be a command line git tool. We would return that same choice without any concern.
  • if the user's choice is jgit or jgitapache, it would look for the right command line git tool using recommendGitToolOnAgent.

if (!userChoice.getName().equals(JGitTool.MAGIC_EXENAME) && !userChoice.getName().equals(JGitApacheTool.MAGIC_EXENAME)) {
return userChoice;
}
else {
return recommendGitToolOnAgent(userChoice);
}
}
final Jenkins jenkins = Jenkins.get();
GitTool tool = GitUtils.resolveGitTool(gitImplementation, jenkins, null, TaskListener.NULL);
if (tool != null) {
gitTool = tool.getGitExe();
}

public GitTool recommendGitToolOnAgent(GitTool userChoice) {
List<GitTool> preferredToolList = new ArrayList<>();
GitTool correctTool = GitTool.getDefaultInstallation();
String toolName = userChoice.getName();
if (toolName.equals(JGitTool.MAGIC_EXENAME) || toolName.equals(JGitApacheTool.MAGIC_EXENAME)) {
GitTool[] toolList = Jenkins.get().getDescriptorByType(GitTool.DescriptorImpl.class).getInstallations();
for (GitTool tool : toolList) {
if (!tool.getProperties().isEmpty()) {
preferredToolList.add(tool);
}
}
for (GitTool tool: preferredToolList) {
if (tool.getName().equals(getResolvedGitTool(tool.getName()).getName())) {
correctTool = getResolvedGitTool(tool.getName());
}
}
}
return correctTool;
}

private GitTool getResolvedGitTool(String recommendation) {
if (currentNode == null) {
currentNode = Jenkins.get();
}
return GitUtils.resolveGitTool(recommendation, currentNode, null, listener);
}

/**
Expand All @@ -165,18 +215,6 @@ public String getGitTool() {
return gitTool;
}

private String determineToolName(String gitExe, String recommendation) {
if (gitExe.contains(recommendation) && !gitExe.equals(JGitTool.MAGIC_EXENAME) && !gitExe.equals(JGitApacheTool.MAGIC_EXENAME)) {
return gitExe;
}
if (!recommendation.equals(gitExe)) {
if (gitExe.equals(JGitApacheTool.MAGIC_EXENAME) && recommendation.equals(JGitTool.MAGIC_EXENAME)) {
return gitExe;
}
}
return recommendation;
}

/**
* Other plugins can estimate the size of repository using this extension point
* The size is assumed to be in KiBs
Expand Down
Loading