Hmm …

Posted Nov 5, 2018 19:47 UTC (Mon) by epa (subscriber, #39769)
In reply to: Hmm … by ms-tg
Parent article: Apache Subversion 1.11.0 released

I often miss the ability to group commits in git -- so one top-level commit could be unpacked into smaller ones if you choose to view it that way (and those smaller ones themselves could be nested commits, etc).

That would take care of the 'doing this commit just so git doesn't become confused' problem and it would also help with patch sets and work on a particular feature -- where the whole work is merged as a single logical commit, but by bumping up the default nesting level you can drill down to the component changes. You can emulate this a little bit with branching and merging (where there is a single commit merging the feature branch) but it's clunky by comparison.

Hmm …

Posted Nov 6, 2018 3:06 UTC (Tue) by neilbrown (subscriber, #359) [Link] (7 responses)

In what way is it "clunky by comparison"?
Would it require changes in the git engine to achieve what you want, or would it be sufficient to add some commit-group-aware interfaces?

Hmm …

Posted Nov 6, 2018 12:00 UTC (Tue) by epa (subscriber, #39769) [Link] (6 responses)

I suspect that no new low-level machinery would be needed; instead a special type of commit declares the previous N commits (with the given SHAs) to have been part of a group, with a certain log message. 'git bisect' could become aware of groups (bisecting at top-level granularity first, then optionally breaking down to the next level to find the particular sub-commit, and so on) and 'git rebase -i' would need to show them somehow, and do something reasonable when you want to rewrite history changing the contents of a group. Then web interfaces like Github would let you review the grouped commit with an ability to click through to see the sub-commits it contains.

Perhaps all this could be done with tags, I'm not sure -- but I like the idea that groups are immutable once created (just like ordinary commits), yet at the same time you can push a new group-declaring commit retrospectively making some previous commits part of your group. (I think it might be wise to forbid group spaghetti where a commit can be part of two overlapping groups, except for the straightforward case of nesting. That is something a git hook could take care of. The git machinery itself doesn't really get affected by that, however.)

Hmm …

Posted Nov 6, 2018 13:45 UTC (Tue) by smurf (subscriber, #17840) [Link]

IMHO a group marker shouldn't mark the "previous N commits" to be a group (that won't work if one of these is itself a merge to something "outside" the group) – instead, it should declare a specific ancestor to be the one that's displayed when the group is collapsed.

That special-ness could be even signalled without touching the core data structure: a normal no-follow merge commit mentions its parents in a specific order (the "null" parent is first). A "this is a group" commit would simply invert that.

Hmm … commit groups

Posted Nov 6, 2018 23:02 UTC (Tue) by neilbrown (subscriber, #359) [Link]

From a pragmatic perspective (rather than aesthetic) you seem to have identified two particular elements of functionality.

For the moment I'm thinking of a group as a merge commit plus all the commits on the right side of the merge that aren't also on the left (for some reasonable understanding of "left" and "right").

1/ you want "git bisect" to recognise these merge points and preferentially test those, rather than commits on the right. I suspect it would be fairly easy to do this for all merges. It would be expected to increase the number of steps a little, but probably just by one or two.

2/ you want to allow a new commit to mark an existing commit as part of it's group. This is comparatively huge. For git-bisect to be able to notice, you would need to create some sort of index from child commits back to their parents, at least for merged commits.
Between 4.1 and 4.19 (just a random sample) there are 20808 merge commits and 232659 non-merges. That means about 10% of commits are merges. Adding an index for the children of all merges should be manageable. If it was just "all merges with some magic string in the message" it would be even more manageable. I'm not sure if that is quite enough to allow git-bisect to do what we want.

I've occasionally thought this would be good for patches labeled "Fixes:". If they were attached to the patch they fix in a way that git-bisect could detect, then maybe it could always pull in fix - or a least warn that a fix was missing.

I think there are ideas worth exploring here (if only I had the time).

Hmm …

Posted Nov 9, 2018 19:52 UTC (Fri) by mathstuf (subscriber, #69389) [Link] (3 responses)

> 'git bisect' could become aware of groups (bisecting at top-level granularity first, then optionally breaking down to the next level to find the particular sub-commit

I think this would be handled well with a `git bisect --first-parent` option which bisects the first-parent history and then starts bisecting the history between the two commits recursively on the remaining history using the same first-parent semantics. Then your "groupings" are exactly merges.

Hmm …

Posted Nov 12, 2018 4:17 UTC (Mon) by neilbrown (subscriber, #359) [Link] (2 responses)

> I think this would be handled well with a `git bisect --first-parent` option ...

What version of git do you need? Do you give "--first-parent" when you "git bisect start" or on every "git bisect good/bad"??

"revision.c" in current git contains the line

revision.c: die(_("--first-parent is incompatible with --bisect"));

which makes me wonder....

Hmm …

Posted Nov 12, 2018 8:26 UTC (Mon) by smurf (subscriber, #17840) [Link]

> which makes me wonder....

If you only do first-parent then you may end up at the point where your only possible refinement is to start bisecting a group, which you forbade git to do – after all, you used "--first-parent".

Thus, this situation requires additional code – which hasn't been implemented yet.

Hmm …

Posted Nov 12, 2018 16:24 UTC (Mon) by mathstuf (subscriber, #69389) [Link]

I'd say you just give it to `git bisect start`. Offhand, I imagine it could basically treat all non-first-parent commits as `skip`. When it finds out that it is one of the X auto-skipped commits, it unmarks the first-parent history of those candidate commits and then restarts the bisect on those commits.

Hmm …

Posted Nov 6, 2018 6:28 UTC (Tue) by smurf (subscriber, #17840) [Link] (3 responses)

Grouping commits is reasonably simple – you create a branch for them, and then do a non-fast-forward merge to the parent.

Hmm …

Posted Nov 6, 2018 7:08 UTC (Tue) by epa (subscriber, #39769) [Link] (2 responses)

Yes, as I mentioned you can get some of the benefits by merging a branch in one commit. But that forces a nonlinear history, while allowing nested changes would still give a linear timeline for bisecting, etc.

Also it only really gives one level of nesting and is a bit clunky. Take for example the problem mentioned in an earlier post. To rename a file you may first do ‘git mv’ in one commit then change the contents in the next. Does anyone really land a separate branch just for that? What I envisage is something like ‘git group open; git commit; ...; git group close’. On closing the group you are prompted for a top-level commit message. Or you could group commits when rebasing. This would be similar to the current support for squashing commits into one, but still allowing the individual changes to be retrieved if you choose to ‘look inside’.

Hmm …

Posted Nov 6, 2018 19:44 UTC (Tue) by MrWim (subscriber, #47432) [Link] (1 responses)

Put this in git-group.sh, mark it executable and run git config --global alias.group '!/path/to/git-group.sh'. You will then be able to run git group open and git group close. It's a quick and dirty hack, with precious little error handling, but it will support nesting and should be convenient to use.

If you want to get a log with these groups collapsed run git log --first-parent.

git-group.sh follows:

#!/bin/sh -e

fail() {
    echo $@ >&2
    exit 1
}

current_branch=$(git symbolic-ref --short HEAD)

cmd=$1
shift

case "$cmd" in
open)
    git checkout -b ${current_branch}-group
    ;;

close)
    parent=$(echo $current_branch | sed 's/-group$//')
    [ "$parent" != "$current_branch" ] || fail "Not in a group!"
    git checkout "$parent"
    git merge --no-ff --no-commit --log "$current_branch" --edit
    printf '# Enter top-level commit message here.  Sub-commit messages follow:\n\n' >.group_commit_msg
    git log --first-parent --format=%B "${parent}...${current_branch}" >>.group_commit_msg
    git commit --file .group_commit_msg --edit
    rm .group_commit_msg
    git branch -D "$current_branch"
    ;;

*)
    fail "Unknown command $cmd"
    ;;
esac

Hmm …

Posted Nov 6, 2018 20:08 UTC (Tue) by epa (subscriber, #39769) [Link]

Thanks!