Re: [PATCH 3/3] builtin/describe: describe blobs

On Sat, Oct 28, 2017 at 10:32 AM, Johannes Schindelin
[off-list ref] wrote:

Hi Stefan,

On Fri, 27 Oct 2017, Stefan Beller wrote:

quoted

Sometimes users are given a hash of an object and they want to identify
it further (ex.: Use verify-pack to find the largest blobs, but what are
these? or [1])

The best identification of a blob hash is done via a its path at a
given commit, which this implements.

[1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob

I also came up with a script to do that:
https://github.com/msysgit/msysgit/blob/master/bin/what-made-this-repo-so-large.sh

Your method is much more elegant, of course (it describes the commit in the
same run as it finds the object, and it does not output tons of stuff only
to be filtered).

That was the task I was given. Discussion here turned out that users
mostly only ever care about commit object names, trees and blobs are
only useful when specifically given.

quoted

@@ -282,6 +283,50 @@ static void show_suffix(int depth, const struct object_id *oid)
      printf("-%d-g%s", depth, find_unique_abbrev(oid->hash, abbrev));
 }

+struct blob_descriptor {
+     struct object_id current_commit;
+     struct object_id looking_for;
+};

Personally, I would call this process_commit_data, but I do not mind too
much about the name.

I'll take any naming suggestion, as this code was assembled in a hurry using
copy/paste and trial&error as the high-tech methods used.

quoted

+static void process_object(struct object *obj, const char *name, void *data)
+{
+     struct blob_descriptor *bd = data;
+
+     if (!oidcmp(&bd->looking_for, &obj->oid))
+             printf(_("blob %s present at path %s in commit %s\n"),
+                     oid_to_hex(&bd->looking_for), name,
+                     oid_to_hex(&bd->current_commit));
+}

s/name/path/

quoted

@@ -295,9 +340,14 @@ static void describe(const char *arg, int last_one)

      if (get_oid(arg, &oid))
              die(_("Not a valid object name %s"), arg);
-     cmit = lookup_commit_reference(&oid);
-     if (!cmit)
-             die(_("%s is not a valid '%s' object"), arg, commit_type);
+     cmit = lookup_commit_reference_gently(&oid, 1);
+     if (!cmit) {
+             if (lookup_blob(&oid))
+                     describe_blob(oid);
+             else
+                     die(_("%s is not a commit nor blob"), arg);

s/not/neither/

Nicely done, sir!

I wonder whether it would make sense to extend this to tree objects while
we are at it, but maybe that's an easy up-for-grabs.

I can look into incorporating that, too. What is the use case though?
(Is there any error message, common enough that users want to
identify trees?)

Thanks for the review,
Stefan