Fix staging/approval

As the update bot's account does not have approver rights, do not use it to attempt
to review and stage the pushed changes, that doesn't work.

This is also how coin's propose-supermodule-update command behaves.

Change-Id: I126429c2019165159e825f1e0321b5c3bdb12fa0
Reviewed-by: Aapo Keskimolo <aapo.keskimolo@qt.io>

4 files changed, 22 insertions, 15 deletions

diff --git a/src/qtmoduleupdater/gerrit.go b/src/qtmoduleupdater/gerrit.go
index 4cebe951..897ebfa7 100644
--- a/src/qtmoduleupdater/gerrit.go
+++ b/src/qtmoduleupdater/gerrit.go
@@ -72,11 +72,6 @@ type GerritChangeOrStats struct {
 }
 
 func gerritSSHCommand(gerritURL url.URL, arguments ...string) (*exec.Cmd, error) {
-	user := os.Getenv("GIT_SSH_USER")
-	if user != "" {
-		gerritURL.User = url.User(user)
-	}
-
 	host, port, err := net.SplitHostPort(gerritURL.Host)
 	if err != nil {
 		return nil, fmt.Errorf("Error splitting host and port from gerrit URL: %s", err)
@@ -102,7 +97,7 @@ func gerritSSHCommand(gerritURL url.URL, arguments ...string) (*exec.Cmd, error)
 }
 
 func getGerritChangeStatus(project string, branch string, changeID string) (status string, err error) {
-	gerritURL, err := RepoURL(project)
+	gerritURL, err := RepoPushURL(project)
 	if err != nil {
 		return "", fmt.Errorf("Error parsing gerrit URL: %s", err)
 	}
@@ -149,7 +144,7 @@ func getGerritChangeStatus(project string, branch string, changeID string) (stat
 }
 
 func getExistingChange(project string, branch string) (gerritChangeID string, changeNumber int, patchSetNr int, err error) {
-	gerritURL, err := RepoURL(project)
+	gerritURL, err := RepoPushURL(project)
 	if err != nil {
 		return "", 0, 0, fmt.Errorf("Error parsing gerrit URL: %s", err)
 	}
@@ -226,14 +221,13 @@ func pushChange(repoPath string, branch string, commitID OID, summary string, pu
 	return repo.Push(pushURL, commitID, "refs/for/"+branch)
 }
 
-func reviewAndStageChange(repoPath string, branch string, commitID OID, summary string, pushUserName string) error {
-	pushURL, err := RepoURL(repoPath)
+func reviewAndStageChange(repoPath string, branch string, commitID OID, summary string) error {
+	pushURL, err := RepoPushURL(repoPath)
 	if err != nil {
 		return err
 	}
-	if pushUserName != "" {
-		pushURL.User = url.User(pushUserName)
-	}
+	// Always review/approve as current user as the bot does not have approval rights.
+	pushURL.User = nil
 
 	reviewArgs := []string{"gerrit", "review", string(commitID)}
 
diff --git a/src/qtmoduleupdater/moduleupdatebatch.go b/src/qtmoduleupdater/moduleupdatebatch.go
index 11b73f43..85565ef3 100644
--- a/src/qtmoduleupdater/moduleupdatebatch.go
+++ b/src/qtmoduleupdater/moduleupdatebatch.go
@@ -71,7 +71,7 @@ func (batch *ModuleUpdateBatch) scheduleUpdates(pushUserName string, manualStage
 			}
 
 			if !manualStage {
-				if err = reviewAndStageChange(moduleToUpdate.RepoPath, moduleToUpdate.Branch, update.commitID, update.summary, pushUserName); err != nil {
+				if err = reviewAndStageChange(moduleToUpdate.RepoPath, moduleToUpdate.Branch, update.commitID, update.summary); err != nil {
 					return fmt.Errorf("error pushing change upate: %s", err)
 				}
 			}
diff --git a/src/qtmoduleupdater/qt5.go b/src/qtmoduleupdater/qt5.go
index 1567878d..19c7cafe 100644
--- a/src/qtmoduleupdater/qt5.go
+++ b/src/qtmoduleupdater/qt5.go
@@ -254,5 +254,5 @@ func prepareQt5Update(product string, branch string, updatedModules map[string]*
 		return nil
 	}
 
-	return reviewAndStageChange(product, branch, commitOid, "Updating all submodules with a new consistent set", pushUserName)
+	return reviewAndStageChange(product, branch, commitOid, "Updating all submodules with a new consistent set")
 }
diff --git a/src/qtmoduleupdater/repo.go b/src/qtmoduleupdater/repo.go
index 39589520..6a8991eb 100644
--- a/src/qtmoduleupdater/repo.go
+++ b/src/qtmoduleupdater/repo.go
@@ -73,7 +73,7 @@ type Repository string
 // OID is a git object identifier, in the form of a SHA1 check-sum.
 type OID string
 
-// RepoURL returns a clone/push/fetch URL for the given project.
+// RepoURL returns a clone/fetch URL for the given project.
 func RepoURL(project string) (*url.URL, error) {
 	gerritConfig := struct {
 		URL  string
@@ -89,6 +89,19 @@ func RepoURL(project string) (*url.URL, error) {
 	return repo, nil
 }
 
+// RepoPushURL returns a URL to use for pusing changes to the project.
+func RepoPushURL(project string) (*url.URL, error) {
+	pushUrl, err := RepoURL(project)
+	if err != nil {
+		return nil, err
+	}
+	user := os.Getenv("GIT_SSH_USER")
+	if user != "" {
+		pushUrl.User = url.User(user)
+	}
+	return pushUrl, nil
+}
+
 // OpenRepository is used to create a new repository wrapper for the specified project.
 // If the repository doesn't exist yet, it will be cloned.
 func OpenRepository(project string) (Repository, error) {