On 9/18/2017 9:32 AM, David Turner wrote:
-----Original Message-----
From: Ben Peart [mailto:peartben@xxxxxxxxx]
Sent: Monday, September 18, 2017 9:07 AM
To: David Turner <David.Turner@xxxxxxxxxxxx>; 'Ben Peart'
<benpeart@xxxxxxxxxxxxx>
Cc: avarab@xxxxxxxxx; christian.couder@xxxxxxxxx; git@xxxxxxxxxxxxxxx;
gitster@xxxxxxxxx; johannes.schindelin@xxxxxx; pclouds@xxxxxxxxx;
peff@xxxxxxxx
Subject: Re: [PATCH v6 04/12] fsmonitor: teach git to optionally utilize a file
system monitor to speed up detecting new or changed files.
Thanks for taking the time to review/provide feedback!
On 9/15/2017 5:35 PM, David Turner wrote:
-----Original Message-----
From: Ben Peart [mailto:benpeart@xxxxxxxxxxxxx]
Sent: Friday, September 15, 2017 3:21 PM
To: benpeart@xxxxxxxxxxxxx
Cc: David Turner <David.Turner@xxxxxxxxxxxx>; avarab@xxxxxxxxx;
christian.couder@xxxxxxxxx; git@xxxxxxxxxxxxxxx; gitster@xxxxxxxxx;
johannes.schindelin@xxxxxx; pclouds@xxxxxxxxx; peff@xxxxxxxx
Subject: [PATCH v6 04/12] fsmonitor: teach git to optionally utilize
a file system monitor to speed up detecting new or changed files.
+int git_config_get_fsmonitor(void)
+{
+ if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor))
+ core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
+
+ if (core_fsmonitor && !*core_fsmonitor)
+ core_fsmonitor = NULL;
+
+ if (core_fsmonitor)
+ return 1;
+
+ return 0;
+}
This functions return values are backwards relative to the rest of the
git_config_* functions.
I'm confused. If core.fsmonitor is configured, it returns 1. If it is not
configured, it returns 0. I don't make use of the -1 /* default value */ option
as I didn't see any use/value in this case. What is backwards?
The other git_config_* functions return 1 for error and 0 for success.
Hmm, I followed the model (ie copy/paste :)) used by the tracked cache.
If you look at how it uses, the return value, it is 0 == false == remove
the extension, 1 == true == add the extension. I'm doing the same with
fsmonitor.
static void tweak_untracked_cache(struct index_state *istate)
{
switch (git_config_get_untracked_cache()) {
case -1: /* keep: do nothing */
break;
case 0: /* false */
remove_untracked_cache(istate);
break;
case 1: /* true */
add_untracked_cache(istate);
break;
default: /* unknown value: do nothing */
break;
}
}
void tweak_fsmonitor(struct index_state *istate)
{
switch (git_config_get_fsmonitor()) {
case -1: /* keep: do nothing */
break;
case 0: /* false */
remove_fsmonitor(istate);
break;
case 1: /* true */
add_fsmonitor(istate);
break;
default: /* unknown value: do nothing */
break;
}
}
[snip]
+> /*
+> * With fsmonitor, we can trust the untracked cache's valid field.
+> */
Did you intend to make a comment here?
Sorry. I was going to make a comment that I didn't see how that could work
since we weren't touching the untracked cache here, but then I saw the bit
further down. I'm still not sure it works (see comment on 10/12), but at
least it could in theory work.
The logic here assumes that when the index is written out, it is valid
including the untracked cache and the CE_FSMONITOR_VALID bits.
Therefore it should still all be valid as of the time the fsmonitor was
queried and the index saved.
Another way of thinking about this is that the fsmonitor extension is
simply adding another persisted bit on each index entry. It just gets
persisted/restored differently than the other persisted bits.
Obviously, before we can use it assuming it reflects the *current* state
of the working directory, we have to refresh the bits via the refresh logic.
