Re: [PATCH v7 3/3] cat-file: add --follow-symlinks to --batch

From: Junio C Hamano <gitster@pobox.com>
Date: May 13, 2015, 03:57 UTC
Message-ID: <xmqqfv71ceoo.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1431481799-23560-4-git-send-email-dturner@twopensource.com>

dturner@twopensource.com writes:

Show 8 quoted lines

> From: David Turner <dturner@twitter.com>
>
> This wires the in-repo-symlink following code through to the cat-file
> builtin.  In the event of an out-of-repo link, cat-file will print
> the link in a new format.
>
> Signed-off-by: David Turner <dturner@twitter.com>
> ---

Thanks.

Show 7 quoted lines

> +--follow-symlinks::
> +	With --batch or --batch-check, follow symlinks inside the
> +	repository when requesting objects with extended SHA-1
> +...
> +	printed.
> +[normal]
> +	This option does not (currently) work correctly when an

Nice; didn't realize that "[normal]" trick would work, but why not.

Show 9 quoted lines

> +	object in the index is specified (e.g. `:link` instead of
> +	`HEAD:link`) rather than one in the tree.
> +[normal]
> +	This option cannot be used unless `--batch` or `--batch-check`
> +	is used, because it would be impossible to distinguish between
> +	the output for an out-of-repo symlink, and the contents of a
> +	blob.
> +
> +For example, consider a git repository containing:

... but I am not sure about this paragraph. Does it align and look to be at the same level as the first paragraph that begins with "With `--batch` or `--batch-check`" (asking because I do not have an access to a good environment to check the resulting HTML easily right now)?

> +	result = get_sha1_with_context(obj_name, flags, data->sha1, &ctx);
> +	if (result != FOUND) {
> +		switch(result) {

	switch (result) {

Show 15 quoted lines

> +		case MISSING_OBJECT:
> +			printf("%s missing\n", obj_name);
> +			break;
> +		case DANGLING_SYMLINK:
> +			printf("dangling %"PRIuMAX"\n%s\n", strlen(obj_name),
> +			       obj_name);
> +			break;
> +		case SYMLINK_LOOP:
> +			printf("loop %"PRIuMAX"\n%s\n", strlen(obj_name),
> +			       obj_name);
> +			break;
> +		case NOT_DIR:
> +			printf("notdir %"PRIuMAX"\n%s\n", strlen(obj_name),
> +			       obj_name);
> +			break;

Don't all these lengths need to be cast to uintmax_t, like the "symlink" one below?

By the way, I noticed that my compiler is so stupid that it does not realize this "switch (result)" is inside "if (result != FOUND)" and missing "case FOUND:" is perfectly fine. The remainder of the code I have been compiling with -Wswitch, but this one makes me to drop it.

Show 11 quoted lines

> +		}
> +		fflush(stdout);
> +		return 0;
> +	}
> +
> +	if (ctx.mode == 0) {
> +		printf("symlink %"PRIuMAX"\n%s\n",
> +		       (uintmax_t)ctx.symlink_path.len,
> +		       ctx.symlink_path.buf);
>  		fflush(stdout);
>  		return 0;