On 3/22/23 5:24 PM, Junio C Hamano wrote:
"Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
writes:
From: Johannes Schindelin <johannes.schindelin@xxxxxx>
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.
There are several calls to do_write_locked_index() that pass 0 or 1
as the new final arg. If we update them to use these enum values,
then we don't need integer values here.
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.
Agreed, I thought about suggesting a set of bits too, but right now
we only need to strip all of them or just this one.
+/*
+ * 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);
In the else of the ?: operator, could we use the WRITE_ALL_EXTENSIONS
instead of 0?
trace2_region_leave_printf("index", "do_write_index", the_repository,
"%s", get_lock_file_path(lock));
OK.
Very nicely done.
