Re: [PATCH v3 4/4] fsmonitor: Delay updating state until after split index is merged

[Alex Vandiver]

On Tue, 31 Oct 2017, Junio C Hamano wrote:
> This makes local variable "int i;" in this function unused and gets
> compiler warning.

Apologies for leaving that detritus -- I saw you added a 'SQUASH??' commit
to deal with it, which LGTM.

On Tue, 31 Oct 2017, Johannes Schindelin wrote:
> ... to which end we introduced the DEVELOPER flag to catch these: if you
> call
> 
>   make DEVELOPER=1

Aha!  Thanks for the tip; I'll be sure to use that from now on.
 - Alex

Re: [PATCH v3 4/4] fsmonitor: Delay updating state until after split index is merged

[Johannes Schindelin]

Hi,

On Tue, 31 Oct 2017, Junio C Hamano wrote:

> Alex Vandiver  writes:
> 
> > diff --git a/fsmonitor.c b/fsmonitor.c
> > index 4ea44dcc6..417759224 100644
> > --- a/fsmonitor.c
> > +++ b/fsmonitor.c
> > @@ -49,20 +49,7 @@ int read_fsmonitor_extension(struct index_state *istate, 
> > const void *data,
> > ewah_free(fsmonitor_dirty);
> > return error("failed to parse ewah bitmap reading fsmonitor 
> > index extension");
> > }
> > -
> > -   if (git_config_get_fsmonitor()) {
> > -   /* Mark all entries valid */
> > -   for (i = 0; i < istate->cache_nr; i++)
> > -   istate->cache[i]->ce_flags |= CE_FSMONITOR_VALID;
> > -
> > -   /* Mark all previously saved entries as dirty */
> > -   ewah_each_bit(fsmonitor_dirty, fsmonitor_ewah_callback, istate);
> > -
> > -   /* Now mark the untracked cache for fsmonitor usage */
> > -   if (istate->untracked)
> > -   istate->untracked->use_fsmonitor = 1;
> > -   }
> > -   ewah_free(fsmonitor_dirty);
> > +   istate->fsmonitor_dirty = fsmonitor_dirty;
> 
> This makes local variable "int i;" in this function unused and gets
> compiler warning.

... to which end we introduced the DEVELOPER flag to catch these: if you
call

make DEVELOPER=1

and compile with GCC or Clang, it will elevate such warnings to errors,
and we highly encourage contributors to build their patched source code
with said flag.

Thanks,
Johannes

Re: [PATCH v3 4/4] fsmonitor: Delay updating state until after split index is merged

[Junio C Hamano]

Alex Vandiver  writes:

> diff --git a/fsmonitor.c b/fsmonitor.c
> index 4ea44dcc6..417759224 100644
> --- a/fsmonitor.c
> +++ b/fsmonitor.c
> @@ -49,20 +49,7 @@ int read_fsmonitor_extension(struct index_state *istate, 
> const void *data,
>   ewah_free(fsmonitor_dirty);
>   return error("failed to parse ewah bitmap reading fsmonitor 
> index extension");
>   }
> -
> - if (git_config_get_fsmonitor()) {
> - /* Mark all entries valid */
> - for (i = 0; i < istate->cache_nr; i++)
> - istate->cache[i]->ce_flags |= CE_FSMONITOR_VALID;
> -
> - /* Mark all previously saved entries as dirty */
> - ewah_each_bit(fsmonitor_dirty, fsmonitor_ewah_callback, istate);
> -
> - /* Now mark the untracked cache for fsmonitor usage */
> - if (istate->untracked)
> - istate->untracked->use_fsmonitor = 1;
> - }
> - ewah_free(fsmonitor_dirty);
> + istate->fsmonitor_dirty = fsmonitor_dirty;

This makes local variable "int i;" in this function unused and gets
compiler warning.

[PATCH v3 4/4] fsmonitor: Delay updating state until after split index is merged

[Alex Vandiver]

If the fsmonitor extension is used in conjunction with the split index
extension, the set of entries in the index when it is first loaded is
only a subset of the real index.  This leads to only the non-"base"
index being marked as CE_FSMONITOR_VALID.

Delay the expansion of the ewah bitmap until after tweak_split_index
has been called to merge in the base index as well.

The new fsmonitor_dirty is kept from being leaked by dint of being
cleaned up in post_read_index_from, which is guaranteed to be called
after do_read_index in read_index_from.

Signed-off-by: Alex Vandiver 
---
 cache.h |  1 +
 fsmonitor.c | 39 ---
 2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/cache.h b/cache.h
index 25adcf681..0a4f43ec2 100644
--- a/cache.h
+++ b/cache.h
@@ -348,6 +348,7 @@ struct index_state {
unsigned char sha1[20];
struct untracked_cache *untracked;
uint64_t fsmonitor_last_update;
+   struct ewah_bitmap *fsmonitor_dirty;
 };
 
 extern struct index_state the_index;
diff --git a/fsmonitor.c b/fsmonitor.c
index 4ea44dcc6..417759224 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -49,20 +49,7 @@ int read_fsmonitor_extension(struct index_state *istate, 
const void *data,
ewah_free(fsmonitor_dirty);
return error("failed to parse ewah bitmap reading fsmonitor 
index extension");
}
-
-   if (git_config_get_fsmonitor()) {
-   /* Mark all entries valid */
-   for (i = 0; i < istate->cache_nr; i++)
-   istate->cache[i]->ce_flags |= CE_FSMONITOR_VALID;
-
-   /* Mark all previously saved entries as dirty */
-   ewah_each_bit(fsmonitor_dirty, fsmonitor_ewah_callback, istate);
-
-   /* Now mark the untracked cache for fsmonitor usage */
-   if (istate->untracked)
-   istate->untracked->use_fsmonitor = 1;
-   }
-   ewah_free(fsmonitor_dirty);
+   istate->fsmonitor_dirty = fsmonitor_dirty;
 
trace_printf_key(_fsmonitor, "read fsmonitor extension 
successful");
return 0;
@@ -239,7 +226,29 @@ void remove_fsmonitor(struct index_state *istate)
 
 void tweak_fsmonitor(struct index_state *istate)
 {
-   switch (git_config_get_fsmonitor()) {
+   int i;
+   int fsmonitor_enabled = git_config_get_fsmonitor();
+
+   if (istate->fsmonitor_dirty) {
+   if (fsmonitor_enabled) {
+   /* Mark all entries valid */
+   for (i = 0; i < istate->cache_nr; i++) {
+   istate->cache[i]->ce_flags |= 
CE_FSMONITOR_VALID;
+   }
+
+   /* Mark all previously saved entries as dirty */
+   ewah_each_bit(istate->fsmonitor_dirty, 
fsmonitor_ewah_callback, istate);
+
+   /* Now mark the untracked cache for fsmonitor usage */
+   if (istate->untracked)
+   istate->untracked->use_fsmonitor = 1;
+   }
+
+   ewah_free(istate->fsmonitor_dirty);
+   istate->fsmonitor_dirty = NULL;
+   }
+
+   switch (fsmonitor_enabled) {
case -1: /* keep: do nothing */
break;
case 0: /* false */
-- 
2.15.0.rc1.413.g76aedb451