smartlog, hide and branches

[claytonrcarter]

I'm getting a lot of usage and value out of this, even those it's called branchless and I'm feeling like my workflow is branchmore much of the time. 😄 So, I may be suggesting things that are out of scope for this tool; please forgive me if I am.

Here's a couple of things that would have been handy recently:

1. smartlog flag to only show commit trees that have branches on them. For example:

❯ git sl
...
◇ c1c06456 6d Merge branch 'work/942-checkout-bug-fixes' into 'main'
┣━┓
┃ ◯ 8f774fd0 2d add `misc/` dir and scripts
┃
◇ dcab5f1a 1d Merge branch 'emily/sunflower-mix-update' into 'main'
┣━┓
⋮ ◯ cd77797a 5h (remove-users-table) landscape: remove users table
⋮

❯ git sl --only-branches
...
⋮
◇ dcab5f1a 1d Merge branch 'emily/sunflower-mix-update' into 'main'
┣━┓
⋮ ◯ cd77797a 5h (remove-users-table) landscape: remove users table
⋮

2. Config option to automatically hide commits when an unmerged branch is manually deleted (eg git branch -D my-branch) because manually deleted an unmerged branch is likely to mean that you want to abandon/discard those commits as well. This happens for me b/c we do a log of GitLab MRs w/ lots of commits that end up being squashed @ merge, but those commits still hang around, haunting my smartlog.

❯ git sl
⋮
◯ 9337b169 35d edit file-b
┣━┓
┃ ◯ 5d86daa1 35d (branch-c) add file-c
┃
● 079ca218 2s (branch-b) more file-b

❯ git co branch-c && git br -D branch-b
Switched to branch 'branch-c'
branchless: processing checkout

branchless: processing 1 update: branch branch-b
branchless: processing 1 update: branch branch-b
Deleted branch branch-b (was 079ca21).

❯ git sl
⋮
◇ 7f1da559 35d (main) add file-a
┃
◯ 64f4794b 35d add file-b
┃
◯ 9337b169 35d edit file-b
┣━┓
┃ ● 5d86daa1 35d (branch-c) add file-c
┃
◯ 079ca218 20s more file-b

# but I don't want to see 079ca218 anymore!

3. Config option to prevent me from hiding commits/trees that include branches. This comes from me cleaning up my smartlog for # 2 above, and copy-pasting the wrong commits into git hide -r. Here's a possible example/suggestion:

❯ git sl
⋮
◇ 7f1da559 35d (main) add file-a
┃
◯ 64f4794b 35d add file-b
┃
◯ 9337b169 35d edit file-b
┣━┓
┃ ● 5d86daa1 35d (branch-c) add file-c
┃
◯ 079ca218 22m more file-b

❯ git hide -r 5d86daa1 079ca218
Hid commit: 5d86daa1 add file-c
To unhide this commit, run: git unhide 5d86daa1
Hid commit: 079ca218 more file-b
To unhide this commit, run: git unhide 079ca218

❯ git sl
⋮
◇ 7f1da559 35d (main) add file-a
┃
◯ 64f4794b 35d add file-b
┃
◯ 9337b169 35d edit file-b
┃
⦻ 5d86daa1 35d (manually hidden) (branch-c) add file-c

# So 5d86daa1 isn't actually hidden b/c it's got a branch on it
# Let's try again... 

❯ git unhide -r 5d86daa1 079ca218
Unhid commit: 5d86daa1 add file-c
To hide this commit, run: git hide 5d86daa1
Unhid commit: 079ca218 more file-b
To hide this commit, run: git hide 079ca218

❯ git sl
⋮
◇ 7f1da559 35d (main) add file-a
┃
◯ 64f4794b 35d add file-b
┃
◯ 9337b169 35d edit file-b
┣━┓
┃ ● 5d86daa1 35d (branch-c) add file-c
┃
◯ 079ca218 23m more file-b


# Now lets turn on the speculative "warn me when hiding a branch" flag...

❯ git config branchless.hide.warnOnBranch true
❯ git hide -r 5d86daa1 079ca218
Hid commit: 079ca218 more file-b
To unhide this commit, run: git unhide 079ca218
warning: refusing to hide commit 5d86daa1 because it includes branch branch-c

Do any of these seem like they make sense in this tool?

[arxanas]

I'm getting a lot of usage and value out of this, even those it's called branchless and I'm feeling like my workflow is branchmore much of the time. 😄 So, I may be suggesting things that are out of scope for this tool; please forgive me if I am.

This is all on-topic. It's unfortunately turned out that the name "git-branchless" is a bad one, since so many people preface their issues with "I'm using a branch-based workflow but..." when in fact git-branchless fully supports branch-based workflows. I'm open to other names, but unfortunately, I haven't come up with a good one, considering there's no obvious unifying theme between "branchlessness"/"in-memory rebases"/"undo"/etc.

smartlog flag to only show commit trees that have branches on them. For example:

In the long-term, we'd probably want to support revsets (#175) and be able to write something like git sl 'branches()' to accomplish the same behavior. But if you want to try making an --only-branches flag for now, it actually might not be too difficult. You would need to update this function:

git-branchless/src/commands/smartlog.rs

Lines 219 to 226 in 496ad07

	pub fn make_smartlog_graph<'repo>(
	effects: &Effects,
	repo: &'repo Repo,
	dag: &Dag,
	event_replayer: &EventReplayer,
	event_cursor: EventCursor,
	remove_commits: bool,
	) -> eyre::Result<SmartlogGraph<'repo>> {

It would have to support walking only from commits with branches pointing to them, instead of all public commits. That information is already in the passed-in dag variable:

git-branchless/src/core/dag.rs

Lines 78 to 86 in 3c88340

	/// A set containing the commit which `HEAD` points to. If `HEAD` is unborn,
	/// this is an empty set.
	pub head_commit: CommitSet,
	/// A set containing the commit that the main branch currently points to.
	pub main_branch_commit: CommitSet,
	/// A set containing all commits currently pointed to by local branches.
	pub branch_commits: CommitSet,

So you would just need to make an alternative to this function which only queries branch heads, and use that:

git-branchless/src/core/dag.rs

Lines 361 to 365 in 3c88340

	pub fn query_active_heads(
	&self,
	public_commits: &CommitSet,
	observed_commits: &CommitSet,
	) -> eyre::Result<CommitSet> {

Config option to automatically hide commits when an unmerged branch is manually deleted

This also hits me when I just want to delete a branch. It might instead be best to make it so that git hide also deletes any pointed-to branches. Implementing revsets would also let us write something like git hide stack(branch-b). For now, maybe it would be better to add a -b/--branch flag indicating that 1. branches should be deleted and 2. ancestors should be hidden (rather than descendants as with -r).

It would be nice to do something automatically when a branch is deleted, but even though we can observe the branch disappearing, it's not really possible to tell what the user (or tool!) intent might have been. There's also an unfortunate issue in most recent versions of Git where the reference-transaction hook spuriously reports branches as being deleted and then recreated, which might make it difficult.

Config option to prevent me from hiding commits/trees that include branches.

Rather than prevent the hide operation, I think it would be better to surface a warning with remediation steps:

❯ git hide -r 5d86daa1 079ca218
Hid commit: 079ca218 more file-b
To unhide this commit, run: git unhide 079ca218
warning: The commit at branch `branch-c` will still appear in the smartlog.
warning: - To hide it, delete the branch with `git branch -d branch-c`.
warning: - To undo this operation, run `git undo`.

We should remove the help text from git hide/git unhide and just suggest git undo.

(If it wasn't clear, you can actually run git branch -d and delete the branch yourself to hide the commits after the fact.)

[arxanas]

It occurs to me that more intuitive behavior might just be to make git hide, when receiving a branch as an argument, delete that branch and hide any ancestor commits. You could still hide individual commits by hashes. It's not clear how that behavior would interact with revsets, though.

[claytonrcarter]

I really like the idea of having git hide <branch> hide the head all ancestors; but I'm lukewarm to the having it delete the branch, too, at least without an add'l --delete/-d flag could be used to confirm the action. I get that hiding the commits w/o deleting the branch is a little counterintuitive, but deleting the branch automatically feels like a footgun to me, like it might a little too easy to paste the wrong thing into the command line, etc.

(If it wasn't clear, you can actually run git branch -d and delete the branch yourself to hide the commits after the fact.)

Yes, of course, but to clarify, I didn't want to hide or delete those branches. I had copy-pasted the wrong commits into git hide and I was/am looking for an add'l safety feature to prevent me from being a bonehead. 😄

I think that a warning message would probably be fine, but git hide can be a little noisy when hiding lots of commits, so I would be concerned that the warning could be easy to miss. For my workflow, if I run git hide -r and the hidden commits include a branch tip, that will be a mistake 100% of the time.

Finally, I don't quite get what revsets are (I think they're some cool way to query commits?) but from what I've read implementing that would be well above my skill level. If I get a chance, I may take you up on trying to add --only-branches to git sl, though. Your thorough guidance makes it sound not too scary. 😄 Thank you!

[arxanas]

For my workflow, if I run git hide -r and the hidden commits include a branch tip, that will be a mistake 100% of the time.

I see. I think it would be reasonable to show a summary of branches that were abandoned by the git hide operation. If you want to take a go at implementing a warning, it should be pretty similar to how the smartlog code works. At the end of hide:

git-branchless/src/commands/hide.rs

Line 20 in d3c818c

pub fn hide(effects: &Effects, hashes: Vec<String>, recursive: bool) -> eyre::Result<isize> {

you would just need to print out any values in the map reference_snapshot.branch_oid_to_names whose keys appear in commits.

This could also enable us to make a change later where we do automatically clean up the branches. In either case, if you hid those commits mistakenly, you would want to run git undo, so the remediation steps would be the same.

[arxanas]

Finally, I don't quite get what revsets are (I think they're some cool way to query commits?) but from what I've read implementing that would be well above my skill level.

That's correct. The effect of implementing revsets would be that we could skip adding a lot of configuration flags and instead let the user express the same queries via revset expression. But yeah, the change is a lot more involved, so I'm happy to add sensible one-off configuration flags in the meantime.

If I get a chance, I may take you up on trying to add --only-branches to git sl, though.

Also feel free to join the Discord channel at https://discord.gg/caYQBJ82A4 where you can ask any questions (both workflow-related and development-related) in a more interactive manner 😃

[rouge8]

Should git smartlog 'branches()' work today? It's mentioned on https://github.com/arxanas/git-branchless/wiki/Command:-git-smartlog#custom-queries, but it doesn't seem to work -- it still shows commits without a branch for me.

Nevermind, it seems to work now...

[arxanas]

This happens for me b/c we do a log of GitLab MRs w/ lots of commits that end up being squashed @ merge, but those commits still hang around, haunting my smartlog.

One solution is to attach change IDs to the commit messages, so that it has a field like

Branchless-Change-Id: abcdef

in the commit message somewhere. Assuming that the commit messages for squash-merges are all combined, then it should be possible to use that for commit deduplication. Would that work for you?

I think there are other solutions that should be explored as well to detect squash merges automatically. Since most users merge stuff at the bottom of a stack, you could calculate O(n) extra commits starting from the first commit and spanning to the second, to the third, etc., and see if any of those would duplicate an upstream commit, which might still scale reasonably well in practice.

[claytonrcarter]

attach change IDs to the commit messages

My first reaction is that this would probably work OK for individuals, but it might not scale well in a team setting. Everyone would have to be on the same page about commit formatting (or have the tools installed to do so, etc) and the code hosting service would also have to reliably preserve the commit messages when commits are squashed or manipulated. It feels fragile, like there are too many moving parts involved for it to be reliable.

other solutions that should be explored as well to detect squash merges automatically

I wonder if there are some heuristics that could be applied to make use of existing info or behaviors, though. For example:

• I'm assuming that this is probably only a concern for commits added from a remote, and even then it would only affect local branches that are tracking a branch on that remote (per perhaps any remote)
• GitLab always includes actual merge commits, even for squashed merges, which almost always include the name of the squashed/merged branch, similar to the default message used when merging locally (GitHub does not seem to do this, as far as I can tell)
• as you're suggesting, there are only a finite number of local branches to consider, and it's most common for folks to squash/merge from the bottom of a stack, so that would further limit what needs to be considered.

Even then, I'm not sure that automatically taking action is the right outcome; I think I would lean more towards to a help message that might be different based on how certain we are about the match. For example:

• High confidence:
  • Squash detected: merge commit abc123 squashed branch name. To remove name, run 'git hide --delete <name>'
• Medium confidence:
  • Possible squash detected: remote commit def456 may squash local branch: name
• Low/no confidence
  • No squashed detected.

I'm probably way off base, though, given my flimsy knowledge of how this project works under the hood. There are just some thoughts that struck me while I was running errands today. If they're helpful, great! If not, sorry for the noise. Thanks again!

[arxanas]

Everyone would have to be on the same page about commit formatting

Assuming that you're primarily trying to clean up your own feature branches, then you would be the only person on the team who would need to include metadata for your own commits. It wouldn't matter if others didn't, because you wouldn't be trying to clean up their branches, usually.

the code hosting service would also have to reliably preserve the commit messages when commits are squashed or manipulated

I suspect that this might be more reliable than you think. In particular, Git already encourages including metadata such as the Signed-off-by: tag (by running git commit --signoff), and services like Github know about the existence of this style of tag, such as Co-authored-by:. Adding one more type of tag shouldn't be a problem.

GitLab always includes actual merge commits, even for squashed merges

Interesting. This seems more sensible than how Github does it. In that case, git move should detect that the commit has been applied upstream in one of the parents and skip the commit anyways. If this doesn't happen, post a repro case and I can look into it.

@epage how does git-stack handle squash merges, if at all? I think I vaguely remember that it used some heuristics based on the commit message, but I didn't find that from a quick search.

[rouge8]

I think there are other solutions that should be explored as well to detect squash merges automatically

git-delete-merged-branches uses git cherry to detect squash merges and delete the merged branch.

[arxanas]

@rouge8 thanks for the reference. Currently, git-branchless uses an equivalent mechanism to git cherry to detect single commits, but it doesn't use branches as a hint to suggest ranges of commits to squash. I'll take a look at the approach they use when I have some time.