[PATCH 0/3] Create stronger guard rails on replace refs

(This series is based on tb/pack-bitmap-traversal-with-boundary due to wanting to modify prepare_repo_settings() in a similar way.)

The replace-refs can be ignored via the core.useReplaceRefs=false config setting. This setting is possible to miss in some Git commands if they do not load default config at the appropriate time. See [1] for a recent example of this.

[1] https://lore.kernel.org/git/pull.1530.git.1683745654800.gitgitgadget@gmail.com/

This series aims to avoid this kind of error from happening in the future. The idea is to encapsulate the setting in such a way that we can guarantee that config has been checked before using the in-memory value.

Further, we must be careful that some Git commands want to disable replace refs unconditionally, as if GIT_NO_REPLACE_REFS was enabled in the environment.

The approach taken here is to split the global into two different sources. First, read_replace_refs is kept (but moved to replace-objects.c scope) and reflects whether or not the feature is permitted by the environment and the current command. Second, a new value is added to repo-settings and this is checked after using prepare_repo_settings() to guarantee the config has been read.

This presents a potential behavior change, in that now core.useReplaceRefs is specific to each in-memory repository instead of applying the superproject value to all submodules. I could not find a Git command that has multiple in-memory repositories and follows OIDs to object contents, so I'm not sure how to demonstrate it in a test.

Here is the breakdown of the series:

 * Patch 1 creates disable_replace_refs() to encapsulate the global
   disabling of the feature.
 * Patch 2 creates replace_refs_enabled() to check if the feature is enabled
   (with respect to a given repository). This is a thin wrapper of the
   global at this point, but does allow us to remove it from environment.h.
 * Patch 3 creates the value in repo-settings as well as ensures that the
   repo settings have been prepared before accessing the value within
   replace_refs_enabled().

Thanks, -Stolee

Derrick Stolee (3):
  repository: create disable_replace_refs()
  replace-objects: create wrapper around setting
  repository: create read_replace_refs setting

 builtin/cat-file.c       |  2 +-
 builtin/commit-graph.c   |  5 ++---
 builtin/fsck.c           |  2 +-
 builtin/index-pack.c     |  2 +-
 builtin/pack-objects.c   |  3 +--
 builtin/prune.c          |  3 ++-
 builtin/replace.c        |  3 ++-
 builtin/unpack-objects.c |  3 +--
 builtin/upload-pack.c    |  3 ++-
 commit-graph.c           |  4 +---
 config.c                 |  5 -----
 environment.c            |  3 +--
 git.c                    |  2 +-
 log-tree.c               |  2 +-
 replace-object.c         | 23 ++++++++++++++++++++++-
 replace-object.h         | 31 ++++++++++++++++++++++---------
 repo-settings.c          |  2 ++
 repository.h             |  8 ++++++++
 18 files changed, 71 insertions(+), 35 deletions(-)

base-commit: b0afdce5dab61f224fd66c13768facc36a7f8705
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1537%2Fderrickstolee%2Freplace-refs-safety-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1537/derrickstolee/replace-refs-safety-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1537