[PATCH v2 0/4] Add an option to git-format-patch to record base tree info

DORMANTno replies

Revision v2 of 3 in this series.

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

[PATCH v2 0/4] Add an option to git-format-patch to record base tree info

From: Xiaolong Ye <hidden>
Date: 2016-06-15 23:08:54

This is a re-roll of 

   http://article.gmane.org/gmane.comp.version-control.git/286873

It is almost a rewrite of v1, Changes include:

 - Make commit_patch_id() a publib helper function, so it can be available to other modules.

 - The new option "--base" will not only record the base commit id(a public well-known commit),
   but also the prerequisite patches's patch ids, then the 0day test robot could get these info,
   thus to checkout the well-known base tree-ish, apply the prerequisite patches and then the
   patches being send can be applied to correct tree to be evaluated. (0day catches every patch
   series to LKML and maintains a patch-id => commit-id database for in-flight patches.)

 - For developers' convenience, we propose a new format.base configuration to track the base commit
   automatically, if current branch the developer work on has its remote-tracking branch,
   the base commit would be the tip commit of the remote branch, if current branch is not tracked
   with any remote branch(eg. checkout by a local branch or a commit), thus its upstream could not
   be obtained, it will just record the parent commit-id of the patch series, as well as patch-id for
   reference.

   so here is the text UI:

   1) for the cases that exact base commit could be obtained(eithor from manual input --base=<base-commit-id>
	   or from upstream if base=auto is set) 

	 ** base-commit-info **
	 base-commit: ab5d01a29eb7380ceab070f0807c2939849c44bc
	 prerequisite-patch-id: 61400f965fdc0c2fbe8ad9cb5316c3efe6e05c14
	 prerequisite-patch-id: 063a398ef398647d8d839e6f9090d38ea9e0551c

   2) for cases that exact base commit is failed to obtained
	
	 ** parent-commit-info **
	 parent-commit: ab5d01a29eb7380ceab070f0807c2939849c44bc
	 parent-patch-id: caf2dae24db5b4c49a6c4134dd6d9908e898424b

Thanks for fengguang and junio's suggestions and prototype of implementation.

Thanks,
Xiaolong.

Xiaolong Ye (4):
  patch-ids: make commit_patch_id() a public helper function
  format-patch: add '--base' option to record base tree info
  format-patch: introduce --base=auto option
  format-patch: introduce format.base configuration

 Documentation/git-format-patch.txt |  20 ++++++
 builtin/log.c                      | 137 +++++++++++++++++++++++++++++++++++++
 patch-ids.c                        |   2 +-
 patch-ids.h                        |   2 +
 4 files changed, 160 insertions(+), 1 deletion(-)

-- 
2.8.0.rc4.4.ga41a987

** base-commit-info **
base-commit: 808ecd4cca75acac5e4868f15d3e647fc73698d3

[PATCH v2 2/4] format-patch: add '--base' option to record base tree info

From: Xiaolong Ye <hidden>
Date: 2016-06-15 23:08:54

Maintainers or third party testers may want to know the exact base tree
the patch series applies to. Teach git format-patch a '--base' option to
record the base tree info and append this information at the end of the
_first_ message (either the cover letter or the first patch in the series).

Signed-off-by: Xiaolong Ye <redacted>
---
 Documentation/git-format-patch.txt | 15 ++++++
 builtin/log.c                      | 98 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 6821441..a5f145e 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -265,6 +265,21 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.
   Output an all-zero hash in each patch's From header instead
   of the hash of the commit.
 
+--base=<commit>::
+	Record the base tree information to identify the whole tree
+	the patch series applies to. For example, the patch submitter
+	has a commit history of this shape:
+
+	---P---X---Y---Z---A---B---C
+
+	where "P" is the well-known public commit (e.g. one in Linus's tree),
+	"X", "Y", "Z" are prerequisite patches in flight, and "A", "B", "C"
+	are the work being sent out, the submitter would say "format-patch
+	--base=P -3 C" (or variants thereof, e.g. with "--cover" or using
+	"Z..C" instead of "-3 C" to specify the range), and the identifiers
+	for P, X, Y, Z are appended at the end of the _first_ message (either
+	the cover letter or the first patch in the series).
+
 --root::
 	Treat the revision argument as a <revision range>, even if it
 	is just a single commit (that would normally be treated as a
diff --git a/builtin/log.c b/builtin/log.c
index 0d738d6..d3804b3 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1185,6 +1185,88 @@ static int from_callback(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+struct base_tree_info {
+	struct object_id base_commit;
+	int nr_patch_id, alloc_patch_id;
+	struct object_id *patch_id;
+};
+
+static void prepare_bases(struct base_tree_info *bases,
+			  const char *base_commit,
+			  struct commit *prerequisite_head)
+{
+	struct commit *base = NULL, *commit;
+	struct rev_info revs;
+	struct diff_options diffopt;
+	struct object_id *patch_id;
+	unsigned char sha1[20];
+	int pos = 0;
+
+	if (!prerequisite_head)
+		return;
+	base = lookup_commit_reference_by_name(base_commit);
+	if (!base)
+		die(_("Unknown commit %s"), base_commit);
+	oidcpy(&bases->base_commit, &base->object.oid);
+
+	if (base == prerequisite_head)
+		return;
+
+	if (!in_merge_bases(base, prerequisite_head))
+		die(_("base commit should be the ancestor of revs you specified"));
+
+	init_revisions(&revs, NULL);
+	revs.max_parents = 1;
+
+	base->object.flags |= UNINTERESTING;
+	add_pending_object(&revs, &base->object, "base");
+	prerequisite_head->object.flags |= 0;
+	add_pending_object(&revs, &prerequisite_head->object, "prerequisite-head");
+
+	diff_setup(&diffopt);
+	DIFF_OPT_SET(&diffopt, RECURSIVE);
+	diff_setup_done(&diffopt);
+
+	if (prepare_revision_walk(&revs))
+		die(_("revision walk setup failed"));
+	/*
+	 * Traverse the commits list between base and prerequisite head,
+	 * get the patch ids and stuff them in bases structure.
+	 */
+	while ((commit = get_revision(&revs)) != NULL) {
+		if (commit_patch_id(commit, &diffopt, sha1))
+			return;
+		ALLOC_GROW(bases->patch_id, bases->nr_patch_id + 1, bases->alloc_patch_id);
+		patch_id = bases->patch_id + pos;
+		hashcpy(patch_id->hash, sha1);
+		pos++;
+		bases->nr_patch_id++;
+	}
+}
+
+static void print_bases(struct base_tree_info *bases)
+{
+	int i;
+
+	/* Only do this once, either for the cover or for the first one */
+	if (is_null_oid(&bases->base_commit))
+		return;
+
+	printf("** base-commit-info **\n");
+
+	/* Show the base commit */
+	printf("base-commit: %s\n", oid_to_hex(&bases->base_commit));
+
+	/* Show the prerequisite patches */
+	for (i = 0; i < bases->nr_patch_id; i++)
+		printf("prerequisite-patch-id: %s\n", oid_to_hex(&bases->patch_id[i]));
+
+	free(bases->patch_id);
+	bases->nr_patch_id = 0;
+	bases->alloc_patch_id = 0;
+	oidclr(&bases->base_commit);
+}
+
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
 	struct commit *commit;
@@ -1209,6 +1291,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	int reroll_count = -1;
 	char *branch_name = NULL;
 	char *from = NULL;
+	char *base_commit = NULL;
+	struct base_tree_info bases;
+
 	const struct option builtin_format_patch_options[] = {
 		{ OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
 			    N_("use [PATCH n/m] even with a single patch"),
@@ -1271,6 +1356,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			    PARSE_OPT_OPTARG, thread_callback },
 		OPT_STRING(0, "signature", &signature, N_("signature"),
 			    N_("add a signature")),
+		OPT_STRING(0, "base", &base_commit, N_("base-commit"),
+			   N_("add prerequisite tree info to the patch series")),
 		OPT_FILENAME(0, "signature-file", &signature_file,
 				N_("add a signature from a file")),
 		OPT__QUIET(&quiet, N_("don't print the patch filenames")),
@@ -1507,6 +1594,15 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		signature = strbuf_detach(&buf, NULL);
 	}
 
+	if (base_commit) {
+		struct commit *prerequisite_head = NULL;
+		if (list[nr - 1]->parents)
+			prerequisite_head = list[nr - 1]->parents->item;
+		memset(&bases, 0, sizeof(bases));
+		reset_revision_walk();
+		prepare_bases(&bases, base_commit, prerequisite_head);
+	}
+
 	if (in_reply_to || thread || cover_letter)
 		rev.ref_message_ids = xcalloc(1, sizeof(struct string_list));
 	if (in_reply_to) {
@@ -1520,6 +1616,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			gen_message_id(&rev, "cover");
 		make_cover_letter(&rev, use_stdout,
 				  origin, nr, list, branch_name, quiet);
+		print_bases(&bases);
 		total++;
 		start_number--;
 	}
@@ -1585,6 +1682,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 				       rev.mime_boundary);
 			else
 				print_signature();
+			print_bases(&bases);
 		}
 		if (!use_stdout)
 			fclose(stdout);
-- 
2.8.0.rc4.4.ga41a987

[PATCH v2 1/4] patch-ids: make commit_patch_id() a public helper function

From: Xiaolong Ye <hidden>
Date: 2016-06-15 23:08:54

Make commit_patch_id() available to other builtins.

Signed-off-by: Xiaolong Ye <redacted>
---
 patch-ids.c | 2 +-
 patch-ids.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/patch-ids.c b/patch-ids.c
index b7b3e5a..a4d0016 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -4,7 +4,7 @@
 #include "sha1-lookup.h"
 #include "patch-ids.h"
 
-static int commit_patch_id(struct commit *commit, struct diff_options *options,
+int commit_patch_id(struct commit *commit, struct diff_options *options,
 		    unsigned char *sha1)
 {
 	if (commit->parents)
diff --git a/patch-ids.h b/patch-ids.h
index c8c7ca1..eeb56b3 100644
--- a/patch-ids.h
+++ b/patch-ids.h
@@ -13,6 +13,8 @@ struct patch_ids {
 	struct patch_id_bucket *patches;
 };
 
+int commit_patch_id(struct commit *commit, struct diff_options *options,
+		    unsigned char *sha1);
 int init_patch_ids(struct patch_ids *);
 int free_patch_ids(struct patch_ids *);
 struct patch_id *add_commit_patch_id(struct commit *, struct patch_ids *);
-- 
2.8.0.rc4.4.ga41a987

[PATCH v2 3/4] format-patch: introduce --base=auto option

From: Xiaolong Ye <hidden>
Date: 2016-06-15 23:08:54

Introduce --base=auto to record the base commit info automatically, the base_commit
will be the tip commit of the upstream branch. If upstream branch cannot be determined,
it will just record the parent's commit id and patch id.

Signed-off-by: Xiaolong Ye <redacted>
---
 builtin/log.c | 70 +++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 51 insertions(+), 19 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index d3804b3..e8a3964 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1187,6 +1187,8 @@ static int from_callback(const struct option *opt, const char *arg, int unset)
 
 struct base_tree_info {
 	struct object_id base_commit;
+	struct object_id parent_commit;
+	struct object_id parent_patch_id;
 	int nr_patch_id, alloc_patch_id;
 	struct object_id *patch_id;
 };
@@ -1199,15 +1201,38 @@ static void prepare_bases(struct base_tree_info *bases,
 	struct rev_info revs;
 	struct diff_options diffopt;
 	struct object_id *patch_id;
+	struct branch *curr_branch;
+	const char *upstream;
 	unsigned char sha1[20];
 	int pos = 0;
 
 	if (!prerequisite_head)
 		return;
-	base = lookup_commit_reference_by_name(base_commit);
-	if (!base)
-		die(_("Unknown commit %s"), base_commit);
-	oidcpy(&bases->base_commit, &base->object.oid);
+
+	diff_setup(&diffopt);
+	DIFF_OPT_SET(&diffopt, RECURSIVE);
+	diff_setup_done(&diffopt);
+
+	if (!strcmp(base_commit, "auto")) {
+		curr_branch = branch_get(NULL);
+		upstream = branch_get_upstream(curr_branch, NULL);
+		if (upstream) {
+			if (get_sha1(upstream, sha1))
+				die(_("Failed to resolve '%s' as a valid ref."), upstream);
+			base = lookup_commit_or_die(sha1, "upstream base");
+			oidcpy(&bases->base_commit, &base->object.oid);
+		} else {
+			commit_patch_id(prerequisite_head, &diffopt, sha1);
+			oidcpy(&bases->parent_commit, &prerequisite_head->object.oid);
+			hashcpy(bases->parent_patch_id.hash, sha1);
+			return;
+		}
+	} else {
+		base = lookup_commit_reference_by_name(base_commit);
+		if (!base)
+			die(_("Unknown commit %s"), base_commit);
+		oidcpy(&bases->base_commit, &base->object.oid);
+	}
 
 	if (base == prerequisite_head)
 		return;
@@ -1223,10 +1248,6 @@ static void prepare_bases(struct base_tree_info *bases,
 	prerequisite_head->object.flags |= 0;
 	add_pending_object(&revs, &prerequisite_head->object, "prerequisite-head");
 
-	diff_setup(&diffopt);
-	DIFF_OPT_SET(&diffopt, RECURSIVE);
-	diff_setup_done(&diffopt);
-
 	if (prepare_revision_walk(&revs))
 		die(_("revision walk setup failed"));
 	/*
@@ -1249,22 +1270,33 @@ static void print_bases(struct base_tree_info *bases)
 	int i;
 
 	/* Only do this once, either for the cover or for the first one */
-	if (is_null_oid(&bases->base_commit))
+	if (is_null_oid(&bases->base_commit) && is_null_oid(&bases->parent_commit))
 		return;
 
-	printf("** base-commit-info **\n");
+	if (!is_null_oid(&bases->base_commit)) {
+		printf("** base-commit-info **\n");
+
+		/* Show the base commit */
+		printf("base-commit: %s\n", oid_to_hex(&bases->base_commit));
 
-	/* Show the base commit */
-	printf("base-commit: %s\n", oid_to_hex(&bases->base_commit));
+		/* Show the prerequisite patches */
+		for (i = 0; i < bases->nr_patch_id; i++)
+			printf("prerequisite-patch-id: %s\n", oid_to_hex(&bases->patch_id[i]));
 
-	/* Show the prerequisite patches */
-	for (i = 0; i < bases->nr_patch_id; i++)
-		printf("prerequisite-patch-id: %s\n", oid_to_hex(&bases->patch_id[i]));
+		free(bases->patch_id);
+		bases->nr_patch_id = 0;
+		bases->alloc_patch_id = 0;
+		oidclr(&bases->base_commit);
+	} else if (!is_null_oid(&bases->parent_commit)) {
+		printf("** parent-commit-info **\n");
 
-	free(bases->patch_id);
-	bases->nr_patch_id = 0;
-	bases->alloc_patch_id = 0;
-	oidclr(&bases->base_commit);
+		/* Show the parent commit */
+		printf("parent-commit: %s\n", oid_to_hex(&bases->parent_commit));
+		/* Show the parent patch id */
+		printf("parent-patch-id: %s\n", oid_to_hex(&bases->parent_patch_id));
+
+		oidclr(&bases->parent_commit);
+	}
 }
 
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
-- 
2.8.0.rc4.4.ga41a987

[PATCH v2 4/4] format-patch: introduce format.base configuration

From: Xiaolong Ye <hidden>
Date: 2016-06-15 23:08:54

We can set format.base=auto to record the base commit info automatically,
it is equivalent to set --base=auto in cmdline.

The format.base has lower priority than command line option, so if user
set format.base=auto and pass the command line option in the meantime,
base_commit will be the one passed to command line option.

Signed-off-by: Xiaolong Ye <redacted>
---
 Documentation/git-format-patch.txt |  5 +++++
 builtin/log.c                      | 21 ++++++++++++++-------
 2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index a5f145e..7c7a61a 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -279,6 +279,11 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.
 	"Z..C" instead of "-3 C" to specify the range), and the identifiers
 	for P, X, Y, Z are appended at the end of the _first_ message (either
 	the cover letter or the first patch in the series).
+	If 'format.base=auto' is set in configuration file, it is equivalent
+	to set '--base=auto' in cmdline, it will track the base commit
+	automatically, the base commit will be the tip commit of upstream
+	branch, if upstream branch cannot be determined, it will just record
+	the parent's commit id and patch id.
 
 --root::
 	Treat the revision argument as a <revision range>, even if it
diff --git a/builtin/log.c b/builtin/log.c
index e8a3964..fc468ce 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -699,6 +699,7 @@ static int do_signoff;
 static const char *signature = git_version_string;
 static const char *signature_file;
 static int config_cover_letter;
+static int config_base_commit;
 static const char *config_output_directory;
 
 enum {
@@ -780,6 +781,12 @@ static int git_format_config(const char *var, const char *value, void *cb)
 	}
 	if (!strcmp(var, "format.outputdirectory"))
 		return git_config_string(&config_output_directory, var, value);
+	if (!strcmp(var, "format.base")){
+		if (value && !strcasecmp(value, "auto")) {
+			config_base_commit = 1;
+			return 0;
+		}
+	}
 
 	return git_log_config(var, value, cb);
 }
@@ -1213,7 +1220,12 @@ static void prepare_bases(struct base_tree_info *bases,
 	DIFF_OPT_SET(&diffopt, RECURSIVE);
 	diff_setup_done(&diffopt);
 
-	if (!strcmp(base_commit, "auto")) {
+	if (base_commit && strcmp(base_commit, "auto")) {
+		base = lookup_commit_reference_by_name(base_commit);
+		if (!base)
+			die(_("Unknown commit %s"), base_commit);
+		oidcpy(&bases->base_commit, &base->object.oid);
+	} else if ((base_commit && !strcmp(base_commit, "auto")) || config_base_commit) {
 		curr_branch = branch_get(NULL);
 		upstream = branch_get_upstream(curr_branch, NULL);
 		if (upstream) {
@@ -1227,11 +1239,6 @@ static void prepare_bases(struct base_tree_info *bases,
 			hashcpy(bases->parent_patch_id.hash, sha1);
 			return;
 		}
-	} else {
-		base = lookup_commit_reference_by_name(base_commit);
-		if (!base)
-			die(_("Unknown commit %s"), base_commit);
-		oidcpy(&bases->base_commit, &base->object.oid);
 	}
 
 	if (base == prerequisite_head)
@@ -1626,7 +1633,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		signature = strbuf_detach(&buf, NULL);
 	}
 
-	if (base_commit) {
+	if (base_commit || config_base_commit) {
 		struct commit *prerequisite_head = NULL;
 		if (list[nr - 1]->parents)
 			prerequisite_head = list[nr - 1]->parents->item;
-- 
2.8.0.rc4.4.ga41a987
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help