[PATCH] Let format-patch and rebase ignore trivial merges.

Subsystems: documentation, the rest

DORMANTno replies

5 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH] Let format-patch and rebase ignore trivial merges.

From: Bernhard R. Link <hidden>
Date: 2016-06-15 22:47:55

As git rebase and git format-patch linearize commits,
having the same change in different branches causes in the
best case duplicate patches in the produced series and in the
worst case conflicts. If there are trivial merges involved
(i.e. merges that do not change the tree), then this patch
will cause git to only look at one branch, thereby avoiding
duplicates and reducing the chance of conflicts.

There are two new options --prune-tree and --no-prune-tree
added.

--prune-tree makes rev-list without paths equivalent to
"git rev-list $options -- ." (or .. or ../.. and so on,
if you are in some subdirectory).
This is the new default for format-patch and rebase

--no-prune-tree deactivates --prune-tree.

Signed-off-by: Bernhard R. Link <redacted>
---
 Documentation/rev-list-options.txt |   11 +++++++++++
 builtin-log.c                      |    1 +
 git-rebase--interactive.sh         |    1 +
 git-rebase.sh                      |    2 +-
 revision.c                         |   11 ++++++++++-
 revision.h                         |    1 +
 6 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 1f57aed..6c5e90c 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -328,6 +328,17 @@ The following options select the commits to be shown:
 
 	Commits modifying the given <paths> are selected.
 
+--prune-tree::
+
+	No paths is equivalent to the whole tree as path.
+	That means merges with the same tree follow only one parent.
+	(Default for format-patch and rebase).
+
+--no-prune-tree::
+
+	No paths means not doing history simplification based on paths.
+	(Default for everything but format-patch and rebase).
+
 --simplify-by-decoration::
 
 	Commits that are referred by some branch or tag are selected.
diff --git a/builtin-log.c b/builtin-log.c
index 1766349..efc2f40 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -960,6 +960,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.diff = 1;
 	rev.combine_merges = 0;
 	rev.ignore_merges = 1;
+	rev.prune_tree = 1;
 	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
 
 	rev.subject_prefix = fmt_patch_subject_prefix;
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0bd3bf7..ea23d9b 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -703,6 +703,7 @@ first and then run 'git rebase --continue' again."
 		fi
 		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
 			--abbrev=7 --reverse --left-right --topo-order \
+			--prune-tree \
 			$REVISIONS | \
 			sed -n "s/^>//p" | while read shortsha1 rest
 		do
diff --git a/git-rebase.sh b/git-rebase.sh
index b121f45..2186619 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -539,7 +539,7 @@ echo "$head_name" > "$dotest/head-name"
 echo "$GIT_QUIET" > "$dotest/quiet"
 
 msgnum=0
-for cmt in `git rev-list --reverse --no-merges "$revisions"`
+for cmt in `git rev-list --reverse --no-merges --prune-tree "$revisions"`
 do
 	msgnum=$(($msgnum + 1))
 	echo "$cmt" > "$dotest/cmt.$msgnum"
diff --git a/revision.c b/revision.c
index a8a3c3a..3350af6 100644
--- a/revision.c
+++ b/revision.c
@@ -1112,6 +1112,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->dense = 1;
 	} else if (!strcmp(arg, "--sparse")) {
 		revs->dense = 0;
+	} else if (!strcmp(arg, "--prune-tree")) {
+		revs->prune_tree = 1;
+	} else if (!strcmp(arg, "--no-prune-tree")) {
+		revs->prune_tree = 0;
 	} else if (!strcmp(arg, "--show-all")) {
 		revs->show_all = 1;
 	} else if (!strcmp(arg, "--remove-empty")) {
@@ -1408,8 +1412,13 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		}
 	}
 
-	if (prune_data)
+	if (prune_data) {
 		revs->prune_data = get_pathspec(revs->prefix, prune_data);
+	} else if (revs->prune_tree) {
+		/* limit whole tree (limits trivial merges to one side) */
+		static const char *whole_tree[2] = { "", NULL };
+		revs->prune_data = whole_tree;
+	}
 
 	if (revs->def == NULL)
 		revs->def = def;
diff --git a/revision.h b/revision.h
index d368003..d007aaa 100644
--- a/revision.h
+++ b/revision.h
@@ -38,6 +38,7 @@ struct rev_info {
 	/* Traversal flags */
 	unsigned int	dense:1,
 			prune:1,
+			prune_tree:1,
 			no_merges:1,
 			merges_only:1,
 			no_walk:1,

Re: [PATCH] Let format-patch and rebase ignore trivial merges.

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:47:55

Please do not set Mail-Followup-To (and use reply-to-all to keep the Cc list).

Bernhard R. Link schrieb:
--prune-tree makes rev-list without paths equivalent to
"git rev-list $options -- ." (or .. or ../.. and so on,
if you are in some subdirectory).
This is the new default for format-patch and rebase
Why do you need a new option when you can just add "-- ." to the rev-list
invocation?

-- Hannes

Re: [PATCH] Let format-patch and rebase ignore trivial merges.

From: Bernhard R. Link <hidden>
Date: 2016-06-15 22:47:55

* Johannes Sixt [off-list ref] [091216 17:53]:
Bernhard R. Link schrieb:
quoted
--prune-tree makes rev-list without paths equivalent to
"git rev-list $options -- ." (or .. or ../.. and so on,
if you are in some subdirectory).
This is the new default for format-patch and rebase
Why do you need a new option when you can just add "-- ." to the rev-list
invocation?
I want the default for format-patch changed.

For this I think it is easiest to add a new rev_info flag, as otherwise
format-patch would need to duplicate parsing the rev_list options
and either duplicate applying revs->prune_data or changing the argv for
setup_revisions with some special casing of bare repository and non-bare
repository cases.

And if there is that rev_info flag I think it is most logical to make
it accessible from the outside.

That also allows to revert to the old format-patch behaviour,
in case someone uses format-patch not to get some appliable patches but
some differently formated log or for something else I cannot imagine.

And when there is that option, I think it is more robust to use that
in merge -m and merge -i, as "-- ." only does the right thing by chance
because both only work with a non-bare repository and have
cd_to_toplevel.

Hochachtungsvoll,
	Bernhard R. Link
-- 
Please do not CC me if git@vger.kernel.org also gets a copy.

Re: [PATCH] Let format-patch and rebase ignore trivial merges.

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:47:55

Bernhard R. Link schrieb:
* Johannes Sixt [off-list ref] [091216 17:53]:
quoted
Bernhard R. Link schrieb:
quoted
--prune-tree makes rev-list without paths equivalent to
"git rev-list $options -- ." (or .. or ../.. and so on,
if you are in some subdirectory).
This is the new default for format-patch and rebase
Why do you need a new option when you can just add "-- ." to the rev-list
invocation?
I want the default for format-patch changed.
I do not see why format-patch would have to be changed. The case that you
outline (a merge -s ours happened and you want to follow only one parent)
is rare enough and even more rarly will somebody want to apply
format-patch to such a history.

But I guess that you are actually not interested in format-patch per se,
but rather in rebase (which uses format-patch).
For this I think it is easiest to add a new rev_info flag, as otherwise
format-patch would need to duplicate parsing the rev_list options
and either duplicate applying revs->prune_data or changing the argv for
setup_revisions with some special casing of bare repository and non-bare
repository cases.
I haven't looked at the code, but wouldn't it be matter of "if we do not
have any pathspec, add '.'" *after* all options are parsed?
And when there is that option, I think it is more robust to use that
in merge -m and merge -i, as "-- ." only does the right thing by chance
because both only work with a non-bare repository and have
cd_to_toplevel.
git rev-list -- . works in a bare repository, too. If you hard-code "-- ."
in the rev-list invocations in git-rebase[--interactive], then it cannot
be said that this works "by chance" due to cd_to_toplevel.

-- Hannes

Re: [PATCH] Let format-patch and rebase ignore trivial merges.

From: Bernhard R. Link <hidden>
Date: 2016-06-15 22:47:55

* Johannes Sixt [off-list ref] [091217 12:40]:
quoted
I want the default for format-patch changed.
I do not see why format-patch would have to be changed. The case that you
outline (a merge -s ours happened and you want to follow only one parent)
is rare enough
While it is rare, the result format-patch currently produces is quite a
desaster without any need.
and even more rarly will somebody want to apply format-patch to such a history.
But I guess that you are actually not interested in format-patch per se,
but rather in rebase (which uses format-patch).
I'm looking for a nice way to store the history of a patches in a Debian package.
Currently the best way is to use quilt and store the patches in git.
Topgit is quite overkill, git directly preserving history means no way to
export sane patches. And git rebase -i means losing history of previous
states and pullability.

An way to combine those is doing many trivial merges, but that kills
rebase and format-patch. (While the patch exporting for creating the
debian source packages could change to the right directory and give the
proper arguments, needing to remember the extra argument and teaching
anyone else involved how to call it to get what to sent to upstream
is annoying).
I haven't looked at the code, but wouldn't it be matter of "if we do not
have any pathspec, add '.'" *after* all options are parsed?
That's what I would say my patch is doing.
git rev-list -- . works in a bare repository, too. If you hard-code "-- ."
in the rev-list invocations in git-rebase[--interactive], then it cannot
be said that this works "by chance" due to cd_to_toplevel.
It works in a bare repository. But it does not work when called from a
subdirectory of the working dir.

The easiest way I see to express generally

git rev-list --prune-tree $args

is

topdir=$(git rev-parse --show-cdup)
if test -z "$topdir" ; then
        topdir=.
fi
set -- $args
while test $# -gt 0 ; do
        if test "x$1" = "x--" ; then
                break
        fi
        shift
done
if test $# -gt 1 ; then
        git rev-list $args
elif test $# -eq 1 ; then
        git rev-list $args $topdir
else
        git rev-list $args -- $topdir
fi

Hochachtungsvoll,
	Bernhard R. Link
-- 
"Never contain programs so few bugs, as when no debugging tools are available!"
	Niklaus Wirth
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help