[staging] Improve "Commit & Push" push behavior

"Commit & Push" in the staging view pushed only specifying the remote.
This would push as if "push.default=current" was specified if the remote
had no push refspecs, but if it had, it might even push several branches
or totally unrelated branches.

Make sure that at most the current branch is pushed. Take into account
the branch configuration: implement a cross between push.default
"upstream" and "current": if an upstream branch is specified, use it,
otherwise fall back to pushing to a branch with the same name as the
local branch.

For the current EGit/JGit implementation, this gives a more intuitive
push behavior. It ignores the git config for "push.default", though.
(The default from the git config would be "simple", and would forbid
the push if no upstream branch was configured, or it had a name
different from the local branch name.) This is by design; using "Commit
& Push" should never use push.default=matching, even if the git config
says so.

Bug: 441031
Change-Id: I61f4fa370ed9c39753b9de52f84c986f46ca2bd1
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

3 files changed, 214 insertions, 29 deletions

diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/push/PushOperationUITest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/push/PushOperationUITest.java
new file mode 100644
index 0000000000..4779c15eaa
--- /dev/null
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/push/PushOperationUITest.java
@@ -0,0 +1,103 @@
+/*******************************************************************************
+ * Copyright (c) 2022 Thomas Wolf <thomas.wolf@paranor.ch> and others.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *******************************************************************************/
+package org.eclipse.egit.ui.internal.push;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+import java.io.File;
+
+import org.eclipse.egit.core.op.PushOperationResult;
+import org.eclipse.egit.ui.common.LocalRepositoryTestCase;
+import org.eclipse.jgit.lib.ConfigConstants;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.StoredConfig;
+import org.eclipse.jgit.transport.PushResult;
+import org.eclipse.jgit.transport.RemoteConfig;
+import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
+import org.junit.Before;
+import org.junit.Test;
+
+public class PushOperationUITest extends LocalRepositoryTestCase {
+
+	private Repository repository;
+
+	private Repository remoteRepository;
+
+	@Before
+	public void createRepositories() throws Exception {
+		File repositoryFile = createProjectAndCommitToRepository();
+		File remoteRepositoryFile = createSimpleRemoteRepository(
+				repositoryFile);
+		repository = lookupRepository(repositoryFile);
+		remoteRepository = lookupRepository(remoteRepositoryFile);
+	}
+
+	@Test
+	public void pushDefaultBranch() throws Exception {
+		touchAndSubmit("new content", "Changed");
+		ObjectId head = repository.resolve(Constants.HEAD);
+		ObjectId remote = repository
+				.resolve(Constants.R_REMOTES + Constants.DEFAULT_REMOTE_NAME
+						+ '/' + Constants.MASTER);
+		assertNotEquals(head, remote);
+		PushOperationUI op = new PushOperationUI(repository,
+				Constants.DEFAULT_REMOTE_NAME, false);
+		assertSuccess(op.execute(null));
+		assertEquals(head,
+				repository.resolve(
+						Constants.R_REMOTES + Constants.DEFAULT_REMOTE_NAME
+								+ '/' + Constants.MASTER));
+		assertEquals(head,
+				repository.resolve(Constants.R_HEADS + Constants.MASTER));
+	}
+
+	@Test
+	public void pushToSpecificRemoteBranch() throws Exception {
+		ObjectId initialHead = repository.resolve(Constants.HEAD);
+		touchAndSubmit("new content", "Changed");
+		ObjectId head = repository.resolve(Constants.HEAD);
+		// Set remote tracking branch
+		StoredConfig cfg = repository.getConfig();
+		cfg.setString(ConfigConstants.CONFIG_BRANCH_SECTION, Constants.MASTER,
+				ConfigConstants.CONFIG_KEY_REMOTE,
+				Constants.DEFAULT_REMOTE_NAME);
+		cfg.setString(ConfigConstants.CONFIG_BRANCH_SECTION, Constants.MASTER,
+				ConfigConstants.CONFIG_KEY_MERGE, Constants.R_HEADS + "foobar");
+		cfg.save();
+		RemoteConfig config = SimpleConfigurePushDialog
+				.getConfiguredRemote(repository);
+		if (config == null) {
+			config = new RemoteConfig(cfg, Constants.DEFAULT_REMOTE_NAME);
+		}
+		PushOperationUI op = new PushOperationUI(repository,
+				repository.getFullBranch(), config, false);
+		assertSuccess(op.execute(null));
+		assertEquals(head,
+				repository.resolve(Constants.R_REMOTES
+						+ Constants.DEFAULT_REMOTE_NAME + '/' + "foobar"));
+		assertEquals(head,
+				remoteRepository.resolve(Constants.R_HEADS + "foobar"));
+		assertEquals(initialHead,
+				remoteRepository.resolve(Constants.R_HEADS + Constants.MASTER));
+	}
+
+	private void assertSuccess(PushOperationResult result) {
+		result.getURIs().forEach(u -> {
+			PushResult inner = result.getPushResult(u);
+			inner.getRemoteUpdates().forEach(up -> {
+				assertEquals(Status.OK, up.getStatus());
+			});
+		});
+	}
+}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/CommitJob.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/CommitJob.java
index 62ef0b29fa..b65659d26a 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/CommitJob.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/CommitJob.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2020 Red Hat, Inc, and others.
+ * Copyright (c) 2012, 2022 Red Hat, Inc, and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
  * which accompanies this distribution, and is available at
@@ -15,6 +15,7 @@
 package org.eclipse.egit.ui.internal.commit;
 
 import java.io.IOException;
+import java.net.URISyntaxException;
 import java.text.MessageFormat;
 
 import org.eclipse.core.runtime.CoreException;
@@ -41,6 +42,8 @@ import org.eclipse.jface.wizard.Wizard;
 import org.eclipse.jgit.api.errors.AbortedByHookException;
 import org.eclipse.jgit.api.errors.CanceledException;
 import org.eclipse.jgit.api.errors.JGitInternalException;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.transport.RemoteConfig;
@@ -135,7 +138,12 @@ public class CommitJob extends Job {
 				openCommitEditor(commit);
 			}
 			if (pushMode != null) {
-				pushUpstream(commit, pushMode);
+				try {
+					pushUpstream(commit, pushMode);
+				} catch (IOException e) {
+					return Activator.createErrorStatus(
+							UIText.PushJob_unexpectedError, e);
+				}
 			}
 		}
 		return Status.OK_STATUS;
@@ -175,31 +183,39 @@ public class CommitJob extends Job {
 		});
 	}
 
-	private void pushUpstream(final RevCommit commit, final PushMode pushTo) {
-		final RemoteConfig config = SimpleConfigurePushDialog
+	private void pushUpstream(final RevCommit commit, final PushMode pushTo)
+			throws IOException {
+		RemoteConfig config = SimpleConfigurePushDialog
 				.getConfiguredRemote(repository);
 		boolean alwaysShowPushWizard = Activator.getDefault()
 				.getPreferenceStore()
 				.getBoolean(UIPreferences.ALWAYS_SHOW_PUSH_WIZARD_ON_COMMIT);
-
-		if (alwaysShowPushWizard || pushTo == PushMode.GERRIT
-				|| config == null) {
+		String currentBranch = repository.getFullBranch();
+		if (ObjectId.isId(currentBranch)) {
+			currentBranch = null;
+		}
+		if (currentBranch != null && config == null) {
+			try {
+				config = new RemoteConfig(repository.getConfig(),
+						Constants.DEFAULT_REMOTE_NAME);
+			} catch (URISyntaxException e) {
+				throw new IOException(e.getLocalizedMessage(), e);
+			}
+		}
+		if (alwaysShowPushWizard || pushTo == PushMode.GERRIT || config == null
+				|| currentBranch == null) {
 			final Display display = PlatformUI.getWorkbench().getDisplay();
-			display.asyncExec(new Runnable() {
-
-				@Override
-				public void run() {
-					Wizard pushWizard = getPushWizard(commit, pushTo);
-					if (pushWizard != null) {
-						PushWizardDialog dialog = new PushWizardDialog(
-								display.getActiveShell(), pushWizard);
-						dialog.open();
-					}
+			display.asyncExec(() -> {
+				Wizard pushWizard = getPushWizard(commit, pushTo);
+				if (pushWizard != null) {
+					PushWizardDialog dialog = new PushWizardDialog(
+							display.getActiveShell(), pushWizard);
+					dialog.open();
 				}
 			});
 		} else {
-			PushOperationUI op = new PushOperationUI(repository,
-					config.getName(), false);
+			PushOperationUI op = new PushOperationUI(repository, currentBranch,
+					config, false);
 			op.start();
 		}
 	}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushOperationUI.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushOperationUI.java
index 41c39ef79a..5b8217a561 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushOperationUI.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushOperationUI.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011, 2017 SAP AG and others.
+ * Copyright (c) 2011, 2022 SAP AG and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
  * which accompanies this distribution, and is available at
@@ -15,6 +15,7 @@ package org.eclipse.egit.ui.internal.push;
 
 import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
+import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -31,6 +32,9 @@ import org.eclipse.egit.ui.Activator;
 import org.eclipse.egit.ui.internal.UIText;
 import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.errors.NotSupportedException;
+import org.eclipse.jgit.lib.BranchConfig;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.CredentialsProvider;
 import org.eclipse.jgit.transport.RefSpec;
@@ -38,7 +42,6 @@ import org.eclipse.jgit.transport.RemoteConfig;
 import org.eclipse.jgit.transport.RemoteRefUpdate;
 import org.eclipse.jgit.transport.Transport;
 import org.eclipse.jgit.transport.URIish;
-import org.eclipse.osgi.util.NLS;
 
 /**
  * UI Wrapper for {@link PushOperation}
@@ -60,6 +63,8 @@ public class PushOperationUI {
 
 	private final String remoteName;
 
+	private final String branchName;
+
 	private PushOperationResult expectedResult;
 
 	private boolean showConfigureButton = true;
@@ -67,10 +72,14 @@ public class PushOperationUI {
 	private @NonNull PushMode pushMode = PushMode.UPSTREAM;
 
 	/**
+	 * Push to the given remote using the git configuration.
+	 *
 	 * @param repository
+	 *            to push from
 	 * @param remoteName
+	 *            {@link RemoteConfig} specifying where to push to
 	 * @param dryRun
-	 *
+	 *            whether the push should be a dry run
 	 */
 	public PushOperationUI(Repository repository, String remoteName,
 			boolean dryRun) {
@@ -78,33 +87,74 @@ public class PushOperationUI {
 		this.spec = null;
 		this.config = null;
 		this.remoteName = remoteName;
+		this.branchName = null;
 		this.dryRun = dryRun;
-		destinationString = NLS.bind("{0} - {1}", repository.getDirectory() //$NON-NLS-1$
+		destinationString = MessageFormat.format("{0} - {1}", //$NON-NLS-1$
+				repository.getDirectory()
 				.getParentFile().getName(), remoteName);
 	}
 
-
 	/**
+	 * Push all branches that match a configured push refspec. (Corresponds to
+	 * PushDefault.MATCHING.)
+	 *
 	 * @param repository
+	 *            to push from
 	 * @param config
+	 *            {@link RemoteConfig} specifying where to push to
 	 * @param dryRun
+	 *            whether the push should be a dry run
 	 *
 	 */
 	public PushOperationUI(Repository repository, RemoteConfig config,
 			boolean dryRun) {
+		this(repository, null, config, dryRun);
+	}
+
+	/**
+	 * Push a specific branch. If a tracking branch is configured, pushes to
+	 * that, otherwise uses the given branch name also as upstream branch name.
+	 * (This is a cross between PushDefault.UPSTREAM and PushDefault.CURRENT.)
+	 *
+	 * @param repository
+	 *            to push from
+	 * @param branchName
+	 *            full name of the branch to push; may be {@code null} to push
+	 *            all branches matching a configured push refspec
+	 * @param config
+	 *            {@link RemoteConfig} specifying where to push to
+	 * @param dryRun
+	 *            whether the push should be a dry run
+	 */
+	public PushOperationUI(Repository repository, String branchName,
+			RemoteConfig config, boolean dryRun) {
 		this.repository = repository;
 		this.spec = null;
 		this.config = config;
 		this.remoteName = null;
+		this.branchName = branchName;
 		this.dryRun = dryRun;
-		destinationString = NLS.bind("{0} - {1}", repository.getDirectory() //$NON-NLS-1$
-				.getParentFile().getName(), config.getName());
+		if (branchName != null) {
+			destinationString = MessageFormat.format("{0} {1} - {2}", //$NON-NLS-1$
+					repository.getDirectory().getParentFile().getName(),
+					branchName, config.getName());
+		} else {
+			destinationString = MessageFormat.format("{0} - {1}", //$NON-NLS-1$
+					repository.getDirectory().getParentFile().getName(),
+					config.getName());
+		}
 	}
 
 	/**
+	 * Push exactly the branches to remotes as specified by the
+	 * {@link PushOperationSpecification}.
+	 *
 	 * @param repository
+	 *            to push from
 	 * @param spec
+	 *            {@link PushOperationSpecification} defining what to push where
 	 * @param dryRun
+	 *            whether the push should be a dry run
 	 */
 	public PushOperationUI(Repository repository,
 			PushOperationSpecification spec, boolean dryRun) {
@@ -112,12 +162,13 @@ public class PushOperationUI {
 		this.spec = spec;
 		this.config = null;
 		this.remoteName = null;
+		this.branchName = null;
 		this.dryRun = dryRun;
 		if (spec.getURIsNumber() == 1)
 			destinationString = spec.getURIs().iterator().next()
 					.toPrivateString();
 		else
-			destinationString = NLS.bind(
+			destinationString = MessageFormat.format(
 					UIText.PushOperationUI_MultiRepositoriesDestinationString,
 					Integer.valueOf(spec.getURIsNumber()));
 	}
@@ -195,7 +246,21 @@ public class PushOperationUI {
 				urisToPush.add(config.getURIs().get(0));
 
 			List<RefSpec> pushRefSpecs = new ArrayList<>();
-			pushRefSpecs.addAll(config.getPushRefSpecs());
+			if (branchName == null) {
+				pushRefSpecs.addAll(config.getPushRefSpecs());
+			} else {
+				Config repoConfig = repository.getConfig();
+				String remoteBranchName = branchName;
+				BranchConfig branchConfig = new BranchConfig(repoConfig,
+						Repository.shortenRefName(branchName));
+				String trackingBranchName = branchConfig.getMerge();
+				if (!branchConfig.isRemoteLocal() && trackingBranchName != null
+						&& trackingBranchName.startsWith(Constants.R_HEADS)) {
+					remoteBranchName = trackingBranchName;
+				}
+				pushRefSpecs
+						.add(new RefSpec(branchName + ':' + remoteBranchName));
+			}
 
 			for (URIish uri : urisToPush) {
 				try {
@@ -239,7 +304,8 @@ public class PushOperationUI {
 			op.setCredentialsProvider(new EGitCredentialsProvider());
 		}
 		Job job = new PushJob(
-				NLS.bind(UIText.PushOperationUI_PushJobName, destinationString),
+				MessageFormat.format(UIText.PushOperationUI_PushJobName,
+						destinationString),
 				repo, op, expectedResult, destinationString,
 				showConfigureButton, pushMode);
 		job.setUser(true);