On Tue, Jul 28, 2020 at 7:35 PM brian m. carlson
<sandals@xxxxxxxxxxxxxxxxxxxx> wrote:
> In dd4b732df7 ("upload-pack: send part of packfile response as uri",
> 2020-06-10), the git http-fetch code learned how to take ac --packfile
> option. This option takes an argument, which is the name of a packfile
> hash, and parses it using parse_oid_hex. It does so before calling
> setup_git_directory.
>
> However, in a SHA-256 repository this fails to work, since we have not
> set the hash algorithm in use and parse_oid_hex fails as a consequence.
> To ensure that we can parse packfile hashes of the right length, let's
> set up the git directory before we start parsing arguments.
>
> Signed-off-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx>
> ---
> diff --git a/http-fetch.c b/http-fetch.c
> @@ -86,6 +86,8 @@ int cmd_main(int argc, const char **argv)
> + setup_git_directory_gently(NULL);
> +
> while (arg < argc && argv[arg][0] == '-') {
> const char *p;
> @@ -115,8 +117,6 @@ int cmd_main(int argc, const char **argv)
> if (argc != arg + 2 - (commits_on_stdin || packfile))
> usage(http_fetch_usage);
>
> - setup_git_directory();
In the previous version of this series the setup_git_directory() call
was moved verbatim above the 'while' loop, and my review pointed out
that doing so made "git-http-fetch -h" fail when not in a Git
repository. It was recommended, therefore, to take advantage of
setup_git_directory_gently() to fix this problem.
But note that setup_git_directory() is defined like this:
const char *setup_git_directory(void)
{
return setup_git_directory_gently(NULL);
}
So, your use of setup_git_directory_gently(NULL) in this version of
the patch is no better than the plain setup_git_directory() call in
the previous version. It is still impossible to ask git-http-fetch for
help when not in a Git repository.
To fix this, you must pass non-NULL for the 'nongit' argument of
setup_git_directory_gently().
For robustness, in order to retain the original behavior (prior to
this patch), it would be best to check the value of 'nongit' _after_
the 'while' loop and error out if not in a repository. Perhaps
something like this (taking a hint from builtin/diff.c):
setup_git_directory_gently(&nongit);
while (...) {
...
}
if (nongit)
die(_("not a git repository"));
