On 05/06/18 16:43, Nguyễn Thái Ngọc Duy wrote:
> Prior to fba92be8f7, this code implicitly (and incorrectly) assumes
> the_index when running the exclude machinery. fba92be8f7 helps show
> this problem clearer because unpack-trees operation is supposed to
> work on whatever index the caller specifies... not specifically
> the_index.
> 
> Update the code to use "istate" argument that's originally from
> mark_new_skip_worktree(). From the call sites, both in unpack_trees(),
> you can see that this function works on two separate indexes:
> o->src_index and o->result. The second mark_new_skip_worktree() so far
> has incorecctly applied exclude rules on o->src_index instead of
> o->result. It's unclear what is the consequences of this, but it's
> definitely wrong.
> 
> [1] fba92be8f7 (dir: convert is_excluded_from_list to take an index -
>     2017-05-05)
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
> ---
>  unpack-trees.c | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 60d1138e08..5268de7af5 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1085,13 +1085,15 @@ static int unpack_callback(int n, unsigned long mask, 
> unsigned long dirmask, str
>       return mask;
>  }
>  
> -static int clear_ce_flags_1(struct cache_entry **cache, int nr,
> +static int clear_ce_flags_1(struct index_state *istate,
> +                         struct cache_entry **cache, int nr,
>                           struct strbuf *prefix,
>                           int select_mask, int clear_mask,
>                           struct exclude_list *el, int defval);
>  
>  /* Whole directory matching */
> -static int clear_ce_flags_dir(struct cache_entry **cache, int nr,
> +static int clear_ce_flags_dir(struct index_state *istate,
> +                           struct cache_entry **cache, int nr,
>                             struct strbuf *prefix,
>                             char *basename,
>                             int select_mask, int clear_mask,
> @@ -1100,7 +1102,7 @@ static int clear_ce_flags_dir(struct cache_entry 
> **cache, int nr,
>       struct cache_entry **cache_end;
>       int dtype = DT_DIR;
>       int ret = is_excluded_from_list(prefix->buf, prefix->len,
> -                                     basename, &dtype, el, &the_index);
> +                                     basename, &dtype, el, istate);
>       int rc;
>  
>       strbuf_addch(prefix, '/');
> @@ -1122,7 +1124,7 @@ static int clear_ce_flags_dir(struct cache_entry 
> **cache, int nr,
>        * calling clear_ce_flags_1(). That function will call
>        * the expensive is_excluded_from_list() on every entry.
>        */
> -     rc = clear_ce_flags_1(cache, cache_end - cache,
> +     rc = clear_ce_flags_1(istate, cache, cache_end - cache,
>                             prefix,
>                             select_mask, clear_mask,
>                             el, ret);
> @@ -1145,7 +1147,8 @@ static int clear_ce_flags_dir(struct cache_entry 
> **cache, int nr,
>   *   cache[0]->name[0..(prefix_len-1)]
>   * Top level path has prefix_len zero.
>   */
> -static int clear_ce_flags_1(struct cache_entry **cache, int nr,
> +static int clear_ce_flags_1(struct index_state *istate,
> +                         struct cache_entry **cache, int nr,
>                           struct strbuf *prefix,
>                           int select_mask, int clear_mask,
>                           struct exclude_list *el, int defval)
> @@ -1179,7 +1182,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, 
> int nr,
>                       len = slash - name;
>                       strbuf_add(prefix, name, len);
>  
> -                     processed = clear_ce_flags_dir(cache, cache_end - cache,
> +                     processed = clear_ce_flags_dir(istate, cache, cache_end 
> - cache,
>                                                      prefix,
>                                                      prefix->buf + 
> prefix->len - len,
>                                                      select_mask, clear_mask,
> @@ -1193,7 +1196,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, 
> int nr,
>                       }
>  
>                       strbuf_addch(prefix, '/');
> -                     cache += clear_ce_flags_1(cache, cache_end - cache,
> +                     cache += clear_ce_flags_1(istate, cache, cache_end - 
> cache,
>                                                 prefix,
>                                                 select_mask, clear_mask, el, 
> defval);
>                       strbuf_setlen(prefix, prefix->len - len - 1);
> @@ -1203,7 +1206,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, 
> int nr,
>               /* Non-directory */
>               dtype = ce_to_dtype(ce);
>               ret = is_excluded_from_list(ce->name, ce_namelen(ce),
> -                                         name, &dtype, el, &the_index);
> +                                         name, &dtype, el, istate);
>               if (ret < 0)
>                       ret = defval;
>               if (ret > 0)
> @@ -1213,15 +1216,17 @@ static int clear_ce_flags_1(struct cache_entry 
> **cache, int nr,
>       return nr - (cache_end - cache);
>  }
>  
> -static int clear_ce_flags(struct cache_entry **cache, int nr,
> -                         int select_mask, int clear_mask,
> -                         struct exclude_list *el)
> +static int clear_ce_flags(struct index_state *istate,
> +                       int select_mask, int clear_mask,
> +                       struct exclude_list *el)

OK, so that last hunk of patch #3 should be moved to the
end of this patch. (A 'git rebase -i' problem?)

ATB,
Ramsay Jones

Reply via email to