From: Derrick Stolee <derrickstolee@github.com>
To: Victoria Dye <vdye@github.com>,
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org
Cc: me@ttaylorr.com, newren@gmail.com, gitster@pobox.com
Subject: Re: [PATCH 3/3] repository: create read_replace_refs setting
Date: Thu, 1 Jun 2023 15:52:40 -0400 [thread overview]
Message-ID: <31962d5e-b14f-86b1-8428-de29650d89ee@github.com> (raw)
In-Reply-To: <68fd5ca2-a6c2-d742-5d74-ba93c1497169@github.com>
On 6/1/2023 12:36 PM, Victoria Dye wrote:
> Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <derrickstolee@github.com>
>> Unfortunately, the existing processes that recurse into submodules do
>> not appear to follow object IDs to their contents, so this behavior
>> change is not visible in the current implementation. It is something
>> valuable for future behavior changes.
>
> AFAIK, the only '--recurse-submodules' commands that recurse in-process are
> 'ls-files' and 'grep'. However, 'grep' does call 'parse_object_or_die()',
> which (further down in the call stack) calls 'lookup_replace_object()'.
> Maybe I'm misreading and the replaced object isn't actually used, but could
> 'git grep --recurse-submodules' be used to test this?
You're right. I was laser-focused on 'ls-files', but it shouldn't be hard
to construct an example where 'git grep --recurse-submodules' would show
different behavior when this config is toggled.
>> + /*
>> + * Do replace refs need to be checked this run? This variable is
>> + * initialized to true unless --no-replace-object is used or
>> + * $GIT_NO_REPLACE_OBJECTS is set, but is set to false by some
>> + * commands that do not want replace references to be active.
>> + */
>> + int read_replace_refs;
>
> I don't think this comment is accurate anymore, since the repo-scoped
> 'read_replace_refs' value is determined *only* by the 'core.usereplacerefs'
> config. It's 'replace_refs_enabled()' that makes the overall determination
> (taking into account 'GIT_NO_REPLACE_OBJECTS'/'--no-replace-object').
Thank you for catching my mistake here. I'll be sure to update it in v2.
Thanks,
-Stolee
next prev parent reply other threads:[~2023-06-01 19:52 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 [this message]
2023-05-31 5:11 ` [PATCH 0/3] Create stronger guard rails on replace refs Elijah Newren
2023-06-02 14:29 ` [PATCH v2 " 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=31962d5e-b14f-86b1-8428-de29650d89ee@github.com \
--to=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=me@ttaylorr.com \
--cc=newren@gmail.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).