Skip to content
Permalink
Browse files

Merge pull request #284 from MarkEWaite/fix-submodule-url-regex

Fix submodule url regex [JENKINS-46054]
  • Loading branch information
MarkEWaite committed Nov 18, 2017
2 parents c2b1ff4 + 52f681e commit b8bad8ab164e146e24eb38bc3a0715374eb25587
@@ -141,6 +141,25 @@
private StandardCredentials defaultCredentials;
private StandardCredentials lfsCredentials;

/* git config --get-regex applies the regex to match keys, and returns all matches (including substring matches).
* Thus, a config call:
* git config -f .gitmodules --get-regexp "^submodule\.([^ ]+)\.url"
* will report two lines of output if the submodule URL includes ".url":
* submodule.modules/JENKINS-46504.url.path modules/JENKINS-46504.url
* submodule.modules/JENKINS-46504.url.url https://github.com/MarkEWaite/JENKINS-46054.url
* The code originally used the same pattern for get-regexp and for output parsing.
* By using the same pattern in both places, it incorrectly took the first line
* of output as the URL of a submodule (when it is instead the path of a submodule).
*/
private final static String SUBMODULE_REMOTE_PATTERN_CONFIG_KEY = "^submodule\\.([^ ]+)\\.url";

/* See comments for SUBMODULE_REMOTE_PATTERN_CONFIG_KEY to explain why this
* regular expression string adds the trailing space character as part of its match.
* Without the trailing whitespace character in the pattern, too many matches are found.
*/
/* Package protected for testing */
final static String SUBMODULE_REMOTE_PATTERN_STRING = SUBMODULE_REMOTE_PATTERN_CONFIG_KEY + "\\b\\s";

private void warnIfWindowsTemporaryDirNameHasSpaces() {
if (!isWindows()) {
return;
@@ -1089,7 +1108,7 @@ else if (!referencePath.isDirectory())
try {
// We might fail if we have no modules, so catch this
// exception and just return.
cfgOutput = launchCommand("config", "-f", ".gitmodules", "--get-regexp", "^submodule\\.(.*)\\.url");
cfgOutput = launchCommand("config", "-f", ".gitmodules", "--get-regexp", SUBMODULE_REMOTE_PATTERN_CONFIG_KEY);
} catch (GitException e) {
listener.error("No submodules found.");
return;
@@ -1098,7 +1117,7 @@ else if (!referencePath.isDirectory())
// Use a matcher to find each configured submodule name, and
// then run the submodule update command with the provided
// path.
Pattern pattern = Pattern.compile("^submodule\\.(.*)\\.url", Pattern.MULTILINE);
Pattern pattern = Pattern.compile(SUBMODULE_REMOTE_PATTERN_STRING, Pattern.MULTILINE);
Matcher matcher = pattern.matcher(cfgOutput);
while (matcher.find()) {
ArgumentListBuilder perModuleArgs = args.clone();
@@ -16,6 +16,7 @@
import java.io.UnsupportedEncodingException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -1437,6 +1438,54 @@ public void testSubmodulesUsedFromOtherBranches() throws Exception {
assertSubmoduleStatus(gitClient, true, "firewall", "ntp", "sshkeys"); // newDirName module won't be there
}

@Issue("JENKINS-46054")
@Test
public void testSubmoduleUrlEndsWithDotUrl() throws Exception {
// Create a new repository that includes ".url" in directory name
File baseDir = tempFolder.newFolder();
File urlRepoDir = new File(baseDir, "my-submodule.url");
assertTrue("Failed to create URL repo dir", urlRepoDir.mkdir());
GitClient urlRepoClient = Git.with(TaskListener.NULL, new EnvVars()).in(urlRepoDir).using(gitImplName).getClient();
urlRepoClient.init();
File readme = new File(urlRepoDir, "readme");
String readmeText = "This repo includes .url in its directory name (" + random.nextInt() + ")";
Files.write(Paths.get(readme.getAbsolutePath()), readmeText.getBytes());
urlRepoClient.add("readme");
urlRepoClient.commit("Added README to repo used as a submodule");

// Add new repository as submodule to repository that ends in .url
File repoHasSubmodule = new File(baseDir, "has-submodule.url");
assertTrue("Failed to create repo dir that will have submodule", repoHasSubmodule.mkdir());
GitClient repoHasSubmoduleClient = Git.with(TaskListener.NULL, new EnvVars()).in(repoHasSubmodule).using(gitImplName).getClient();
repoHasSubmoduleClient.init();
File hasSubmoduleReadme = new File(repoHasSubmodule, "readme");
String hasSubmoduleReadmeText = "Repo has a submodule that includes .url in its directory name (" + random.nextInt() + ")";
Files.write(Paths.get(hasSubmoduleReadme.getAbsolutePath()), hasSubmoduleReadmeText.getBytes());
repoHasSubmoduleClient.add("readme");
repoHasSubmoduleClient.commit("Added README to repo that will include a submodule whose URL ends in '.url'");
String moduleDirBaseName = "module.named.url";
File modulesDir = new File(repoHasSubmodule, "modules");
assertTrue("Failed to create modules dir in repoHasSubmodule", modulesDir.mkdir());
repoHasSubmoduleClient.addSubmodule(repoHasSubmodule.getAbsolutePath(), "modules/" + moduleDirBaseName);
repoHasSubmoduleClient.add(".");
repoHasSubmoduleClient.commit("Add modules/" + moduleDirBaseName + " as submodule");
repoHasSubmoduleClient.submoduleInit();
repoHasSubmoduleClient.submoduleUpdate(false);
assertSubmoduleStatus(repoHasSubmoduleClient, true, moduleDirBaseName);

// Clone repoHasSubmodule to new repository with submodule
File cloneDir = new File(baseDir, "cloned-submodule");
assertTrue("Failed to create clone dir", cloneDir.mkdir());
GitClient cloneGitClient = Git.with(TaskListener.NULL, new EnvVars()).in(cloneDir).using(gitImplName).getClient();
cloneGitClient.init();
cloneGitClient.clone_().url(repoHasSubmodule.getAbsolutePath()).execute();
String branch = "master";
cloneGitClient.checkoutBranch(branch, "origin/" + branch);
cloneGitClient.submoduleInit();
cloneGitClient.submoduleUpdate().recursive(false).execute();
assertSubmoduleStatus(cloneGitClient, true, moduleDirBaseName);
}

@Test
public void testGetSubmodules() throws Exception {
assumeThat(gitImplName, is("git")); // JGit implementation doesn't handle renamed submodules
@@ -0,0 +1,110 @@
package org.jenkinsci.plugins.gitclient;

import java.util.regex.Matcher;
import java.util.regex.Pattern;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.*;
import org.junit.Before;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;

public class SubmodulePatternStringTest {

private Pattern submoduleConfigPattern;
private String remoteName = "simple-name";

@Before
public void configure() {
// String patternString = "^submodule\\.([^ ]+)\\.url ";
String patternString = CliGitAPIImpl.SUBMODULE_REMOTE_PATTERN_STRING;
submoduleConfigPattern = Pattern.compile(patternString, Pattern.MULTILINE);
}

@Issue("46054")
@Test
public void urlEmbeddedInRepoURL() {
String repoUrl = "file://gitroot/thirdparty.url.repo.git";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
}

@Issue("46054")
@Test
public void urlEmbeddedInRepoURLsubmoduleEmbeddedDot() {
String repoUrl = "https://mark.url:some%20pass.urlify@gitroot/repo.git";
remoteName = "simple.name";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
}

@Issue("46054")
@Test
public void urlEmbeddedInSubmoduleRepoNameEndsWithURL() {
String repoUrl = "https://gitroot/repo.url";
remoteName = "simple.name";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
}

@Issue("46054")
@Test
public void urlEmbeddedInSubmoduleRepoNameEndsWithURLSpace() {
String repoUrl = "https://gitroot/repo.url ";
remoteName = "simple.name";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
}

@Issue("46054")
@Test
public void urlEmbeddedInSubmoduleNameAndRepoNameEndsWithURL() {
String repoUrl = "https://gitroot/repo.url.git";
remoteName = "simple.name.url";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
}

@Issue("46054")
@Test
public void urlExploratoryTestFailureCase() {
/* See https://github.com/MarkEWaite/JENKINS-46054.url/ */
String repoUrl = "https://github.com/MarkEWaite/JENKINS-46054.url";
remoteName = "modules/JENKINS-46504.url";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
}

@Issue("46054")
@Test
public void remoteNameIncludesSubmodule() {
/* See https://github.com/MarkEWaite/JENKINS-46054.url/ */
String repoUrl = "https://github.com/MarkEWaite/JENKINS-46054.url";
remoteName = "submodule.JENKINS-46504.url";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
}

@Issue("46054")
@Test
public void urlEmbeddedInRepoNameEndsWithURLEmptyRemoteName() {
String repoUrl = "https://github.com/MarkEWaite/JENKINS-46054.url.git";
remoteName = "";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertFalse("Unexpected match found for '" + submoduleConfigOutput + "'", matcher.find());
}
}

0 comments on commit b8bad8a

Please sign in to comment.
You can’t perform that action at this time.