| Message ID | 20200728233446.3066485-30-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SHA-256, part 3/3 | expand |
On Tue, Jul 28, 2020 at 7:35 PM brian m. carlson <sandals@crustytoothpaste.net> 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@crustytoothpaste.net> > --- > 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"));
diff --git a/http-fetch.c b/http-fetch.c index 1df376e745..65050dee2b 100644 --- a/http-fetch.c +++ b/http-fetch.c @@ -86,6 +86,8 @@ int cmd_main(int argc, const char **argv) int packfile = 0; struct object_id packfile_hash; + 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(); - git_config(git_default_config, NULL); if (packfile) {
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@crustytoothpaste.net> --- http-fetch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)