Re: [PATCH 3/4] cat-file --textconv/--filters: allow specifying the path separately

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > There are circumstances when it is relatively easy to figure out the
> > object name for a given path, but not the revision. For example, when
> 
> revision of the containing tree, I presume?

> > Let's simplify this dramatically, because we do not really need that
> > revision at all: all we care about is that we know the path. In the
> > scenario described above, we do know the path, and we just want to
> > specify it separately from the object name.
> 
> I wouldn't call it "simplifying dramatically".  It is just to
> support two use cases that is different from the use case where you
> want to use "<tree>:<path>".

> We already have a precedent in "hash-object --path=<path>" for these
> two different uses cases from the primary one.  That form can be
> used when we know the contents but we do not know where the contents
> came from.  It can also be used when we want to pretend that
> contents came from a specific path, that may be different from the
> file we are hashing.

> Let's be consistent and (1) call it "--path", and (2) explain it as
> a feature to allow you to tell the path separately or allow you to
> pretend that the content is at a path different from reality.
> 
> After all, you would want to ignore <path2> part in this construct:
> 
> 	git cat-file --filter --path=<path1> HEAD:<path2>
> 
> for the purpose of filtering, right?

> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > index 0b74afa..5ff58b3 100644
> > --- a/builtin/cat-file.c
> > +++ b/builtin/cat-file.c
> > @@ -20,6 +20,8 @@ struct batch_options {
> >  	const char *format;
> >  };
> >  
> > +static const char *force_path;
> > +
> >  static int filter_object(const char *path, unsigned mode,
> >  			 const unsigned char *sha1,
> >  			 char **buf, unsigned long *size)
> > @@ -89,21 +91,24 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
> >  		return !has_sha1_file(sha1);
> >  
> >  	case 'w':
> > -		if (!obj_context.path[0])
> > +		if (!force_path && !obj_context.path[0])
> >  			die("git cat-file --filters %s: <object> must be "
> >  			    "<sha1:path>", obj_name);
> >  
> > -		if (filter_object(obj_context.path, obj_context.mode,
> > +		if (filter_object(force_path ? force_path : obj_context.path,
> > +				  force_path ? 0100644 : obj_context.mode,
> >  				  sha1, &buf, &size))
> 
> The mode override is somewhat questionable.  Wouldn't you want to
> reject
> 
> 	git cat-file --filter --path=Makefile HEAD:RelNotes
> 
> when HEAD:RelNotes blob is known to be a symbolic link?  After all,
> you would reject this
> 
> 	git cat-file --filter --path=Makefile HEAD:t/
> 
> and it is quite similar, no?  I think this can be argued both ways,
> but I have a feeling that obj_context.mode, if available, should be
> honored with or without force_path.

> > diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
> > index e466634..fd17159 100755
> > --- a/t/t8010-cat-file-filters.sh
> > +++ b/t/t8010-cat-file-filters.sh
> > @@ -31,4 +31,17 @@ test_expect_success 'cat-file --filters converts to worktree version' '
> >  	has_cr actual
> >  '
> >  
> > +test_expect_success 'cat-file --filters --use-path=<path> works' '
> > +	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
> > +	git cat-file --filters --use-path=world.txt $sha1 >actual &&
> > +	has_cr actual
> > +'
> > +
> > +test_expect_success 'cat-file --textconv --use-path=<path> works' '
> > +	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
> > +	test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
> > +	git cat-file --textconv --use-path=hello.txt $sha1 >rot13 &&
> > +	test uryyb = "$(cat rot13 | remove_cr)"
> > +'
> 
> I think I saw some code to ensure "when giving this option you need
> that option in effect, too"; they should be tested here, too, no?