Message-ID: <xmqqtwkyt0tj.fsf@gitster.mtv.corp.google.com>
Date:	Wed, 24 Feb 2016 10:34:00 -0800
From:	Junio C Hamano <gitster@...ox.com>
To:	Fengguang Wu <fengguang.wu@...el.com>
Cc:	Dan Carpenter <dan.carpenter@...cle.com>,
	Xiaolong Ye <xiaolong.ye@...el.com>, git@...r.kernel.org,
	ying.huang@...el.com, philip.li@...el.com, julie.du@...el.com,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Christoph Hellwig <hch@....de>,
	"H. Peter Anvin" <hpa@...or.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC/PATCH 1/1] format-patch: add an option to record base tree info

Fengguang Wu <fengguang.wu@...el.com> writes:

>> This starts to sound more like something you would want to write in
>> the cover letter, or the trailer block next to Signed-off-by: at the
>> end of the first patch in the series.
>
> Yes, that's roughly what the current patch does, except in the latter
> case we add new info after diffstat.
>
>> Or even after the mail
>> signature at the very end of the message (incidentally that would
>> probably minimize the damage to the Git codebase needed for this
>> addition--you should be able to do this without touching anything
>> other than builtin/log.c).
>
> That's an interesting place. It looks worth trying. 

Here is an outline of such a structure.  The idea is for a history
of this shape, 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 for testing,

	 ---P---X---Y---Z---A---B---C

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).

You are welcome to steal this as a skeleton and take authorship, as
I do not plan to spend too much more time on actually implementing
prepare_bases() function myself unless I run out of things to do and
get bored, but I think there are already sufficient helper functions
to do this (you may want to make commit_patch_id() in patch-ids.c a
public helper function).

 builtin/log.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/builtin/log.c b/builtin/log.c
index 069bd3a..8f6cc17 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1182,6 +1182,59 @@ 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 **list)
+{
+	/*
+	 * If you are given --base=$commit command line argument, that
+	 * names commit 'P' when you are formatting A, B and C in this
+	 * history (i.e. "format-patch --base=P Z..C"):
+	 *
+	 * ---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 whose patch-ids are well-known, and "A", "B", "C"
+	 * are the work being sent out for testing, you would compute
+	 * base_commit=P, patch_ids[] = map(commit_patch_id, [X, Y,
+	 * Z]) here and stuff them in bases structure.
+	 *
+	 * Be sure to check error (e.g. invalid base_commit) and die()
+	 * here as necessary.
+	 */
+	
+}
+
+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("base-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;
@@ -1205,6 +1258,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"),
@@ -1265,6 +1321,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")),
@@ -1496,6 +1554,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		signature = strbuf_detach(&buf, NULL);
 	}
 
+	if (base_commit) {
+		memset(&bases, 0, sizeof(bases));
+		prepare_bases(&bases, base_commit, list);
+	}
+
 	if (in_reply_to || thread || cover_letter)
 		rev.ref_message_ids = xcalloc(1, sizeof(struct string_list));
 	if (in_reply_to) {
@@ -1509,6 +1572,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--;
 	}
@@ -1574,6 +1638,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);

Powered by blists - more mailing lists