Re: [PATCH 2/4] cat-file: introduce the --filters option

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 18, 2016, 16:37 UTC
Message-ID: <xmqqk2fe6njs.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <f1e188907f31abef9e82bd6b0da120ab7d9bd4a7.1471524357.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

Show 10 quoted lines

> As suggested by its name, the --filters option applies the filters
> that are currently configured for the specified path.
>
> This feature comes in handy when a 3rd-party tool wants to work with
> the contents of files from past revisions as if they had been checked
> out, but without detouring via temporary files.
>
> Note that we ensure that symbolic links are unaffected (we know from
> looking at the mode whether it refers to a symbolic link or a regular
> file).

I think you can lose the last paragraph and enrich the first one instead. The text as-given does not even say anything about the new option affecting only blobs.

	The --filters option applies the convert_to_working_tree()
	filter for the path when showing the contents of a regular
	file blob object.

or something like that.

Show 5 quoted lines

> +--filters::
> +	Show the content as transformed by the filters configured in
> +	the current working tree for the given <path> (i.e. smudge filters,
> +	end-of-line conversion, etc). In this case, <object> has to be of
> +	the form <tree-ish>:<path>, or :<path>.

Makes sense. We show the contents as if we are checking the blob out to the given <path>, in other words.

Show 31 quoted lines

> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 2dfe626..0b74afa 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -20,6 +20,33 @@ struct batch_options {
>  	const char *format;
>  };
>  
> +static int filter_object(const char *path, unsigned mode,
> +			 const unsigned char *sha1,
> +			 char **buf, unsigned long *size)
> +{
> +	enum object_type type;
> +
> +	*buf = read_sha1_file(sha1, &type, size);
> +	if (!*buf)
> +		return error(_("cannot read object %s '%s'"),
> +			sha1_to_hex(sha1), path);
> +	if (type != OBJ_BLOB) {
> +		free(*buf);
> +		return error(_("blob expected for %s '%s'"),
> +			sha1_to_hex(sha1), path);
> +	}
> +	if (S_ISREG(mode)) {
> +		struct strbuf strbuf = STRBUF_INIT;
> +		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
> +			free(*buf);
> +			*size = strbuf.len;
> +			*buf = strbuf_detach(&strbuf, NULL);
> +		}
> +	}

This is just a tangent for future clean-up of the coding convention, but if we used size_t internally throughout the system, we would be able to use strbuf_detach(&strbuf, size) instead. With the current coding convention, all internal sizes are ulong, and this is the best we could do.

Show 14 quoted lines

> @@ -61,6 +88,16 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
>  	case 'e':
>  		return !has_sha1_file(sha1);
>  
> +	case 'w':
> +		if (!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,
> +				  sha1, &buf, &size))
> +			return -1;
> +		break;
> +

Makes sense.

Show 18 quoted lines

> diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
> new file mode 100755
> index 0000000..e466634
> --- /dev/null
> +++ b/t/t8010-cat-file-filters.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +test_description='git cat-file filters support'
> +. ./test-lib.sh
> +
> +test_expect_success 'setup ' '
> +	echo "*.txt eol=crlf diff=txt" >.gitattributes &&
> +	echo "hello" | append_cr >world.txt &&
> +	git add .gitattributes world.txt &&
> +	test_tick &&
> +	git commit -m "Initial commit"
> +'

OK, so we ask *.txt in the working tree to be CRLF (and presumably that means the in-repository objects are LF).

Show 14 quoted lines

> +has_cr () {
> +	tr '\015' Q <"$1" | grep Q >/dev/null
> +}
> +
> +test_expect_success 'no filters with `git show`' '
> +	git show HEAD:world.txt >actual &&
> +	! has_cr actual
> +
> +'
> +
> +test_expect_success 'no filters with cat-file' '
> +	git cat-file blob HEAD:world.txt >actual &&
> +	! has_cr actual
> +'

OK.

> +test_expect_success 'cat-file --filters converts to worktree version' '
> +	git cat-file --filters HEAD:world.txt >actual &&
> +	has_cr actual
> +'

OK.