From: Junio C Hamano <gitster@pobox.com>
To: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Jeff Hostetler <git@jeffhostetler.com>,
Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 2/4] split-index; stop abusing the `base_oid` to strip the "link" extension
Date: Wed, 22 Mar 2023 14:24:34 -0700 [thread overview]
Message-ID: <xmqqjzz8cwdp.fsf@gitster.g> (raw)
In-Reply-To: <f1897b880729b649ab24da14cbc3432d44b7c731.1679500859.git.gitgitgadget@gmail.com> (Johannes Schindelin via GitGitGadget's message of "Wed, 22 Mar 2023 16:00:57 +0000")
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When a split-index is in effect, the `$GIT_DIR/index` file needs to
> contain a "link" extension that contains all the information about the
> split-index, including the information about the shared index.
> ...
> Let's stop zeroing out the `base_oid` to indicate that the "link"
> extension should not be written.
Nicely explained.
> One might be tempted to simply call `discard_split_index()` instead,
> under the assumption that Git decided to write a non-split index and
> therefore the the `split_index` structure might no longer be wanted.
"the the".
> +enum strip_extensions {
> + WRITE_ALL_EXTENSIONS = 0,
> + STRIP_ALL_EXTENSIONS = 1,
> + STRIP_LINK_EXTENSION_ONLY = 2
> +};
We do not need to spell out the specific values for this enum; the
users' (i.e. the callers of do_write_index()) sole requirement is
for these symbols to have different values.
Also do we envision that (1) we would need to keep STRIP_LINK_ONLY
to be with the largest value among the enum values, or (2) we would
never add new value to the set? Otherwise let's end the last one
with a trailing comma.
Looking at the way strip_extensions variable is used in
do_write_index(), an alternative design might be to make it a set of
bits (e.g. unsigned write_extension) and give one bit to each
extension. But such a clean-up is better left outside the topic, I
would imagine, as we do not have any need to skip an arbitrary set
of extensions right now.
> +/*
> + * Write the Git index into a `.lock` file
> + *
> + * If `strip_link_extension` is non-zero, avoid writing any "link" extension
> + * (used by the split-index feature).
> + */
Not exposing "enum strip_extensions" to the caller of this function,
like this patch does, is probably a very safe and sensible thing to
do. We do not have a reason to allow its callers to (perhaps
mistakenly) pass STRIP_ALL_EXTENSIONS to it.
> static int do_write_locked_index(struct index_state *istate, struct lock_file *lock,
> - unsigned flags)
> + unsigned flags, int strip_link_extension)
> {
> int ret;
> int was_full = istate->sparse_index == INDEX_EXPANDED;
> @@ -3185,7 +3197,7 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l
> */
> trace2_region_enter_printf("index", "do_write_index", the_repository,
> "%s", get_lock_file_path(lock));
> - ret = do_write_index(istate, lock->tempfile, 0, flags);
> + ret = do_write_index(istate, lock->tempfile, strip_link_extension ? STRIP_LINK_EXTENSION_ONLY : 0, flags);
> trace2_region_leave_printf("index", "do_write_index", the_repository,
> "%s", get_lock_file_path(lock));
>
OK.
Very nicely done.
next prev parent reply other threads:[~2023-03-22 21:24 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-22 16:00 [PATCH 0/4] Fix a few split-index bugs Johannes Schindelin via GitGitGadget
2023-03-22 16:00 ` [PATCH 1/4] split-index & fsmonitor: demonstrate a bug Johannes Schindelin via GitGitGadget
2023-03-23 13:39 ` Jeff Hostetler
2023-03-22 16:00 ` [PATCH 2/4] split-index; stop abusing the `base_oid` to strip the "link" extension Johannes Schindelin via GitGitGadget
2023-03-22 21:24 ` Junio C Hamano [this message]
2023-03-23 13:59 ` Jeff Hostetler
2023-03-23 16:06 ` Junio C Hamano
2023-03-23 15:22 ` Jeff Hostetler
2023-03-22 16:00 ` [PATCH 3/4] fsmonitor: avoid overriding `cache_changed` bits Johannes Schindelin via GitGitGadget
2023-03-22 16:00 ` [PATCH 4/4] unpack-trees: take care to propagate the split-index flag Johannes Schindelin via GitGitGadget
2023-03-23 14:32 ` Jeff Hostetler
2023-03-22 16:22 ` [PATCH 0/4] Fix a few split-index bugs Junio C Hamano
2023-03-26 22:45 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2023-03-26 22:45 ` [PATCH v2 1/4] split-index & fsmonitor: demonstrate a bug Johannes Schindelin via GitGitGadget
2023-03-26 22:45 ` [PATCH v2 2/4] split-index; stop abusing the `base_oid` to strip the "link" extension Johannes Schindelin via GitGitGadget
2023-03-26 22:45 ` [PATCH v2 3/4] fsmonitor: avoid overriding `cache_changed` bits Johannes Schindelin via GitGitGadget
2023-03-26 22:45 ` [PATCH v2 4/4] unpack-trees: take care to propagate the split-index flag Johannes Schindelin via GitGitGadget
2023-03-27 14:48 ` [PATCH v2 0/4] Fix a few split-index bugs Jeff Hostetler
2023-03-27 16:42 ` 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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqjzz8cwdp.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@jeffhostetler.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=johannes.schindelin@gmx.de \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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).