git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/2] revision: keep track of the end-user input from the command line

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 26, 2011, 01:00 UTC
Message-ID
<7vbovdgf73.fsf_-_@alter.siamese.dyndns.org>
In-Reply-To
<438ea0b254ccafb3fc9f3431f8f86007cc03132b.1314290439.git.brad.king@kitware.com>

Given a complex set of revision specifiers on the command line, it is too late to look at the flags of the objects in the initial traversal list at the beginning of limit_list() in order to determine what the objects the end-user explicitly listed on the command line were. The process to move objects from the pending array to the traversal list may have marked objects that are not mentioned as UNINTERESTING, when handle_commit() marked the parents of UNINTERESTING commits mentioned on the command line by calling mark_parents_uninteresting().

This made "rev-list --ancestry-path ^A ..." to mistakenly list commits that are descendants of A's parents but that are not descendants of A itself, as ^A from the command line causes A and its parents marked as UNINTERESTING before coming to limit_list(), and we try to enumerate the commits that are descendants of these commits that are UNINTERESTING before we start walking the history.

It actually is too late even if we inspected the pending object array before calling prepare_revision_walk(), as some of the same objects might have been mentioned twice, once as positive and another time as negative. The "rev-list --some-option A --not --all" command may want to notice, even if the resulting set is empty, that the user showed some interest in "A" and do something special about it.

Prepare a separate array to keep track of what syntactic element was used to cause each object to appear in the pending array from the command line, and populate it as setup_revisions() parses the command line.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Should apply cleanly on top of your test case.
 revision.c |   37 +++++++++++++++++++++++++++++++++----
 revision.h |   20 ++++++++++++++++++++
 2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/revision.c b/revision.c
index 96d7fa7..3e87c86 100644
--- a/revision.c
+++ b/revision.c
@@ -797,6 +797,23 @@ static int limit_list(struct rev_info *revs)
 	return 0;
 }
 
+static void add_rev_cmdline(struct rev_info *revs,
+			    struct object *item,
+			    const char *name,
+			    int whence,
+			    unsigned flags)
+{
+	struct rev_cmdline_info *info = &revs->cmdline;
+	int nr = info->nr;
+
+	ALLOC_GROW(info->rev, nr + 1, info->alloc);
+	info->rev[nr].item = item;
+	info->rev[nr].name = name;
+	info->rev[nr].whence = whence;
+	info->rev[nr].flags = flags;
+	info->nr++;
+}
+
 struct all_refs_cb {
 	int all_flags;
 	int warned_bad_reflog;
@@ -809,6 +826,7 @@ static int handle_one_ref(const char *path, const unsigned char *sha1, int flag,
 	struct all_refs_cb *cb = cb_data;
 	struct object *object = get_reference(cb->all_revs, path, sha1,
 					      cb->all_flags);
+	add_rev_cmdline(cb->all_revs, object, path, REV_CMD_REF, cb->all_flags);
 	add_pending_object(cb->all_revs, object, path);
 	return 0;
 }
@@ -835,6 +853,7 @@ static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
 		struct object *o = parse_object(sha1);
 		if (o) {
 			o->flags |= cb->all_flags;
+			/* ??? CMDLINEFLAGS ??? */
 			add_pending_object(cb->all_revs, o, "");
 		}
 		else if (!cb->warned_bad_reflog) {
@@ -871,12 +890,13 @@ static void handle_reflog(struct rev_info *revs, unsigned flags)
 	for_each_reflog(handle_one_reflog, &cb);
 }
 
-static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
+static int add_parents_only(struct rev_info *revs, const char *arg_, int flags)
 {
 	unsigned char sha1[20];
 	struct object *it;
 	struct commit *commit;
 	struct commit_list *parents;
+	const char *arg = arg_;
 
 	if (*arg == '^') {
 		flags ^= UNINTERESTING;
@@ -898,6 +918,7 @@ static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
 	for (parents = commit->parents; parents; parents = parents->next) {
 		it = &parents->item->object;
 		it->flags |= flags;
+		add_rev_cmdline(revs, it, arg_, REV_CMD_PARENTS_ONLY, flags);
 		add_pending_object(revs, it, arg);
 	}
 	return 1;
@@ -987,7 +1008,7 @@ static void prepare_show_merge(struct rev_info *revs)
 	revs->limited = 1;
 }
 
-int handle_revision_arg(const char *arg, struct rev_info *revs,
+int handle_revision_arg(const char *arg_, struct rev_info *revs,
 			int flags,
 			int cant_be_filename)
 {
@@ -996,6 +1017,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 	struct object *object;
 	unsigned char sha1[20];
 	int local_flags;
+	const char *arg = arg_;
 
 	dotdot = strstr(arg, "..");
 	if (dotdot) {
@@ -1004,6 +1026,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 		const char *this = arg;
 		int symmetric = *next == '.';
 		unsigned int flags_exclude = flags ^ UNINTERESTING;
+		unsigned int a_flags;
 
 		*dotdot = 0;
 		next += symmetric;
@@ -1036,10 +1059,15 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 				add_pending_commit_list(revs, exclude,
 							flags_exclude);
 				free_commit_list(exclude);
-				a->object.flags |= flags | SYMMETRIC_LEFT;
+				a_flags = flags | SYMMETRIC_LEFT;
 			} else
-				a->object.flags |= flags_exclude;
+				a_flags = flags_exclude;
+			a->object.flags |= a_flags;
 			b->object.flags |= flags;
+			add_rev_cmdline(revs, &a->object, this,
+					REV_CMD_LEFT, a_flags);
+			add_rev_cmdline(revs, &b->object, next,
+					REV_CMD_RIGHT, flags);
 			add_pending_object(revs, &a->object, this);
 			add_pending_object(revs, &b->object, next);
 			return 0;
@@ -1070,6 +1098,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 	if (!cant_be_filename)
 		verify_non_filename(revs->prefix, arg);
 	object = get_reference(revs, arg, sha1, flags ^ local_flags);
+	add_rev_cmdline(revs, object, arg_, REV_CMD_REV, flags ^ local_flags);
 	add_pending_object_with_mode(revs, object, arg, mode);
 	return 0;
 }
diff --git a/revision.h b/revision.h
index 855464f..031cc7c 100644
--- a/revision.h
+++ b/revision.h
@@ -23,6 +23,23 @@ struct rev_info;
 struct log_info;
 struct string_list;
 
+struct rev_cmdline_info {
+	unsigned int nr;
+	unsigned int alloc;
+	struct rev_cmdline_entry {
+		struct object *item;
+		const char *name;
+		enum {
+			REV_CMD_REF,
+			REV_CMD_PARENTS_ONLY,
+			REV_CMD_LEFT,
+			REV_CMD_RIGHT,
+			REV_CMD_REV
+		} whence;
+		unsigned flags;
+	} *rev;
+};
+
 struct rev_info {
 	/* Starting list */
 	struct commit_list *commits;
@@ -31,6 +48,9 @@ struct rev_info {
 	/* Parents of shown commits */
 	struct object_array boundary_commits;
 
+	/* The end-points specified by the end user */
+	struct rev_cmdline_info cmdline;
+
 	/* Basic information */
 	const char *prefix;
 	const char *def;

-- 
1.7.6.1.385.gb7fcd0
Previous: Junio C HamanoNext: Sverre Rabbelier
Message 12 of 25 in “submodule: Demonstrate known breakage during recursive merge”
  1. submodule: Demonstrate known breakage during recursive mergeBrad King, Aug 24, 2011
  2. Heiko VoigtAug 24, 2011
  3. Junio C HamanoAug 24, 2011
  4. Heiko VoigtAug 24, 2011
  5. Brad KingAug 24, 2011
  6. Heiko VoigtAug 24, 2011
  7. Brad KingAug 24, 2011
  8. Heiko VoigtAug 24, 2011
  9. rev-list: Demonstrate breakage with --ancestry-path --allBrad King, Aug 25, 2011
  10. Junio C HamanoAug 25, 2011
  11. Junio C HamanoAug 25, 2011
  12. 1/2 revision: keep track of the end-user input from the command lineJunio C Hamano, Aug 26, 2011
  13. Sverre RabbelierAug 26, 2011
  14. Junio C HamanoAug 26, 2011
  15. 2/2 revision: do not include sibling history in --ancestry-path outputJunio C Hamano, Aug 26, 2011
  16. Brad KingAug 26, 2011
  17. Junio C HamanoAug 24, 2011
  18. allow multiple calls to submodule merge search for the same pathHeiko Voigt, Aug 25, 2011
  19. Junio C HamanoAug 25, 2011
  20. Junio C HamanoAug 25, 2011
  21. Junio C HamanoAug 25, 2011
  22. submodule: Demonstrate known breakage during recursive mergeBrad King, Aug 25, 2011
  23. submodule: Search for merges only at end of recursive mergeBrad King, Aug 26, 2011
  24. Junio C HamanoAug 26, 2011
  25. submodule: Search for merges only at end of recursive mergeBrad King, Aug 26, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.