Re: [PATCH 2/7] sparse-index: update command for expand/collapse test

2 messages, 2 authors, 2021-09-30 · open the first message on its own page

Re: [PATCH 2/7] sparse-index: update command for expand/collapse test

From: Junio C Hamano <hidden>
Date: 2021-09-30 21:32:45

Victoria Dye [off-list ref] writes:
Taylor Blau wrote:
quoted
On Thu, Sep 30, 2021 at 02:50:56PM +0000, Victoria Dye via GitGitGadget wrote:
quoted
From: Victoria Dye <redacted>

In anticipation of multiple commands being fully integrated with sparse
index, update the test for index expand and collapse for non-sparse index
integrated commands to use `mv`.
...
 	GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
-		git -C sparse-index -c core.fsmonitor="" reset --hard &&
+		git -C sparse-index -c core.fsmonitor="" mv a b &&
Double-checking my understanding as somebody who is not so familiar with
t1092: this test is to ensure that commands which don't yet understand
the sparse index can temporarily expand it in order to do their work?
Exactly - if a command doesn't explicitly enable use of the sparse index by
setting `command_requires_full_index` to 0, the index is expanded if/when it
is first read during the command's execution and collapsed if/when it is
written to disk. This test makes sure that mechanism works as intended.
Sorry, I do not quite follow.  

So is this "before this series of patches, 'reset --hard' can be
used to as a sample of a command that expands and then collapses,
but because it no longer is a good sample of a command so we replace
it with 'mv a b'"?  Do we need to update this further when "mv a b"
learns to expand and then collapse?

Re: [PATCH 2/7] sparse-index: update command for expand/collapse test

From: Victoria Dye <hidden>
Date: 2021-09-30 23:00:08

Junio C Hamano wrote:
Victoria Dye [off-list ref] writes:
quoted
Taylor Blau wrote:
quoted
On Thu, Sep 30, 2021 at 02:50:56PM +0000, Victoria Dye via GitGitGadget wrote:
quoted
From: Victoria Dye <redacted>

In anticipation of multiple commands being fully integrated with sparse
index, update the test for index expand and collapse for non-sparse index
integrated commands to use `mv`.
...
 	GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
-		git -C sparse-index -c core.fsmonitor="" reset --hard &&
+		git -C sparse-index -c core.fsmonitor="" mv a b &&
Double-checking my understanding as somebody who is not so familiar with
t1092: this test is to ensure that commands which don't yet understand
the sparse index can temporarily expand it in order to do their work?
Exactly - if a command doesn't explicitly enable use of the sparse index by
setting `command_requires_full_index` to 0, the index is expanded if/when it
is first read during the command's execution and collapsed if/when it is
written to disk. This test makes sure that mechanism works as intended.
Sorry, I do not quite follow.  

So is this "before this series of patches, 'reset --hard' can be
used to as a sample of a command that expands and then collapses,
but because it no longer is a good sample of a command so we replace
it with 'mv a b'"?
Yes, because this series enables sparse index integration in `git reset`,
the test no longer applies to that command (but it does apply to `git mv`).
Do we need to update this further when "mv a b"
learns to expand and then collapse?
Unfortunately, yes. `git mv` was picked more-or-less at random from the set
of commands that read the index and don't already have sparse index
integrations (excluding those I know are planned for sparse index
integration in the near future). If `git mv` were to be updated to disable
`command_requires_full_index`, the command in the test would need to change
again.

For what it's worth, I do think the test itself is valuable, since it makes
sure a command's capability to use the sparse index is always the result of
an intentional update to (and review of) the code.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help