[PATCH v2 0/5] Sparse checkout: fix bug with worktree of bare repo

["Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>]

This patch series includes a fix to the bug reported by Sean Allred [1] and
diagnosed by Eric Sunshine [2].

The root cause is that 'git sparse-checkout init' writes to the worktree
config without checking that core.bare might need to be set. This only
matters when the base repository is bare, since creating the config.worktree
file and enabling extensions.worktreeConfig will cause Git to treat the base
repo's core.bare=false as important for this worktree.

This series fixes this, but also puts in place some helpers to prevent this
from happening in the future. While here, some of the config paths are
modified to take a repository struct.

The critical bits are in Patches 3, 4, and 5 which introduce a helper for
upgrading to worktree config, a helper to write to worktree config, and then
consume that config helper in builtin/sparse-checkout.c and sparse-index.c.

[1]
https://lore.kernel.org/git/CABceR4bZmtC4rCwgxZ1BBYZP69VOUca1f_moJoP989vTUZWu9Q@mail.gmail.com/
[2]
https://lore.kernel.org/git/CAPig+cQ6U_yFw-X2OWrizB1rbCvc4bNxuSzKFzmoLNnm0GH8Eg@mail.gmail.com/


Update in v2
============

 * Eric correctly pointed out that I was writing core.bare incorrectly. It
   should move out of the core config and into the core repository's
   worktree config.
 * Patch 3 is new, separating the "upgrade" logic out of config.c, but it is
   still called by the config helper to make it painless to write worktree
   config.

Thanks, -Stolee

Derrick Stolee (5):
  setup: use a repository when upgrading format
  config: make some helpers repo-aware
  worktree: add upgrade_to_worktree_config()
  config: add repo_config_set_worktree_gently()
  sparse-checkout: use repo_config_set_worktree_gently()

 builtin/sparse-checkout.c          | 25 +++++-----------
 config.c                           | 39 +++++++++++++++++++++++--
 config.h                           | 14 +++++++++
 list-objects-filter-options.c      |  2 +-
 repository.h                       |  2 +-
 setup.c                            |  6 ++--
 sparse-index.c                     | 10 ++-----
 t/t1091-sparse-checkout-builtin.sh | 16 +++++++++-
 worktree.c                         | 47 ++++++++++++++++++++++++++++++
 worktree.h                         | 12 ++++++++
 10 files changed, 140 insertions(+), 33 deletions(-)


base-commit: 69a9c10c95e28df457e33b3c7400b16caf2e2962
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1101%2Fderrickstolee%2Fsparse-checkout%2Fbare-worktree-bug-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1101/derrickstolee/sparse-checkout/bare-worktree-bug-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1101

Range-diff vs v1:

 1:  28813703ff6 = 1:  889e69dc45d setup: use a repository when upgrading format
 2:  3b549770eb9 = 2:  3e01356815a config: make some helpers repo-aware
 -:  ----------- > 3:  ed8e2a7b19d worktree: add upgrade_to_worktree_config()
 3:  67993f6cff2 ! 4:  22896e9bb04 config: add repo_config_set_worktree_gently()
     @@ Metadata
       ## Commit message ##
          config: add repo_config_set_worktree_gently()
      
     -    When adding config values to the worktree config, we might enable the
     -    extensions.worktreeConfig setting and create the config.worktree file
     -    for the first time. When the base repository is bare, this creates a
     -    change of behavior for determining if the worktree is bare or not. A
     -    worktree off of a bare repository is assumed to be non-bare when
     -    extensions.worktreeConfig is disabled. When extensions.worktreeConfig is
     -    enabled but config.worktree is empty, the worktree is considered bare
     -    because the base repo's core.bare=true setting is used.
     +    The previous change added upgrade_to_worktree_config() to assist
     +    creating a worktree-specific config for the first time. However, this
     +    requires every config writer to care about that upgrade before writing
     +    to the worktree-specific config. In addition, callers need to know how
     +    to generate the name of the config.worktree file and pass it to the
     +    config API.
      
     -    To avoid issues like this, create a helper that initializes all the
     -    right settings in the correct order. A caller will be added in the next
     -    change.
     +    To assist, create a new repo_config_set_worktree_gently() method in the
     +    config API that handles the upgrade_to_worktree_config() method in
     +    addition to assigning the value in the worktree-specific config. This
     +    will be consumed by an upcoming change.
      
          Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
      
       ## config.c ##
     +@@
     + #include "dir.h"
     + #include "color.h"
     + #include "refs.h"
     ++#include "worktree.h"
     + 
     + struct config_source {
     + 	struct config_source *prev;
      @@ config.c: int git_config_set_gently(const char *key, const char *value)
       	return git_config_set_multivar_gently(key, value, NULL, 0);
       }
     @@ config.c: int git_config_set_gently(const char *key, const char *value)
      +int repo_config_set_worktree_gently(struct repository *r,
      +				    const char *key, const char *value)
      +{
     -+	int res;
     -+	const char *config_filename = repo_git_path(r, "config.worktree");
     -+
     -+	/*
     -+	 * Ensure that core.bare reflects the current worktree, since the
     -+	 * logic for is_bare_repository() changes if extensions.worktreeConfig
     -+	 * is disabled.
     -+	 */
     -+	if ((res = git_config_set_multivar_in_file_gently(config_filename, "core.bare",
     -+							  r->worktree ? "false" : "true",
     -+							  NULL, 0))) {
     -+		error(_("unable to set core.bare setting in worktree config"));
     -+		return res;
     -+	}
     -+	if (upgrade_repository_format(r, 1) < 0)
     -+		return error(_("unable to upgrade repository format to enable worktreeConfig"));
     -+	if ((res = git_config_set_gently("extensions.worktreeConfig", "true"))) {
     -+		error(_("failed to set extensions.worktreeConfig setting"));
     -+		return res;
     -+	}
     -+
     -+	return git_config_set_multivar_in_file_gently(config_filename, key, value, NULL, 0);
     ++	return upgrade_to_worktree_config(r) ||
     ++	       git_config_set_multivar_in_file_gently(
     ++			 repo_git_path(r, "config.worktree"),
     ++			 key, value, NULL, 0);
      +}
      +
       void git_config_set(const char *key, const char *value)
     @@ config.h: void git_config_set_in_file(const char *, const char *, const char *);
       
      +/**
      + * Write a config value into the config.worktree file for the current
     -+ * worktree. This will initialize extensions.worktreeConfig if necessary.
     ++ * worktree. This will initialize extensions.worktreeConfig if necessary,
     ++ * which might trigger some changes to the root repository's config file.
      + */
      +int repo_config_set_worktree_gently(struct repository *, const char *, const char *);
      +
 4:  6202f767f4a ! 5:  06457fafa78 sparse-checkout: use repo_config_set_worktree_gently()
     @@ t/t1091-sparse-checkout-builtin.sh: test_expect_success 'git sparse-checkout ini
      +	(
      +		cd worktree &&
      +		git sparse-checkout init &&
     -+		test_cmp_config false core.bare &&
     ++		test_must_fail git config core.bare &&
      +		git sparse-checkout set /*
     -+	)
     ++	) &&
     ++	git -C bare config --list --show-origin >actual &&
     ++	grep "file:config.worktree	core.bare=true" actual
      +'
      +
       test_expect_success 'git sparse-checkout list after init' '

-- 
gitgitgadget