Subject: Re: [PATCH 01/19] dir.c: optionally compute sha-1 of a .gitignore file

From: Junio C Hamano <[email-masked]>
Date: Mon, 27 Oct 2014 15:46:03 -0700

Nguyễn Thái Ngọc Duy  <[email-masked]> writes:

> This is not used anywhere yet. But the goal is to compare quickly if a
> .gitignore file has changed when we have the SHA-1 of both old (cached
> somewhere) and new (from index or a tree) versions.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
> ---
>  dir.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
>  dir.h |  5 +++++
>  2 files changed, 48 insertions(+), 7 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index bd274a7..33a35c1 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -466,7 +466,8 @@ void add_exclude(const char *string, const char *base,
>  	x->el = el;
>  }
>  
> -static void *read_skip_worktree_file_from_index(const char *path, size_t *size)
> +static void *read_skip_worktree_file_from_index(const char *path, size_t *size,
> +						struct sha1_stat *ss)
>  {
>  	int pos, len;
>  	unsigned long sz;
> @@ -485,6 +486,10 @@ static void *read_skip_worktree_file_from_index(const char *path, size_t *size)
>  		return NULL;
>  	}
>  	*size = xsize_t(sz);
> +	if (ss) {
> +		memset(&ss->stat, 0, sizeof(ss->stat));
> +		hashcpy(ss->sha1, active_cache[pos]->sha1);
> +	}
>  	return data;
>  }
>  
> @@ -529,11 +534,18 @@ static void trim_trailing_spaces(char *buf)
>  		*last_space = '\0';
>  }
>  
> -int add_excludes_from_file_to_list(const char *fname,
> -				   const char *base,
> -				   int baselen,
> -				   struct exclude_list *el,
> -				   int check_index)
> +/*
> + * Given a file with name "fname", read it (either from disk, or from
> + * the index if "check_index" is non-zero), parse it and store the
> + * exclude rules in "el".
> + *
> + * If "ss" is not NULL, compute SHA-1 of the exclude file and fill
> + * stat data from disk (only valid if add_excludes returns zero). If
> + * ss_valid is non-zero, "ss" must contain good value as input.
> + */

Hmm, do we want a separate parameter for that?  Wouldn't it be
sufficient and cleaner to pass is_null_sha1(ss->sha1[]), or
alternatively have an element ss->valid that is initialized to
false?

That is...

> @@ -571,6 +587,19 @@ int add_excludes_from_file_to_list(const char *fname,
>  		}
>  		buf[size++] = '\n';
>  		close(fd);
> +		if (ss) {
> +			int pos;
> +			if (ss_valid && !match_stat_data(&ss->stat, &st))

s/ss_valid/ss->valid/;

> +				; /* no content change, ss->sha1 still good */
> +			else if (check_index &&
> +				 (pos = cache_name_pos(fname, strlen(fname))) >= 0 &&
> +				 !ce_stage(active_cache[pos]) &&
> +				 ce_uptodate(active_cache[pos]))
> +				hashcpy(ss->sha1, active_cache[pos]->sha1);
> +			else
> +				hash_sha1_file(buf, size, "blob", ss->sha1);
> +			fill_stat_data(&ss->stat, &st);

And toggle "ss->valid = 1"; here...

> @@ -589,6 +618,13 @@ int add_excludes_from_file_to_list(const char *fname,
>  	return 0;
>  }
>  
> +int add_excludes_from_file_to_list(const char *fname, const char *base,
> +				   int baselen, struct exclude_list *el,
> +				   int check_index)
> +{
> +	return add_excludes(fname, base, baselen, el, check_index, NULL, 0);
> +}
> +
>  struct exclude_list *add_exclude_list(struct dir_struct *dir,
>  				      int group_type, const char *src)
>  {
> diff --git a/dir.h b/dir.h
> index 6c45e9d..032d197 100644
> --- a/dir.h
> +++ b/dir.h
> @@ -73,6 +73,11 @@ struct exclude_list_group {
>  	struct exclude_list *el;
>  };
>  
> +struct sha1_stat {
> +	struct stat_data stat;
> +	unsigned char sha1[20];
> +};
> +
>  struct dir_struct {
>  	int nr, alloc;
>  	int ignored_nr, ignored_alloc;