Best way to apply textconv to a working tree file

Best way to apply textconv to a working tree file

[Clément Poulain]

Hello,

In order to add textconv support to "git-gui blame", we have to run
textconv on a file which is in the working tree.
Currently, "git-gui blame" uses the Tcl function 'open' to display the
content of the working-tree file. This doesn't allow us to run textconv on
it.

We are wondering what is the best way to do the textconv. Here are some
solutions we thought about:

- For revisions, "git-gui blame" is based on "git cat-file blob
<sha1:path>". Therefore, we thought of adding a "--textconv" option on
cat-file as well as a "--working-tree" option ("git cat-file --working-tree
<file>" will display the content of <file> in the working-tree)

- Use the "-p" (pretty print) cat-file's option, with a path beginning by
./ (to avoid ambiguous name like HEAD)

- Create a "git textconv" command, to easily run textconv on any object.

Which way you think is the best?

Thanks for your time and comments.
Regards

Re: Best way to apply textconv to a working tree file

[Matthieu Moy]

[ Just had IRL discussion with Clément and his co-workers ]

Clément Poulain <clement.poulain@ensimag.imag.fr> writes:

> In order to add textconv support to "git-gui blame", we have to run
> textconv on a file which is in the working tree.
> Currently, "git-gui blame" uses the Tcl function 'open' to display the
> content of the working-tree file. This doesn't allow us to run textconv on
> it.

There are actually two distinct issues :

1) The textconv functionality has an API, but isn't available as
   plumbing for scripting purposes (and git gui would like to be able
   to call textconv from the command-line).

2) If one solves issue 1) by adding textconv support to "git
   cat-file", then we can call textconv on a blob in the object
   database, but not on a worktree file. Hence, we still need to add
   support to display worktree files in "git cat-file".

> - For revisions, "git-gui blame" is based on "git cat-file blob
> <sha1:path>". Therefore, we thought of adding a "--textconv" option on
> cat-file as well as a "--working-tree" option ("git cat-file --working-tree
> <file>" will display the content of <file> in the working-tree)
>
> - Use the "-p" (pretty print) cat-file's option, with a path beginning by
> ./ (to avoid ambiguous name like HEAD)

After thinking about it, I think a mix of these solutions would be
OK:

git cat-file --textconv <blob-sha1>            # Run textconv on blob

git cat-file --textconv <tree-sha1>:<filename> # Run textconv on file
                                               # (in object database)

git cat-file --textconv ./<filename>           # Run textconv on file
                                               # (in worktree)

This preserves the property "1st argument is an option, 2nd argument
is the thing to do the cat-file on", hence avoid any possible
ambiguities in command-line option parsing.

Another option would be to resurect the WOKRTREE/STAGE magit tokens [1]
proposal, and allow

git cat-file --textconv WORKTREE:<filename>

That sounds the most natural to me, but it's probably the most
controversial too ...

[1] http://thread.gmane.org/gmane.comp.version-control.git/133087

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Best way to apply textconv to a working tree file

[Jeff King]

On Tue, Jun 01, 2010 at 06:07:06PM +0200, Matthieu Moy wrote:

> After thinking about it, I think a mix of these solutions would be
> OK:
> 
> git cat-file --textconv <blob-sha1>            # Run textconv on blob

This wouldn't work. The textconv is defined by the diff driver, which is
associated with a pathname, not a blob. You don't have a pathname here
(and in fact the same blob could potentially even be represented by
different textconvs if it appeared in different contexts).

> git cat-file --textconv <tree-sha1>:<filename> # Run textconv on file
>                                                # (in object database)

This could work in theory, but will not be as easy to implement as you
hope. The code to parse <tree-ish>:<filename> will give you only a blob
sha1, and you will have to look up the tree manually (which you will
have to split at the ":" manually, remembering that it may be as complex
as "HEAD@{12:00 yesterday}:foo.c", then check the diff attr manually for
that filename.

It would be nice if there was some way in the get_sha1* functions to
save some context, like tree context and filename. This would be helpful
for something like "git show HEAD:foo.txt", which probably should be
respecting autocrlf and smudge/clean filters.

> git cat-file --textconv ./<filename>           # Run textconv on file
>                                                # (in worktree)

FWIW, I think this combination is the best of the possible syntaxes
outlined in the original mail.

-Peff

Re: Best way to apply textconv to a working tree file

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

>> git cat-file --textconv <blob-sha1>            # Run textconv on blob
>
> This wouldn't work. The textconv is defined by the diff driver, which is
> associated with a pathname,

Right, forget it.

>> git cat-file --textconv <tree-sha1>:<filename> # Run textconv on file
>>                                                # (in object database)
>
> This could work in theory, but will not be as easy to implement as you
> hope.

Actually, Clément has already hit the issue.

> complex as "HEAD@{12:00 yesterday}:foo.c"

_that_ is the actual example I was looking for ;-) (with a : inside
the pathname to make sure you can't search it from the right).

> It would be nice if there was some way in the get_sha1* functions to
> save some context, like tree context and filename. This would be helpful
> for something like "git show HEAD:foo.txt", which probably should be
> respecting autocrlf and smudge/clean filters.

Yup. The code to do the parsing is already there, it "just" needs to
be made available through a clean API.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Best way to apply textconv to a working tree file

[Jeff King]

On Tue, Jun 01, 2010 at 07:30:36PM +0200, Matthieu Moy wrote:

> > It would be nice if there was some way in the get_sha1* functions to
> > save some context, like tree context and filename. This would be helpful
> > for something like "git show HEAD:foo.txt", which probably should be
> > respecting autocrlf and smudge/clean filters.
> 
> Yup. The code to do the parsing is already there, it "just" needs to
> be made available through a clean API.

I was thinking of something like the patch below, which applies
clean/smudge filters to the output of "cat-file -p".

It keeps a global context for the last sha1 looked up. Probably
get_sha1_with_mode should be folded into get_sha1_with_context, as mode
is really just another case of this exact sort of mid-lookup context.
And then we don't get a proliferation of "get_sha1_with_*" functions,
which doesn't scale.  I'll leave that as an exercise for your students.
:)

You could get fancier, including things like which ref we ended up
looking at to get to the object. I seem to recall running into a
situation where I wanted to know "foo" when looking up "foo^", but I
don't remember where now.  I think it makes sense to keep it simple for
now, and people could add more context elements as needed.

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index a933eaa..4d1e634 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -87,6 +87,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
 	void *buf;
 	unsigned long size;
 
+	object_resolve_context_init(&the_resolve_context);
 	if (get_sha1(obj_name, sha1))
 		die("Not a valid object name %s", obj_name);
 
@@ -129,6 +130,18 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
 			pprint_tag(sha1, buf, size);
 			return 0;
 		}
+		if (type == OBJ_BLOB) {
+			struct strbuf out = STRBUF_INIT;
+
+			if (!the_resolve_context.path[0])
+				break;
+			if (convert_to_working_tree(the_resolve_context.path,
+						buf, size, &out) < 0)
+				die("unable to prepare blob for printing");
+			free(buf);
+			size = out.len;
+			buf = strbuf_detach(&out, NULL);
+		}
 
 		/* otherwise just spit out the data */
 		break;
diff --git a/cache.h b/cache.h
index c966023..c030083 100644
--- a/cache.h
+++ b/cache.h
@@ -730,6 +730,13 @@ static inline unsigned int hexval(unsigned char c)
 #define MINIMUM_ABBREV 4
 #define DEFAULT_ABBREV 7
 
+struct object_resolve_context {
+	unsigned char tree[20];
+	char path[PATH_MAX];
+};
+extern struct object_resolve_context the_resolve_context;
+void object_resolve_context_init(struct object_resolve_context *orc);
+
 extern int get_sha1(const char *str, unsigned char *sha1);
 extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int gently, const char *prefix);
 static inline int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
diff --git a/sha1_name.c b/sha1_name.c
index bf92417..cc049bf 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -7,6 +7,13 @@
 #include "refs.h"
 #include "remote.h"
 
+struct object_resolve_context the_resolve_context;
+
+void object_resolve_context_init(struct object_resolve_context *orc)
+{
+	memset(orc, 0, sizeof(*orc));
+}
+
 static int find_short_object_filename(int len, const char *name, unsigned char *sha1)
 {
 	struct alternate_object_database *alt;
@@ -1104,6 +1111,10 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,
 							   tree_sha1, object_name);
 				free(object_name);
 			}
+			hashcpy(the_resolve_context.tree, tree_sha1);
+			strncpy(the_resolve_context.path, filename,
+				sizeof(the_resolve_context.path));
+			the_resolve_context.path[sizeof(the_resolve_context.path)] = '\0';
 			return ret;
 		} else {
 			if (!gently)

Re: Best way to apply textconv to a working tree file

[Clément Poulain]

We have finally chosen to use the "git cat-file --textconv
<commit_sha1>:<filename>" option, it works fine.
We are now using your patch to easily get the path, and it seems to fit
very well too :)

Thanks a lot

Re: Best way to apply textconv to a working tree file

[Jeff King]

On Tue, Jun 01, 2010 at 03:41:05PM +0200, Clément Poulain wrote:

> We are wondering what is the best way to do the textconv. Here are some
> solutions we thought about:

One solution you didn't mention would be to do it all yourself:

  driver=`git check-attr diff "$file" | cut -d: -f3`
  textconv=`git config diff.$driver.textconv`
  $textconv <$file >$file.converted

This has the advantage of working with existing versions of git. The
downside is that it's more code (e.g., my parsing above is quite sloppy
and loose. Doing it right would be a few more lines).

Furthermore, it doesn't use the textconv cache at all. For working tree
files, this might not matter (if you pull the sha1 out of the index,
though, you can still check the cache, and unchanged working tree files
are likely to be in the cache). But for blobs in general, the cache is
worth using.

-Peff

Re: Best way to apply textconv to a working tree file

[Clément Poulain]

On Tue, 1 Jun 2010 13:04:27 -0400, Jeff King <peff@peff.net> wrote:
> On Tue, Jun 01, 2010 at 03:41:05PM +0200, Clément Poulain wrote:
> 
>> We are wondering what is the best way to do the textconv. Here are some
>> solutions we thought about:
> 
> One solution you didn't mention would be to do it all yourself:
> 
>   driver=`git check-attr diff "$file" | cut -d: -f3`
>   textconv=`git config diff.$driver.textconv`
>   $textconv <$file >$file.converted
> 
> This has the advantage of working with existing versions of git. The
> downside is that it's more code (e.g., my parsing above is quite sloppy
> and loose. Doing it right would be a few more lines).
> 
> Furthermore, it doesn't use the textconv cache at all. For working tree
> files, this might not matter (if you pull the sha1 out of the index,
> though, you can still check the cache, and unchanged working tree files
> are likely to be in the cache). But for blobs in general, the cache is
> worth using.
> 
> -Peff

It seems to really fit for git gui. 
About the cache : this method will be only used by git gui (when it's
launched) on working tree files, so it seems OK for me.
Thank you