Fix ref with clone modules, fixes #213 (#214)

* fix TestCloneGitModules to check for new output, add default empty commandDir parameter

* fix modules with -clonegit if a non-branch was specified, add default empty commandDir parameter

* add default empty commandDir parameter

g10k_test.go

@@ -684,7 +684,7 @@ func TestConfigUseCacheFallback(t *testing.T) {
 	}
 
 	// change the git remote url to something that does not resolve https://.com/...
-	er := executeCommand("git --git-dir "+unresolvableGitDir+" remote set-url origin https://.com/puppetlabs/puppetlabs-firewall.git", 5, false)
+	er := executeCommand("git --git-dir "+unresolvableGitDir+" remote set-url origin https://.com/puppetlabs/puppetlabs-firewall.git", "", 5, false)
 	if er.returnCode != 0 {
 		t.Error("Rewriting the git remote url of " + unresolvableGitDir + " to https://.com/puppetlabs/puppetlabs-firewall.git failed! Errorcode: " + strconv.Itoa(er.returnCode) + "Error: " + er.output)
 	}
@@ -730,7 +730,7 @@ func TestEnvFullSyncIfModuleWasTemporarilyNotAvailable(t *testing.T) {
 	doMirrorOrUpdate(gm, "/tmp/g10k/modules/https-__github.com_puppetlabs_puppetlabs-firewall.git", 0)
 
 	// change the git remote url to something that does not resolve https://.com/...
-	er := executeCommand("git --git-dir "+gitDir+" remote set-url origin https://.com/puppetlabs/puppetlabs-firewall.git", 5, false)
+	er := executeCommand("git --git-dir "+gitDir+" remote set-url origin https://.com/puppetlabs/puppetlabs-firewall.git", "", 5, false)
 	if er.returnCode != 0 {
 		t.Error("Rewriting the git remote url of " + gitDir + " to https://.com/puppetlabs/puppetlabs-firewall.git failed! Errorcode: " + strconv.Itoa(er.returnCode) + "Error: " + er.output)
 	}
@@ -759,7 +759,7 @@ func TestEnvFullSyncIfModuleWasTemporarilyNotAvailable(t *testing.T) {
 		}
 	}
 	// fix module again
-	er = executeCommand("git --git-dir "+gitDir+" remote set-url origin https://github.com/puppetlabs/puppetlabs-firewall.git", 5, false)
+	er = executeCommand("git --git-dir "+gitDir+" remote set-url origin https://github.com/puppetlabs/puppetlabs-firewall.git", "", 5, false)
 	if er.returnCode != 0 {
 		t.Error("Rewriting the git remote url of " + gitDir + " to https://github.com/puppetlabs/puppetlabs-firewall.git failed! Errorcode: " + strconv.Itoa(er.returnCode) + "Error: " + er.output)
 	}
@@ -819,7 +819,7 @@ func TestConfigUseCacheFallbackFalse(t *testing.T) {
 	}
 
 	// change the git remote url to something that does not resolve https://.com/...
-	er := executeCommand("git --git-dir "+unresolvableGitDir+" remote set-url origin https://.com/puppetlabs/puppetlabs-firewall.git", 5, false)
+	er := executeCommand("git --git-dir "+unresolvableGitDir+" remote set-url origin https://.com/puppetlabs/puppetlabs-firewall.git", "", 5, false)
 	if er.returnCode != 0 {
 		t.Error("Rewriting the git remote url of " + unresolvableGitDir + " to https://.com/puppetlabs/puppetlabs-firewall.git failed! Errorcode: " + strconv.Itoa(er.returnCode) + "Error: " + er.output)
 	}
@@ -2000,7 +2000,7 @@ func TestFailedGit(t *testing.T) {
 	doMirrorOrUpdate(gm, gitDir, 0)
 
 	// change the git remote url to something that does not resolve https://.com/...
-	er := executeCommand("git --git-dir "+gitDir+" remote set-url origin https://.com/puppetlabs/puppetlabs-firewall.git", 5, false)
+	er := executeCommand("git --git-dir "+gitDir+" remote set-url origin https://.com/puppetlabs/puppetlabs-firewall.git", "", 5, false)
 	if er.returnCode != 0 {
 		t.Error("Rewriting the git remote url of " + gitDir + " to https://.com/puppetlabs/puppetlabs-firewall.git failed! Errorcode: " + strconv.Itoa(er.returnCode) + "Error: " + er.output)
 	}
@@ -2647,13 +2647,17 @@ func TestCloneGitModules(t *testing.T) {
 	if expectedExitCode != exitCode {
 		t.Errorf("terminated with %v, but we expected exit status %v", exitCode, expectedExitCode)
 	}
-	//fmt.Println(string(out))
+	// fmt.Println(string(out))
 
 	expectedLines := []string{
-		"DEBUG executeCommand(): Executing git clone --single-branch --branch 11.0.0 https://github.com/theforeman/puppet-puppet.git /tmp/full/full_master/modules/puppet",
-		"DEBUG executeCommand(): Executing git clone --single-branch --branch release https://github.com/puppetlabs/puppetlabs-stdlib.git /tmp/full/full_another/modules/stdlib",
-		"DEBUG executeCommand(): Executing git clone --single-branch --branch symlinks https://github.com/xorpaul/g10k-test-module.git /tmp/full/full_symlinks/modules/testmodule",
-		"DEBUG executeCommand(): Executing git clone --single-branch --branch master https://github.com/elastic/puppet-kibana.git /tmp/full/full_qa/modules/kibana",
+		"DEBUG executeCommand(): Executing git clone https://github.com/theforeman/puppet-puppet.git /tmp/full/full_master/modules/puppet",
+		"DEBUG executeCommand(): Executing git clone https://github.com/puppetlabs/puppetlabs-stdlib.git /tmp/full/full_another/modules/stdlib",
+		"DEBUG executeCommand(): Executing git clone https://github.com/xorpaul/g10k-test-module.git /tmp/full/full_symlinks/modules/testmodule",
+		"DEBUG executeCommand(): Executing git clone https://github.com/elastic/puppet-kibana.git /tmp/full/full_qa/modules/kibana",
+		"DEBUG executeCommand(): Executing git checkout 11.0.0 in cwd",
+		"DEBUG executeCommand(): Executing git checkout release in cwd",
+		"DEBUG executeCommand(): Executing git checkout symlinks in cwd",
+		"DEBUG executeCommand(): Executing git checkout master in cwd",
 	}
 
 	for _, expectedLine := range expectedLines {

git.go

@@ -112,7 +112,9 @@ func doMirrorOrUpdate(gitModule GitModule, workDir string, retryCount int) bool
 	er := ExecResult{}
 	gitCmd := "git clone --mirror " + gitModule.git + " " + workDir
 	if config.CloneGitModules && !isControlRepo && !isInModulesCacheDir {
-		gitCmd = "git clone --single-branch --branch " + gitModule.tree + " " + gitModule.git + " " + workDir
+		// only clone here, because we can't be sure if a branch is used or a commit hash or tag
+		// we switch to the defined reference later
+		gitCmd = "git clone " + gitModule.git + " " + workDir
 	}
 	if isDir(workDir) {
 		if detectGitRemoteURLChange(workDir, gitModule.git) && isControlRepo {
@@ -127,9 +129,9 @@ func doMirrorOrUpdate(gitModule GitModule, workDir string, retryCount int) bool
 		if runtime.GOOS == "darwin" {
 			sshAddCmd = "ssh-add -K "
 		}
-		er = executeCommand("ssh-agent bash -c '"+sshAddCmd+gitModule.privateKey+"; "+gitCmd+"'", config.Timeout, gitModule.ignoreUnreachable)
+		er = executeCommand("ssh-agent bash -c '"+sshAddCmd+gitModule.privateKey+"; "+gitCmd+"'", "", config.Timeout, gitModule.ignoreUnreachable)
 	} else {
-		er = executeCommand(gitCmd, config.Timeout, gitModule.ignoreUnreachable)
+		er = executeCommand(gitCmd, "", config.Timeout, gitModule.ignoreUnreachable)
 	}
 
 	if er.returnCode != 0 {
@@ -145,6 +147,17 @@ func doMirrorOrUpdate(gitModule GitModule, workDir string, retryCount int) bool
 		Warnf("WARN: git repository " + gitModule.git + " does not exist or is unreachable at this moment! Error: " + er.output)
 		return false
 	}
+
+	if config.CloneGitModules && !isControlRepo && !isInModulesCacheDir {
+		// if clone of git modules was specified, switch to the module and try to switch to the reference commit hash/tag/branch
+		gitCmd = "git checkout " + gitModule.tree
+		er = executeCommand(gitCmd, workDir, config.Timeout, gitModule.ignoreUnreachable)
+		if er.returnCode != 0 {
+			Warnf("WARN: git repository " + gitModule.git + " does not exist or is unreachable at this moment! Error: " + er.output)
+			return false
+		}
+	}
+
 	return true
 }
 
@@ -167,7 +180,7 @@ func syncToModuleDir(gitModule GitModule, srcDir string, targetDir string, corre
 
 	isControlRepo := strings.HasPrefix(srcDir, config.EnvCacheDir)
 
-	er := executeCommand(revParseCmd, config.Timeout, gitModule.ignoreUnreachable)
+	er := executeCommand(revParseCmd, "", config.Timeout, gitModule.ignoreUnreachable)
 	hashFile := filepath.Join(targetDir, ".latest_commit")
 	deployFile := filepath.Join(targetDir, ".g10k-deploy.json")
 	needToSync := true
@@ -216,7 +229,7 @@ func syncToModuleDir(gitModule GitModule, srcDir string, targetDir string, corre
 		if isControlRepo && isDir(targetDir) {
 			// then check if it contains a Puppetfile
 			gitShowCmd := "git --git-dir " + srcDir + " show " + gitModule.tree + ":Puppetfile"
-			executeResult := executeCommand(gitShowCmd, config.Timeout, true)
+			executeResult := executeCommand(gitShowCmd, "", config.Timeout, true)
 			Debugf("Executing " + gitShowCmd)
 			if executeResult.returnCode != 0 {
 				purgeWholeEnvDir = true
@@ -305,7 +318,7 @@ func syncToModuleDir(gitModule GitModule, srcDir string, targetDir string, corre
 
 func detectDefaultBranch(gitDir string) string {
 	remoteShowOriginCmd := "git ls-remote --symref " + gitDir
-	er := executeCommand(remoteShowOriginCmd, config.Timeout, false)
+	er := executeCommand(remoteShowOriginCmd, "", config.Timeout, false)
 	foundRefs := strings.Split(er.output, "\n")
 	if len(foundRefs) < 1 {
 		Fatalf("Unable to detect default branch for git repository with command git ls-remote --symref " + gitDir)
@@ -321,7 +334,7 @@ func detectDefaultBranch(gitDir string) string {
 func detectGitRemoteURLChange(d string, url string) bool {
 	gitRemoteCmd := "git --git-dir " + d + " remote -v"
 
-	er := executeCommand(gitRemoteCmd, config.Timeout, false)
+	er := executeCommand(gitRemoteCmd, "", config.Timeout, false)
 	if er.returnCode != 0 {
 		Warnf("WARN: Could not detect remote URL for git repository " + d + " trying to purge it and mirror it again")
 		return true

helper.go

@@ -170,8 +170,12 @@ func purgeDir(dir string, callingFunction string) {
 	}
 }
 
-func executeCommand(command string, timeout int, allowFail bool) ExecResult {
-	Debugf("Executing " + command)
+func executeCommand(command string, commandDir string, timeout int, allowFail bool) ExecResult {
+	if len(commandDir) > 0 {
+		Debugf("Executing " + command + " in cwd " + commandDir)
+	} else {
+		Debugf("Executing " + command)
+	}
 	parts := strings.SplitN(command, " ", 2)
 	cmd := parts[0]
 	cmdArgs := []string{}
@@ -185,7 +189,11 @@ func executeCommand(command string, timeout int, allowFail bool) ExecResult {
 	}
 
 	before := time.Now()
-	out, err := exec.Command(cmd, cmdArgs...).CombinedOutput()
+	execCommand := exec.Command(cmd, cmdArgs...)
+	if len(commandDir) > 0 {
+		execCommand.Dir = commandDir
+	}
+	out, err := execCommand.CombinedOutput()
 	duration := time.Since(before).Seconds()
 	er := ExecResult{0, string(out)}
 	if msg, ok := err.(*exec.ExitError); ok { // there is error code
@@ -233,7 +241,7 @@ func checkForAndExecutePostrunCommand() {
 		postrunCommandString = strings.Replace(postrunCommandString, "$modifiedenvs", needSyncEnvText, -1)
 		postrunCommandString = strings.Replace(postrunCommandString, "$branchparam", branchParam, -1)
 
-		er := executeCommand(postrunCommandString, config.Timeout, false)
+		er := executeCommand(postrunCommandString, "", config.Timeout, false)
 		Debugf("postrun command '" + postrunCommandString + "' terminated with exit code " + strconv.Itoa(er.returnCode))
 	}
 }

puppetfile.go

@@ -60,12 +60,12 @@ func resolvePuppetEnvironment(tags bool, outputNameTag string) {
 			if success := doMirrorOrUpdate(controlRepoGit, workDir, 0); success {
 
 				// get all branches
-				er := executeCommand("git --git-dir "+workDir+" branch", config.Timeout, false)
+				er := executeCommand("git --git-dir "+workDir+" branch", "", config.Timeout, false)
 				outputBranches := er.output
 				outputTags := ""
 
 				if tags {
-					er := executeCommand("git --git-dir "+workDir+" tag", config.Timeout, false)
+					er := executeCommand("git --git-dir "+workDir+" tag", "", config.Timeout, false)
 					outputTags = er.output
 				}
 
@@ -489,7 +489,7 @@ func skipBasedOnFilterCommand(branch string, sourceName string, sa Source, workD
 	branchFilterCommand = strings.ReplaceAll(branchFilterCommand, "$G10K_NAME", sourceName)
 	branchFilterCommand = strings.ReplaceAll(branchFilterCommand, "$GIT_DIR", workDir)
 	Debugf("executing filter_command: " + branchFilterCommand)
-	er := executeCommand(branchFilterCommand, 30, true)
+	er := executeCommand(branchFilterCommand, "", 30, true)
 	if debug {
 		fmt.Printf("filter_command %s result: %+v", branchFilterCommand, er)
 	}