On Tue, Aug 22, 2023 at 05:24:14PM +0000, Derrick Stolee via GitGitGadget wrote:
From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
There are many reasons why discovering a Git directory may fail. In
particular, 8959555cee7 (setup_git_directory(): add an owner check for
the top-level directory, 2022-03-02) added ownership checks as a
security precaution.
Callers attempting to set up a Git directory may want to inform the user
about the reason for the failure. For that, expose the enum
discovery_result from within setup.c
and
"by moving it"
into cache.h where
discover_git_directory() is defined.
I initially wanted to change the return type of discover_git_directory()
to be this enum, but several callers rely upon the "zero means success".
The two problems with this are:
1. The zero value of the enum is actually GIT_DIR_NONE, so nonpositive
results are errors.
2. There are multiple successful states; positive results are
successful.
It is worth noting that GIT_DIR_NONE is not returned, so we remove this
option from the enum. We must be careful to keep the successful reasons
as positive values, so they are given explicit positive values.
Further, a use in scalar.c was previously impossible, so it is removed.
i have no clue wha this means. what is "it"?
Instead of updating all callers immediately, add a new method,
discover_git_directory_reason(), and convert discover_git_directory() to
be a thin shim on top of it.
is this really worth it, given that there are just two callers, and the
adjustment is trivial?
if you insist, note that the function name can be easily misread as
"discover_git_(directory_reason)", which is unhelpful. i typically use
an _impl suffix in such "thin wrapper" scenarios.
regards
