Re: [PATCH 2/2] cat-file: configurable "best effort mode" for symlink resolution

[Junio C Hamano <gitster@pobox.com>]

"Miguel Ángel Pastor Olivar via GitGitGadget"
<gitgitgadget@gmail.com> writes:

> From: =?UTF-8?q?Miguel=20=C3=81ngel=20Pastor=20Olivar?= <migue@github.com>
>
> This patch introduces a new "best effort mode" where the object found at
> resolution step N is returned. If we've reached the end of the chain, the
> returned object will be the file at the end of the chain, however, if, after
> n resolutions we haven't reached the end of the chain, the returned object
> will represent a symlink
>
> The goal is to extend the symlink resolution process and provide the ability
> to return the object found at the designated depth instead of returning an
> error.
>
> The current code already provides a limit to the maximum number of
> resolutions that can be performed and something similar to this is returned
> back to the caller:
>
> loop SP <size> LF <object> LF
>
> With the new config setting we are looking to return the actual information
> of the object where the resolution stopped. Something similar to:
>
> <oid> blob <size>\ndata\n

I do not think this should be a configuration variable at all.
Either a command line option, or even better yet would be an
in-stream instruction ("flip into the 'tell me the last symlink
you saw before you gave up' mode"), is understandable though, given
that this is strictly for the "batch" mode.

For that matter, it is dubious that the previous one that added a
configuration variable to lower the symlink recursion limit is a
good idea.  It does not affect anything but "cat-file --batch" and
an in-stream instruction, e.g. "in this session, do not resolve more
than 3 levels", sounds like a much better fit to what this wants to
do.  That way, it will be a lot better isolated from unrelated code
paths.  It might even make sense not to introduce the new
max_symlink_depth global variable, but pass it through as a new
member in "struct object_context" given to get_oid_with_context(),
which in turn is passed as a new parameter to
get_tree_entry_follow_symlinks() function.

So, I am supportive to solving the problem this series attempts to
solve, but I am not on board with the design this series took.

Thanks.