Re: [PATCH 1/5] port --ignore-unmatch to "git add"

From: Luke-Jr <luke@dashjr.org>
Date: Aug 13, 2009, 20:40 UTC
Message-ID: <200908131540.49701.luke@dashjr.org>
In-Reply-To: <7vy6pna4lu.fsf@alter.siamese.dyndns.org>

On Thursday 13 August 2009 02:36:13 pm Junio C Hamano wrote:

Show 9 quoted lines

> Luke Dashjr <luke-jr+git@utopios.org> writes:
> > "git rm" has a --ignore-unmatch option that is also applicable to "git
> > add" and may be useful for persons wanting to ignore unmatched arguments,
> > but not all errors.
>
> Chould you refresh my memory a bit?
>
> In what circumstance is "rm --ignore-unmatch" useful to begin with?
> A similar question for "add --ignore-unmatch".

Not sure on its purpose for "rm", but for "add"... Avoiding a race condition in automation. In particular, if the file is deleted between the time the argument list is built until git scans for matches.

Show 6 quoted lines

> Now the obligatory design level question is behind us, let's take a brief
> look at the codde.
>
> > +static int ignore_unmatch = 0;
>
> Drop " = 0" and let the language initialize this to zero.

Does C define a default initialisation of zero? My understanding was that uninitialised variables are always undefined until explicitly assigned a value.

Show 26 quoted lines

> >  static void fill_pathspec_matches(const char **pathspec, char *seen, int
> > specs) {
> > @@ -63,7 +64,7 @@ static void prune_directory(struct dir_struct *dir,
> > const char **pathspec, int p fill_pathspec_matches(pathspec, seen,
> > specs);
> >
> >  	for (i = 0; i < specs; i++) {
> > -		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]))
> > +		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]) &&
> > !ignore_unmatch) die("pathspec '%s' did not match any files",
> >  					pathspec[i]);
> >  	}
> > @@ -108,7 +109,7 @@ static void refresh(int verbose, const char
> > **pathspec) refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED :
> > REFRESH_QUIET, pathspec, seen);
> >  	for (i = 0; i < specs; i++) {
> > -		if (!seen[i])
> > +		if (!seen[i] && !ignore_unmatch)
> >  			die("pathspec '%s' did not match any files", pathspec[i]);
> >  	}
> >          free(seen);
>
> What's the point of these two loops if under ignore_unmatch everything
> becomes no-op?
>
> That is, wouldn't it be much more clear if you wrote like this?

I'm not overly familiar with the git codebase, but wouldn't a null 'seen' variable break the refresh_index call? The loops themselves can be avoided, I suppose. I'll submit a new patch to optimise the changes (and rebase)...