From: Elijah Newren <newren@gmail.com>
To: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, vdye@github.com, me@ttaylorr.com,
gitster@pobox.com, Derrick Stolee <derrickstolee@github.com>
Subject: Re: [PATCH 0/3] Create stronger guard rails on replace refs
Date: Tue, 30 May 2023 22:11:57 -0700 [thread overview]
Message-ID: <CABPp-BFj0+3y7C4FKy9qzVjHWP2r6-=azW8g0j3WGX-zYpaCQQ@mail.gmail.com> (raw)
In-Reply-To: <pull.1537.git.1685126617.gitgitgadget@gmail.com>
On Fri, May 26, 2023 at 11:43 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> (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 for implementing this. I had a few questions on the first
patch (though I think one of them was answered by noting that you have
both a global and a repository setting for the flag), but otherwise it
looks good.
next prev parent reply other threads:[~2023-05-31 5:12 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-26 18:43 [PATCH 0/3] Create stronger guard rails on replace refs Derrick Stolee via GitGitGadget
2023-05-26 18:43 ` [PATCH 1/3] repository: create disable_replace_refs() Derrick Stolee via GitGitGadget
2023-05-31 4:41 ` Elijah Newren
2023-05-31 13:37 ` Derrick Stolee
2023-06-01 17:47 ` Jeff King
2023-06-03 0:28 ` Junio C Hamano
2023-06-04 6:32 ` Jeff King
2023-06-01 5:23 ` Junio C Hamano
2023-06-01 16:35 ` Victoria Dye
2023-05-26 18:43 ` [PATCH 2/3] replace-objects: create wrapper around setting Derrick Stolee via GitGitGadget
2023-06-01 16:35 ` Victoria Dye
2023-06-01 19:50 ` Derrick Stolee
2023-06-03 1:47 ` Elijah Newren
2023-06-05 15:44 ` Derrick Stolee
2023-05-26 18:43 ` [PATCH 3/3] repository: create read_replace_refs setting Derrick Stolee via GitGitGadget
2023-06-01 16:36 ` Victoria Dye
2023-06-01 19:52 ` Derrick Stolee
2023-05-31 5:11 ` Elijah Newren [this message]
2023-06-02 14:29 ` [PATCH v2 0/3] Create stronger guard rails on replace refs Derrick Stolee via GitGitGadget
2023-06-02 14:29 ` [PATCH v2 1/3] repository: create disable_replace_refs() Derrick Stolee via GitGitGadget
2023-06-02 14:29 ` [PATCH v2 2/3] replace-objects: create wrapper around setting Derrick Stolee via GitGitGadget
2023-06-03 6:22 ` René Scharfe
2023-06-05 13:22 ` Derrick Stolee
2023-06-02 14:29 ` [PATCH v2 3/3] repository: create read_replace_refs setting Derrick Stolee via GitGitGadget
2023-06-05 19:32 ` Victoria Dye
2023-06-03 1:52 ` [PATCH v2 0/3] Create stronger guard rails on replace refs Elijah Newren
2023-06-06 13:24 ` [PATCH v3 " Derrick Stolee via GitGitGadget
2023-06-06 13:24 ` [PATCH v3 1/3] repository: create disable_replace_refs() Derrick Stolee via GitGitGadget
2023-06-06 13:24 ` [PATCH v3 2/3] replace-objects: create wrapper around setting Derrick Stolee via GitGitGadget
2023-06-06 13:24 ` [PATCH v3 3/3] repository: create read_replace_refs setting Derrick Stolee via GitGitGadget
2023-06-06 16:15 ` [PATCH v3 0/3] Create stronger guard rails on replace refs Victoria Dye
2023-06-12 20:45 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CABPp-BFj0+3y7C4FKy9qzVjHWP2r6-=azW8g0j3WGX-zYpaCQQ@mail.gmail.com' \
--to=newren@gmail.com \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=me@ttaylorr.com \
--cc=vdye@github.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).