Push with refspec

[vlad-anger]

Fixes #2537

git2 merged necessary PR

This PR is_blocked until new version git2 lib released
For now use git2 from master to be able to test PR.

---

Assume git config:

[branch "oblava"]
	remote = origin
	merge = refs/heads/master
[push]
  default = upstream

Prev. behavior:
push -> push oblava -> remote oblava <- new remote created

New behavior:
push -> push oblava -> remote master

---

I followed the checklist:

• I added unittests
• I ran make check without errors (see commit with tests fixes)
• I tested the overall application
• I added an appropriate item to the changelog

[naseschwarz]

Just for completeness, I'll add a reference to our discussion @vlad-anger. Thank you for the productive discussion!

In #2537 we discussed that the new behavior should be conditional on the value of push.default: Only if push.default == upstream, merge.*.branch is used as the remote branch.

[cruessler]

Just to make sure I get the context right: does any of this require changes to this part of the codebase?

gitui/asyncgit/src/sync/remotes/mod.rs

Lines 133 to 233 in 35b2529

	/// Tries to find the default repo to fetch from based on configuration.
	///
	/// > branch.<name>.remote
	/// >
	/// > When on branch `<name>`, it tells `git fetch` and `git push` which remote to fetch from or
	/// > push to. [...] If no remote is configured, or if you are not on any branch and there is more
	/// > than one remote defined in the repository, it defaults to `origin` for fetching [...].
	///
	/// [git-config-branch-name-remote]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-branchltnamegtremote
	///
	/// Falls back to `get_default_remote_in_repo`.
	pub fn get_default_remote_for_fetch(
	repo_path: &RepoPath,
	) -> Result<String> {
	let repo = repo(repo_path)?;
	get_default_remote_for_fetch_in_repo(&repo)
	}
	// TODO: Very similar to `get_default_remote_for_push_in_repo`. Can probably be refactored.
	pub(crate) fn get_default_remote_for_fetch_in_repo(
	repo: &Repository,
	) -> Result<String> {
	scope_time!("get_default_remote_for_fetch_in_repo");
	let config = repo.config()?;
	let branch = get_current_branch(repo)?;
	if let Some(branch) = branch {
	let remote_name = bytes2string(branch.name_bytes()?)?;
	let entry_name = format!("branch.{}.remote", &remote_name);
	if let Ok(entry) = config.get_entry(&entry_name) {
	return bytes2string(entry.value_bytes());
	}
	}
	get_default_remote_in_repo(repo)
	}
	/// Tries to find the default repo to push to based on configuration.
	///
	/// > remote.pushDefault
	/// >
	/// > The remote to push to by default. Overrides `branch.<name>.remote` for all branches, and is
	/// > overridden by `branch.<name>.pushRemote` for specific branches.
	///
	/// > branch.<name>.remote
	/// >
	/// > When on branch `<name>`, it tells `git fetch` and `git push` which remote to fetch from or
	/// > push to. The remote to push to may be overridden with `remote.pushDefault` (for all
	/// > branches). The remote to push to, for the current branch, may be further overridden by
	/// > `branch.<name>.pushRemote`. If no remote is configured, or if you are not on any branch and
	/// > there is more than one remote defined in the repository, it defaults to `origin` for fetching
	/// > and `remote.pushDefault` for pushing.
	///
	/// [git-config-remote-push-default]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-remotepushDefault
	/// [git-config-branch-name-remote]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-branchltnamegtremote
	///
	/// Falls back to `get_default_remote_in_repo`.
	pub fn get_default_remote_for_push(
	repo_path: &RepoPath,
	) -> Result<String> {
	let repo = repo(repo_path)?;
	get_default_remote_for_push_in_repo(&repo)
	}
	// TODO: Very similar to `get_default_remote_for_fetch_in_repo`. Can probably be refactored.
	pub(crate) fn get_default_remote_for_push_in_repo(
	repo: &Repository,
	) -> Result<String> {
	scope_time!("get_default_remote_for_push_in_repo");
	let config = repo.config()?;
	let branch = get_current_branch(repo)?;
	if let Some(branch) = branch {
	let remote_name = bytes2string(branch.name_bytes()?)?;
	let entry_name =
	format!("branch.{}.pushRemote", &remote_name);
	if let Ok(entry) = config.get_entry(&entry_name) {
	return bytes2string(entry.value_bytes());
	}
	if let Ok(entry) = config.get_entry("remote.pushDefault") {
	return bytes2string(entry.value_bytes());
	}
	let entry_name = format!("branch.{}.remote", &remote_name);
	if let Ok(entry) = config.get_entry(&entry_name) {
	return bytes2string(entry.value_bytes());
	}
	}
	get_default_remote_in_repo(repo)
	}

Or is that not the case as this PR deals with branches while the code I linked to deals with remotes?

[vlad-anger]

Or is that not the case as this PR deals with branches while the code I linked to deals with remotes?

Yeah those helpers out of scope, they tell which remote can be used,
while this PR is fixing different local/remote branches issue,

Ex. locally i have branch t1, which should be pushed to remote t2.
To achieve that, you could have push.default=upstream &
branch.t1.merge=refs/heads/t2
For now gitui will try to push t1 -> t1 always

[vlad-anger]

Tweaked logic, updated PR desc.
Waiting for upstream version release, so CI build won't fail and we
can do final checks

[naseschwarz]

Thank you very much for addressing my concerns. 👍

Here are some thoughts on the current version.

[naseschwarz]

Just a note: The behavior of the git command line client is to reject a push if the value of push.default is defined but not valid.

% git -c push.default=snerflop push origin HEAD
error: malformed value for push.default: snerflop
error: must be one of nothing, matching, simple, upstream or current
fatal: unable to parse 'push.default' from command-line config
128 %

Doing this might come in handy in case git adds new options in the future, which gitui does not support, so we don't accidentally implement incorrect push behavior.

[vlad-anger]

Good point. Now we'll fall with normal err

[naseschwarz]

Apart from the review item regarding pushing tags, I believe the push logic in this version is correctly aligned with git for push.default in {upstream, simple}. ✔️

matching and current are options that, as far as I can tell, fall back to simple in case a push source is given, which I assume is always the case in push_raw (unless delete is true, in which case only a destination is used). Thus, I'd agree that the implementation covers these correctly, too. It might be worth making this explicit so that devs keep this in mind when working on push_raw, e.g. by actually explicitly writing match on push_default_strategy and adding a short remark in the branch that does nothing.

[vlad-anger]

@naseschwarz I liked first review round, let's go for another one!