Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Derrick Stolee <derrickstolee@github.com>,
	Elijah Newren <newren@gmail.com>,
	Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, vdye@github.com, me@ttaylorr.com
Subject: Re: [PATCH 1/3] repository: create disable_replace_refs()
Date: Sat, 03 Jun 2023 09:28:13 +0900	[thread overview]
Message-ID: <xmqqa5xh8k4i.fsf@gitster.g> (raw)
In-Reply-To: <20230601174746.GA4165405@coredump.intra.peff.net> (Jeff King's message of "Thu, 1 Jun 2023 13:47:46 -0400")

Jeff King <peff@peff.net> writes:

>> Generally, I decided to move it after option-parsing, so it wouldn't
>> be called if we are hitting an option-parse error.
>
> Playing devil's advocate: would option parsing ever access an object? I
> think in most cases the answer is no, but I could imagine it happening
> for some special cases (e.g., update-index uses callbacks to act on
> options as we parse them, since order is important).
>
> So I think as a general principle it makes sense for commands to set
> this flag as early as possible.

I agree with the "devil's advocate" above; indeed my suggestion to
follow-on work that is enabled by introducing this function, i.e. we
can ensure that the objects already instantiated when the call is
made are not affected by the replace mechanism, was exactly for such
a "we already accessed some objects via the replace mechanism and
then try to close the door of the barn afterwards with this call"
case.

Indeed, I think "git branch --no-merged 0369cf" resolves the object
name down to a commit object in parse_opt_merge_filter() before
parse_options() call returns.

Yes.

  reply	other threads:[~2023-06-03  0:28 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 [this message]
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 ` [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=xmqqa5xh8k4i.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --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).