mbox series

[v2,0/2] checkout/fetch/pull/pack-objects: allow -h outside a repository again

Message ID pull.1139.v2.git.1644319314.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series checkout/fetch/pull/pack-objects: allow -h outside a repository again | expand

Message

Philippe Blain via GitGitGadget Feb. 8, 2022, 11:21 a.m. UTC
As reported in https://github.com/git-for-windows/git/issues/3688, calling
git fetch -h outside a repository now results in a very ugly

 BUG: repo-settings.c:23: Cannot add settings for uninitialized repository


The reason is that the prepare_repo_settings() calls (that we introduced to
support sparse index) assume that there is a gitdir, but the hack to allow
parse_options() to handle -h even outside a repository invalidates that
assumption.

One strategy I considered was to move the prepare_repo_settings() calls
after parse_options(). This would work because when parse_options() handles
-h, it exits without returning.

However, this strategy failed in my tests because e.g. cmd_unpack_objects()
does need the pack_use_sparse to be populated correctly before even parsing
the options so that it can be overridden via --sparse/--no-sparse.

Hence the current strategy where the code that prepares the repo settings
and then accesses them is guarded behind the condition that we must have a
gitdir to do so.

Note: There are other instances where prepare_repo_settings() is called
before parse_options(), e.g. in cmd_status(), in seen there are even more
instances (e.g. cmd_checkout_index()). All of those instances that are not
touched by this here patch do have special code to handle -h early, though,
before calling prepare_repo_settings() let alone parse_options().

Johannes Schindelin (2):
  checkout/fetch/pull/pack-objects: allow `-h` outside a repository
  t0012: verify that built-ins handle `-h` even without gitdir

 builtin/checkout.c     | 7 ++++---
 builtin/fetch.c        | 6 ++++--
 builtin/pack-objects.c | 8 +++++---
 builtin/pull.c         | 6 ++++--
 t/t0012-help.sh        | 7 ++++++-
 5 files changed, 23 insertions(+), 11 deletions(-)


base-commit: 4c53a8c20f8984adb226293a3ffd7b88c3f4ac1a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1139%2Fdscho%2Fprepare_repo_settings-after-parse_options-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1139/dscho/prepare_repo_settings-after-parse_options-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1139

Range-diff vs v1:

 1:  25d1a2963f2 ! 1:  6e9cdd10a70 checkout/fetch/pull/pack-objects: allow `-h` outside a repository
     @@ builtin/fetch.c: int cmd_fetch(int argc, const char **argv, const char *prefix)
       
       	argc = parse_options(argc, argv, prefix,
       			     builtin_fetch_options, builtin_fetch_usage, 0);
     -+
     - 	if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
     - 		int *sfjc = submodule_fetch_jobs_config == -1
     - 			    ? &submodule_fetch_jobs_config : NULL;
      
       ## builtin/pack-objects.c ##
      @@ builtin/pack-objects.c: int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 2:  49977ff9f4e = 2:  93df4a73dab t0012: verify that built-ins handle `-h` even without gitdir