git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Stefan Beller <sbeller@google.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, jonathantanmy@google.com,
	Stefan Beller <sbeller@google.com>
Subject: [PATCHv5 0/7] describe blob
Date: Wed, 15 Nov 2017 18:00:32 -0800
Message-ID: <20171116020039.17810-1-sbeller@google.com> (raw)

Thanks Jonathan and Junio for the patient review!

* fixed issues brought up in the last patch, see interdiff below.
  (I found that walking from so many refs as starting points was
   the source of confusion, hence we only want to walk from HEAD
* reworded commit messages from earlier patches
* added a BUGS section to the man page
* took Junios suggestion for the NAME section.

Stefan Beller (7):
  t6120: fix typo in test name
  list-objects.c: factor out traverse_trees_and_blobs
  revision.h: introduce blob/tree walking in order of the commits
  builtin/describe.c: rename `oid` to avoid variable shadowing
  builtin/describe.c: print debug statements earlier
  builtin/describe.c: factor out describe_commit
  builtin/describe.c: describe a blob

 Documentation/git-describe.txt     |  18 +++++-
 Documentation/rev-list-options.txt |   5 ++
 builtin/describe.c                 | 122 ++++++++++++++++++++++++++++---------
 list-objects.c                     |  58 ++++++++++++------
 revision.c                         |   2 +
 revision.h                         |   3 +-
 t/t6100-rev-list-in-order.sh       |  77 +++++++++++++++++++++++
 t/t6120-describe.sh                |  36 ++++++++++-
 8 files changed, 270 insertions(+), 51 deletions(-)
 create mode 100755 t/t6100-rev-list-in-order.sh

-- 
2.15.0.128.gcadd42da22

diff --git c/Documentation/git-describe.txt w/Documentation/git-describe.txt
index a25443ca91..e027fb8c4b 100644
--- c/Documentation/git-describe.txt
+++ w/Documentation/git-describe.txt
@@ -3,8 +3,7 @@ git-describe(1)
 
 NAME
 ----
-git-describe - Describe a commit or blob using the graph relations
-
+git-describe - Give an object a human readable name based on an available ref
 
 SYNOPSIS
 --------
@@ -27,13 +26,9 @@ see the -a and -s options to linkgit:git-tag[1].
 
 If the given object refers to a blob, it will be described
 as `<commit-ish>:<path>`, such that the blob can be found
-at `<path>` in the `<commit-ish>`. Note, that the commit is likely
-not the commit that introduced the blob, but the one that was found
-first; to find the commit that introduced the blob, you need to find
-the commit that last touched the path, e.g.
-`git log <commit-description> -- <path>`.
-As blobs do not point at the commits they are contained in,
-describing blobs is slow as we have to walk the whole graph.
+at `<path>` in the `<commit-ish>`, which itself describes the
+first commit in which this blob occurs in a reverse revision walk
+from HEAD.
 
 OPTIONS
 -------
@@ -197,6 +192,14 @@ selected and output.  Here fewest commits different is defined as
 the number of commits which would be shown by `git log tag..input`
 will be the smallest number of commits possible.
 
+BUGS
+----
+
+Tree objects as well as tag objects not pointing at commits, cannot be described.
+When describing blobs, the lightweight tags pointing at blobs are ignored,
+but the blob is still described as <committ-ish>:<path> despite the lightweight
+tag being favorable.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git c/builtin/describe.c w/builtin/describe.c
index a2a5fdc48d..5b4bfaba3f 100644
--- c/builtin/describe.c
+++ w/builtin/describe.c
@@ -457,7 +457,8 @@ static void process_object(struct object *obj, const char *path, void *data)
 		reset_revision_walk();
 		describe_commit(&pcd->current_commit, pcd->dst);
 		strbuf_addf(pcd->dst, ":%s", path);
-		pcd->revs->max_count = 0;
+		free_commit_list(pcd->revs->commits);
+		pcd->revs->commits = NULL;
 	}
 }
 
@@ -468,12 +469,7 @@ static void describe_blob(struct object_id oid, struct strbuf *dst)
 	struct process_commit_data pcd = { null_oid, oid, dst, &revs};
 
 	argv_array_pushl(&args, "internal: The first arg is not parsed",
-		"--all", "--reflog", /* as many starting points as possible */
-		/* NEEDSWORK: --all is incompatible with worktrees for now: */
-		"--single-worktree",
-		"--objects",
-		"--in-commit-order",
-		"--reverse",
+		"--objects", "--in-commit-order", "--reverse", "HEAD",
 		NULL);
 
 	init_revisions(&revs, NULL);
diff --git c/list-objects.c w/list-objects.c
index 07a92f35fe..4caa6fcb77 100644
--- c/list-objects.c
+++ w/list-objects.c
@@ -239,7 +239,13 @@ void traverse_commit_list(struct rev_info *revs,
 		if (commit->tree)
 			add_pending_tree(revs, commit->tree);
 		show_commit(commit, data);
+
 		if (revs->tree_blobs_in_commit_order)
+			/*
+			 * NEEDSWORK: Adding the tree and then flushing it here
+			 * needs a reallocation for each commit. Can we pass the
+			 * tree directory without allocation churn?
+			 */
 			traverse_trees_and_blobs(revs, &csp, show_object, data);
 	}
 	traverse_trees_and_blobs(revs, &csp, show_object, data);
diff --git c/t/t6120-describe.sh w/t/t6120-describe.sh
index ec4f25d009..4668f0058e 100755
--- c/t/t6120-describe.sh
+++ w/t/t6120-describe.sh
@@ -310,7 +310,7 @@ test_expect_success 'describe ignoring a broken submodule' '
 	grep broken out
 '
 
-test_expect_success 'describe a blob at a tag' '
+test_expect_success 'describe a blob at a directly tagged commit' '
 	echo "make it a unique blob" >file &&
 	git add file && git commit -m "content in file" &&
 	git tag -a -m "latest annotated tag" unique-file &&
@@ -319,14 +319,29 @@ test_expect_success 'describe a blob at a tag' '
 	test_cmp expect actual
 '
 
-test_expect_success 'describe a blob with its last introduction' '
+test_expect_success 'describe a blob with its first introduction' '
 	git commit --allow-empty -m "empty commit" &&
 	git rm file &&
 	git commit -m "delete blob" &&
 	git revert HEAD &&
 	git commit --allow-empty -m "empty commit" &&
 	git describe HEAD:file >actual &&
-	grep unique-file-3-g actual
+	echo "unique-file:file" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'describe directly tagged blob' '
+	git tag test-blob unique-file:file &&
+	git describe test-blob >actual &&
+	echo "unique-file:file" >expect &&
+	# suboptimal: we rather want to see "test-blob"
+	test_cmp expect actual
+'
+
+test_expect_success 'describe tag object' '
+	git tag test-blob-1 -a -m msg unique-file:file &&
+	test_must_fail git describe test-blob-1 2>actual &&
+	grep "fatal: test-blob-1 is neither a commit nor blob" actual
 '
 
 test_expect_failure ULIMIT_STACK_SIZE 'name-rev works in a deep repo' '

             reply index

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-16  2:00 Stefan Beller [this message]
2017-11-16  2:00 ` [PATCHv5 1/7] t6120: fix typo in test name Stefan Beller
2017-11-16  2:00 ` [PATCHv5 2/7] list-objects.c: factor out traverse_trees_and_blobs Stefan Beller
2017-11-16  2:00 ` [PATCHv5 3/7] revision.h: introduce blob/tree walking in order of the commits Stefan Beller
2017-11-16  2:00 ` [PATCHv5 4/7] builtin/describe.c: rename `oid` to avoid variable shadowing Stefan Beller
2017-11-16  2:00 ` [PATCHv5 5/7] builtin/describe.c: print debug statements earlier Stefan Beller
2017-11-16  2:00 ` [PATCHv5 6/7] builtin/describe.c: factor out describe_commit Stefan Beller
2017-11-16  2:00 ` [PATCHv5 7/7] builtin/describe.c: describe a blob Stefan Beller
2017-11-16  3:24   ` Junio C Hamano
2017-11-16 19:34     ` Stefan Beller
2017-11-22  7:55       ` Junio C Hamano
2017-11-22 17:00         ` Stefan Beller
2017-11-24  7:18           ` Junio C Hamano
2017-11-27 19:38             ` Stefan Beller
2017-11-27 23:37               ` Junio C Hamano
2017-11-20 15:10   ` Philip Oakley
2017-12-19 19:22   ` Junio C Hamano
2017-12-19 19:39     ` Stefan Beller
2017-12-22 23:10       ` Junio C Hamano

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171116020039.17810-1-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox