git branchless sync --pull not pulling main branch

[tp-woven]

Description of the bug

When I run git branchless sync --pull, it fetches the "main" branch from the remote and pulls it into my "feature" branches, but it does not update the "main" branch itself.

For example: (irrelevant branches and logs were removed for simplicity)

❯ git sl
⋮
◆ f6217c4 1d (remote origin/main, ᐅ main) Merge pull request #XXX from XXXXXXX
❯ git branchless sync --pull
...
branchless: running command: git checkout main
Switched to branch 'main'
Your branch is behind 'origin/main' by 39 commits, and can be fast-forwarded.
  (use "git pull" to update your local branch)
...
❯ git sl
⋮
◆ f6217c4 1d (ᐅ main) Merge pull request #XXX from XXXXXXX
⋮
◇ 3aafdf4 39m (remote origin/main) Merge pull request #YYY from YYYYYYY

Expected behavior

The main branch should be "pulled" and updated to reference the latest remote ref. In the example above, I would have expected main to also be moved to 3aafdf4 (same as origin/main).

Actual behavior

The main branch is unaffected by sync and needs to be pulled manually.

Version of git-branchless

git-branchless 0.3.12

Version of git

git version 2.25.1

Version of rustc

rustc 1.60.0-nightly (bd3cb5256 2022-01-16)

Automated bug report

Software version

git-branchless 0.3.12

Operating system

Linux 5.4.0-1045-aws

Command-line

/home/devec2/.cargo/bin/git-branchless bug-report

Environment variables

SHELL=/usr/bin/zsh
EDITOR=<not set>

Git version

> git version
git version 2.25.1

Events

Show 5 events

Event ID: 179, transaction ID: 188

1. RefUpdateEvent { timestamp: 1649820518.0892587, event_tx_id: EventTransactionId(188), ref_name: "refs/heads/redacted-ref-0", old_oid: a1ac01f8528c336b64f41e54baeb3d430f6f5dc5, new_oid: 0602d383e956d811831245264a8d3c13a755ffc3, message: None }
2. RewriteEvent { timestamp: 1649820518.1151667, event_tx_id: EventTransactionId(188), old_commit_oid: a1ac01f8528c336b64f41e54baeb3d430f6f5dc5, new_commit_oid: 0602d383e956d811831245264a8d3c13a755ffc3 }
3. RefUpdateEvent { timestamp: 1649820518.1572192, event_tx_id: EventTransactionId(188), ref_name: "HEAD", old_oid: f6217c426cf6693939cee29e4e8e993201b0d901, new_oid: f6217c426cf6693939cee29e4e8e993201b0d901, message: None }

:
O 296b6d3 71d xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
: o 1893dd7 71d xxxxxxxx xxxxxxxxxxxxxx
: |
: o 18d0f41 71d (redacted-ref-1) xxxxxx xxxxxxxxxxxxx
:
O 7a9211a 64d xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
: o 76292f6 64d xxx
: |
: o 0883d75 63d (redacted-ref-2) xxx
:
O 5b7a8c0 39d (redacted-ref-3) xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
:
@ f6217c4 1d (redacted-ref-4) xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
:
O 3aafdf4 46m (remote origin/main) xxxxxxx xxxxxxxxxx xx xxxxxxx xxxxx xxxxxx xx xxxxxxxxxxx xxxxxx
|
o 0602d38 6m (redacted-ref-0) xxx

Event ID: 178, transaction ID: 186

1. CommitEvent { timestamp: 1649719008.0, event_tx_id: EventTransactionId(186), commit_oid: NonZeroOid(f6217c426cf6693939cee29e4e8e993201b0d901) }

:
O 296b6d3 71d xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
: o 1893dd7 71d xxxxxxxx xxxxxxxxxxxxxx
: |
: o 18d0f41 71d (redacted-ref-1) xxxxxx xxxxxxxxxxxxx
:
O 7a9211a 64d xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
: o 76292f6 64d xxx
: |
: o 0883d75 63d (redacted-ref-2) xxx
:
O 5b7a8c0 39d (redacted-ref-3) xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
:
@ f6217c4 1d (redacted-ref-4) xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
:
O 3aafdf4 46m (remote origin/main) xxxxxxx xxxxxxxxxx xx xxxxxxx xxxxx xxxxxx xx xxxxxxxxxxx xxxxxx
|
o 0602d38 6m (redacted-ref-0) xxx

Event ID: 177, transaction ID: 185

1. RefUpdateEvent { timestamp: 1649720107.489089, event_tx_id: EventTransactionId(185), ref_name: "HEAD", old_oid: 5b7a8c03ebbfff54b2189b1f0d65f1c33b7e574a, new_oid: 5b7a8c03ebbfff54b2189b1f0d65f1c33b7e574a, message: None }

:
O 296b6d3 71d xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
: o 1893dd7 71d xxxxxxxx xxxxxxxxxxxxxx
: |
: o 18d0f41 71d (redacted-ref-1) xxxxxx xxxxxxxxxxxxx
:
O 7a9211a 64d xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
: o 76292f6 64d xxx
: |
: o 0883d75 63d (redacted-ref-2) xxx
:
O 5b7a8c0 39d (redacted-ref-3) xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
:
@ f6217c4 1d (redacted-ref-4) xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
:
O 3aafdf4 46m (remote origin/main) xxxxxxx xxxxxxxxxx xx xxxxxxx xxxxx xxxxxx xx xxxxxxxxxxx xxxxxx
|
o 0602d38 6m (redacted-ref-0) xxx

Event ID: 176, transaction ID: 183

1. RefUpdateEvent { timestamp: 1646609831.5132928, event_tx_id: EventTransactionId(183), ref_name: "HEAD", old_oid: 5b7a8c03ebbfff54b2189b1f0d65f1c33b7e574a, new_oid: 5b7a8c03ebbfff54b2189b1f0d65f1c33b7e574a, message: None }

:
O 296b6d3 71d xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
: o 1893dd7 71d xxxxxxxx xxxxxxxxxxxxxx
: |
: o 18d0f41 71d (redacted-ref-1) xxxxxx xxxxxxxxxxxxx
:
O 7a9211a 64d xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
: o 76292f6 64d xxx
: |
: o 0883d75 63d (redacted-ref-2) xxx
:
O 5b7a8c0 39d (redacted-ref-3) xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
:
@ f6217c4 1d (redacted-ref-4) xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
:
O 3aafdf4 46m (remote origin/main) xxxxxxx xxxxxxxxxx xx xxxxxxx xxxxx xxxxxx xx xxxxxxxxxxx xxxxxx
|
o 0602d38 6m (redacted-ref-0) xxx

Event ID: 175, transaction ID: 182

1. RefUpdateEvent { timestamp: 1646609831.4847836, event_tx_id: EventTransactionId(182), ref_name: "refs/heads/redacted-ref-3", old_oid: 0000000000000000000000000000000000000000, new_oid: 5b7a8c03ebbfff54b2189b1f0d65f1c33b7e574a, message: None }

:
O 296b6d3 71d xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
: o 1893dd7 71d xxxxxxxx xxxxxxxxxxxxxx
: |
: o 18d0f41 71d (redacted-ref-1) xxxxxx xxxxxxxxxxxxx
:
O 7a9211a 64d xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
: o 76292f6 64d xxx
: |
: o 0883d75 63d (redacted-ref-2) xxx
:
O 5b7a8c0 39d (redacted-ref-3) xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
:
@ f6217c4 1d (redacted-ref-4) xxxxx xxxx xxxxxxx xxxx xxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
:
O 3aafdf4 46m (remote origin/main) xxxxxxx xxxxxxxxxx xx xxxxxxx xxxxx xxxxxx xx xxxxxxxxxxx xxxxxx
|
o 0602d38 6m (redacted-ref-0) xxx

[arxanas]

Hi @tp-woven, thanks for reporting. This is more of a specification bug than an implementation bug. git sync currently does exactly what I had originally intended it to do, but after using it for a while, I agree that it should also update the main branch.
From an implementation perspective, I suppose this is what should happen:

1. At the end of sync, check if the main branch tracks a remote branch.
2. If it does, check to see if the local main branch points to an ancestor of the remote main branch.
3. If it does, forcibly update the main branch to point to its remote commit;
4. then run check_out_commit 

   git-branchless/git-branchless-lib/src/git/run.rs

   Lines 430 to 436 in f6b4df2

   	pub fn check_out_commit(
   	effects: &Effects,
   	git_run_info: &GitRunInfo,
   	event_tx_id: Option<EventTransactionId>,
   	target: Option<impl AsRef<OsStr> + std::fmt::Debug>,
   	options: &CheckOutCommitOptions,
   	) -> eyre::Result<isize> {

   targeting the main branch.

Some edge cases:

• There is no local main branch. (I do this often; I use the remote branch origin/master as my main branch directly.)
• There is a local main branch, but it doesn't track any remote branch. Checked for above.
• There's local branches other than the main branch which track the remote main branch. I think nothing special should have to happen here.
• The local main branch has a common ancestor with the remote main branch, but has diverged from the remote main branch (i.e. you did local development directly on the main branch). We have to be sure to (attempt to) rebase the local main branch on top of the remote main branch. This is what the ancestor check is for.
• The remote main branch is an ancestor of the local main branch. This should be handled by the divergent case above.
• The local and remote main branches have no common ancestor. This is a pretty rare situation. I think the logical behavior would be to force-update the local main branch and check it out, but I expect we would be forgiven for doing nothing in this situation and instead letting the user manually recover.

If you're interested in making the above fix, let me know and I can walk you through the process in more detail.

[tp-woven]

Thanks for the detailed response! I'll try to find some time to take a look at this and get in touch if I need help (but things have been kinda busy here, so not sure when I can get around to it... 😅)

[talpr]

(Original reporter here, from a different account this time...)

I tried digging around the code a bit to see what it would look like to implement what you suggested. Going through lib::git I could not find an easy way to check whether the main branch was local, or what its relationship to the remote branch was, using the git2 wrappers. As far as I can tell, there are a couple of "courses of action" for this task:

1. Exposing the necessary git2 functionality via lib::git (things like Branch::upstream and Reference::is_remote).
2. Executing git to do the heavy lifting.
3. Maybe I'm missing something?

So before I spend too much time going down the wrong rabbit hole, I was wondering if you could elaborate a bit on what you had in mind.

Thanks! 🙇

[arxanas]

@talpr I actually exposed get_upstream_branch last night in c96a3ac, so you should be able to use that:

git-branchless/git-branchless-lib/src/git/repo.rs

Lines 1613 to 1620 in c96a3ac

	/// If this branch tracks a remote ("upstream") branch, return that branch.
	pub fn get_upstream_branch(&self) -> eyre::Result<Option<Branch<'repo>>> {
	match self.inner.upstream() {
	Ok(upstream) => Ok(Some(Branch { inner: upstream })),
	Err(err) if err.code() == git2::ErrorCode::NotFound => Ok(None),
	Err(err) => Err(err.into()),
	}
	}

See also https://github.com/arxanas/git-branchless/wiki/Coding#the-git-module. If we need to expose more functionality, then, by default, we would want to add our own wrappers over git2. When libgit2 doesn't have an appropriate implementation, we call out to Git. (For example, git status will call the fsmonitor hook to improve performance, but libgit2 won't.)

[talpr]

OK, gotcha. I'll sync with the repo again and go with that approach.

Thanks!

[banool]

For people who want this to happen, wrap git sync and run these commands after:

maincommitid=`git log remotes/origin/main --pretty=format:"%h" --no-patch -1`
git branch --force main $maincommitid

This isn't as sophisticated as the above suggestions but works well enough.