For review: git add --ignore-unmatch

For review: git add --ignore-unmatch

[Luke-Jr]

[-- Attachment #1: Type: Text/Plain, Size: 1 bytes --]



[-- Attachment #2: 0001-port-ignore-unmatch-from-git-rm-to-git-add.patch --]
[-- Type: text/x-patch, Size: 2009 bytes --]

From 54768360aa7b1882dd2b76211661abb6014cbf23 Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Wed, 12 Aug 2009 16:26:31 -0500
Subject: [PATCH 1/3] port --ignore-unmatch from "git rm" to "git add"

---
 builtin-add.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 581a2a1..0597fb9 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -19,6 +19,7 @@ static const char * const builtin_add_usage[] = {
 };
 static int patch_interactive, add_interactive, edit_interactive;
 static int take_worktree_changes;
+static int ignore_unmatch = 0;
 
 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);
@@ -226,6 +227,8 @@ static struct option builtin_add_options[] = {
 	OPT_BOOLEAN('A', "all", &addremove, "add all, noticing removal of tracked files"),
 	OPT_BOOLEAN( 0 , "refresh", &refresh_only, "don't add, only refresh the index"),
 	OPT_BOOLEAN( 0 , "ignore-errors", &ignore_add_errors, "just skip files which cannot be added because of errors"),
+	OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,
+				"exit with a zero status even if nothing matched"),
 	OPT_END(),
 };
 
-- 
1.6.3.3


[-- Attachment #3: 0002-fix-git-add-ignore-errors-to-ignore-pathspec-errors.patch --]
[-- Type: text/x-patch, Size: 761 bytes --]

From c6cd06db8ab3b198eafffd5b0e94d2f338f10072 Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Wed, 12 Aug 2009 16:31:37 -0500
Subject: [PATCH 2/3] fix "git add --ignore-errors" to ignore pathspec errors

---
 builtin-add.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 0597fb9..e3132c8 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -280,6 +280,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		add_interactive = 1;
 	if (add_interactive)
 		exit(interactive_add(argc - 1, argv + 1, prefix));
+	if (ignore_add_errors)
+		ignore_unmatch = 1;
 
 	if (edit_interactive)
 		return(edit_patch(argc, argv, prefix));
-- 
1.6.3.3


[-- Attachment #4: 0003-Document-ignore-unmatch-in-git-add.txt.patch --]
[-- Type: text/x-patch, Size: 1217 bytes --]

From 410a93cb61669304a0b1d10b8013d1635432e8a0 Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Wed, 12 Aug 2009 17:23:44 -0500
Subject: [PATCH 3/3] Document --ignore-unmatch in git-add.txt

---
 Documentation/git-add.txt |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index ab1943c..6b93b4e 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 [verse]
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
 	  [--edit | -e] [--all | [--update | -u]] [--intent-to-add | -N]
-	  [--refresh] [--ignore-errors] [--] <filepattern>...
+	  [--refresh] [--ignore-errors] [--ignore-unmatch] [--]
+	  <filepattern>...
 
 DESCRIPTION
 -----------
@@ -119,6 +120,9 @@ apply.
 	them, do not abort the operation, but continue adding the
 	others. The command shall still exit with non-zero status.
 
+--ignore-unmatch::
+	Exit with a zero status even if no files matched.
+
 \--::
 	This option can be used to separate command-line options from
 	the list of files, (useful when filenames might be mistaken
-- 
1.6.3.3

Re: For review: git add --ignore-unmatch

[Thomas Rast]

Luke-Jr wrote:
> 
> [0001-port-ignore-unmatch-from-git-rm-to-git-add.patch]
> [0002-fix-git-add-ignore-errors-to-ignore-pathspec-errors.patch]
> [0003-Document-ignore-unmatch-in-git-add.txt.patch]

I've already told you on IRC how to go about this, so please:

* send one patch per mail, inline
* write real commit messages
* sign off
* write tests

[Yeah, I'm tired and grumpy, but I'm also trying to save others the
work of saying this again more politely...]

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

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

[Luke Dashjr]

"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.

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 builtin-add.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 581a2a1..0597fb9 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -19,6 +19,7 @@ static const char * const builtin_add_usage[] = {
 };
 static int patch_interactive, add_interactive, edit_interactive;
 static int take_worktree_changes;
+static int ignore_unmatch = 0;
 
 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);
@@ -226,6 +227,8 @@ static struct option builtin_add_options[] = {
 	OPT_BOOLEAN('A', "all", &addremove, "add all, noticing removal of tracked files"),
 	OPT_BOOLEAN( 0 , "refresh", &refresh_only, "don't add, only refresh the index"),
 	OPT_BOOLEAN( 0 , "ignore-errors", &ignore_add_errors, "just skip files which cannot be added because of errors"),
+	OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,
+				"exit with a zero status even if nothing matched"),
 	OPT_END(),
 };
 
-- 
1.6.3.3

[PATCH 2/5] fix "git add --ignore-errors" to ignore pathspec errors

[Luke Dashjr]

Unmatched files are errors, and should be ignored with the rest of them.

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 builtin-add.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 0597fb9..e3132c8 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -280,6 +280,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		add_interactive = 1;
 	if (add_interactive)
 		exit(interactive_add(argc - 1, argv + 1, prefix));
+	if (ignore_add_errors)
+		ignore_unmatch = 1;
 
 	if (edit_interactive)
 		return(edit_patch(argc, argv, prefix));
-- 
1.6.3.3

[PATCH 3/5] Document --ignore-unmatch in git-add.txt

[Luke Dashjr]

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 Documentation/git-add.txt |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index ab1943c..6b93b4e 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 [verse]
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
 	  [--edit | -e] [--all | [--update | -u]] [--intent-to-add | -N]
-	  [--refresh] [--ignore-errors] [--] <filepattern>...
+	  [--refresh] [--ignore-errors] [--ignore-unmatch] [--]
+	  <filepattern>...
 
 DESCRIPTION
 -----------
@@ -119,6 +120,9 @@ apply.
 	them, do not abort the operation, but continue adding the
 	others. The command shall still exit with non-zero status.
 
+--ignore-unmatch::
+	Exit with a zero status even if no files matched.
+
 \--::
 	This option can be used to separate command-line options from
 	the list of files, (useful when filenames might be mistaken
-- 
1.6.3.3

[PATCH 4/5] implement error_errno and warning_errno

[Luke Dashjr]

This allows for easier conversion of code that currently does a (fatal)
die_errno to a safer error (which can be ignored), or perhaps even warning
status.

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 git-compat-util.h |    2 +
 usage.c           |   66 ++++++++++++++++++++++++-----------------------------
 2 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 9f941e4..25d4c1e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -181,7 +181,9 @@ extern void usage(const char *err) NORETURN;
 extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
 extern void die_errno(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
 extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
 
 extern void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN);
 
diff --git a/usage.c b/usage.c
index b6aea45..dce90dc 100644
--- a/usage.c
+++ b/usage.c
@@ -39,7 +39,7 @@ static void warn_builtin(const char *warn, va_list params)
 static void (*usage_routine)(const char *err) NORETURN = usage_builtin;
 static void (*die_routine)(const char *err, va_list params) NORETURN = die_builtin;
 static void (*error_routine)(const char *err, va_list params) = error_builtin;
-static void (*warn_routine)(const char *err, va_list params) = warn_builtin;
+static void (*warning_routine)(const char *err, va_list params) = warn_builtin;
 
 void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN)
 {
@@ -51,19 +51,8 @@ void usage(const char *err)
 	usage_routine(err);
 }
 
-void die(const char *err, ...)
+static void _e_errno(const char *fmt, char *fmt_with_err, size_t sizeof_fmt_with_err)
 {
-	va_list params;
-
-	va_start(params, err);
-	die_routine(err, params);
-	va_end(params);
-}
-
-void die_errno(const char *fmt, ...)
-{
-	va_list params;
-	char fmt_with_err[1024];
 	char str_error[256], *err;
 	int i, j;
 
@@ -81,28 +70,33 @@ void die_errno(const char *fmt, ...)
 		}
 	}
 	str_error[j] = 0;
-	snprintf(fmt_with_err, sizeof(fmt_with_err), "%s: %s", fmt, str_error);
-
-	va_start(params, fmt);
-	die_routine(fmt_with_err, params);
-	va_end(params);
-}
-
-int error(const char *err, ...)
-{
-	va_list params;
-
-	va_start(params, err);
-	error_routine(err, params);
-	va_end(params);
-	return -1;
+	snprintf(fmt_with_err, sizeof_fmt_with_err, "%s: %s", fmt, str_error);
 }
 
-void warning(const char *warn, ...)
-{
-	va_list params;
-
-	va_start(params, warn);
-	warn_routine(warn, params);
-	va_end(params);
-}
+#define BUILD_E(RTYPE, NAME, CODE)  \
+RTYPE NAME(const char *err, ...)  \
+{  \
+	va_list params;  \
+  \
+	va_start(params, err);  \
+	NAME ## _routine(err, params);  \
+	va_end(params);  \
+	CODE  \
+}  \
+  \
+RTYPE NAME ## _errno(const char *fmt, ...)  \
+{  \
+	va_list params;  \
+	static char fmt_with_err[1024];  \
+  \
+	va_start(params, fmt);  \
+	_e_errno(fmt, fmt_with_err, sizeof(fmt_with_err));  \
+	NAME ## _routine(fmt_with_err, params);  \
+	va_end(params);  \
+	CODE  \
+}  \
+// END OF BUILD_E MACRO
+
+BUILD_E(void, die, )
+BUILD_E(int, error, return -1;)
+BUILD_E(void, warning, )
-- 
1.6.3.3

[PATCH 5/5] Convert add_file_to_index's lstat failure from a die to an error

[Luke Dashjr]

In order to make --ignore-errors able to ignore a lstat failure in
add_file_to_index, it needs to raise an error, NOT a fatal die.

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 read-cache.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 4e3e272..074e6b8 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -638,7 +638,7 @@ int add_file_to_index(struct index_state *istate, const char *path, int flags)
 {
 	struct stat st;
 	if (lstat(path, &st))
-		die_errno("unable to stat '%s'", path);
+		return error_errno("unable to stat '%s'", path);
 	return add_to_index(istate, path, &st, flags);
 }
 
-- 
1.6.3.3

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

[Junio C Hamano]

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.
>
> Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>

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".

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.

>  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?

 builtin-add.c |   25 ++++++++++++++++++-------
 1 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 581a2a1..49576b4 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -41,16 +41,25 @@ static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
 	}
 }
 
+static char *alloc_seen(const char **pathspec, int *specs_)
+{
+	int specs;
+
+	if (ignore_unmatch)
+		return NULL;
+	for (specs = 0; pathspec[specs];  specs++)
+		; /* nothing */
+	*specs_ = specs;
+	return xcalloc(specs, 1);
+}
+
 static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
 {
 	char *seen;
 	int i, specs;
 	struct dir_entry **src, **dst;
 
-	for (specs = 0; pathspec[specs];  specs++)
-		/* nothing */;
-	seen = xcalloc(specs, 1);
-
+	seen = alloc_seen(pathspec, &specs);
 	src = dst = dir->entries;
 	i = dir->nr;
 	while (--i >= 0) {
@@ -60,6 +69,8 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
 			*dst++ = entry;
 	}
 	dir->nr = dst - dir->entries;
+	if (!seen)
+		return;
 	fill_pathspec_matches(pathspec, seen, specs);
 
 	for (i = 0; i < specs; i++) {
@@ -102,11 +113,11 @@ static void refresh(int verbose, const char **pathspec)
 	char *seen;
 	int i, specs;
 
-	for (specs = 0; pathspec[specs];  specs++)
-		/* nothing */;
-	seen = xcalloc(specs, 1);
+	seen = alloc_seen(pathspec, &specs);
 	refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED : REFRESH_QUIET,
 		      pathspec, seen);
+	if (!seen)
+		return;
 	for (i = 0; i < specs; i++) {
 		if (!seen[i])
 			die("pathspec '%s' did not match any files", pathspec[i]);

Re: [PATCH 2/5] fix "git add --ignore-errors" to ignore pathspec errors

[Junio C Hamano]

Luke Dashjr <luke-jr+git@utopios.org> writes:

> Unmatched files are errors, and should be ignored with the rest of them.

Why is this a "fix"?

I would understand if it were "Make --ignore-errors imply --ignore-unmatch
unconditionally".  But then I do not think I would necessarily agree it is
a good change.

The user may know that some files in the work tree are unreadable and
cannot be indexed (hence he gives --ignore-errors) but he still may want
to catch a typo on the command line.

I do not think it is wise to make --ignore-errors imply --ignore-unmatch
unconditionally like this patch does without any escape hatch.

> Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
> ---
>  builtin-add.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-add.c b/builtin-add.c
> index 0597fb9..e3132c8 100644
> --- a/builtin-add.c
> +++ b/builtin-add.c
> @@ -280,6 +280,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  		add_interactive = 1;
>  	if (add_interactive)
>  		exit(interactive_add(argc - 1, argv + 1, prefix));
> +	if (ignore_add_errors)
> +		ignore_unmatch = 1;
>  
>  	if (edit_interactive)
>  		return(edit_patch(argc, argv, prefix));
> -- 
> 1.6.3.3

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

[Luke-Jr]

On Thursday 13 August 2009 02:36:13 pm Junio C Hamano wrote:
> 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.

> 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.

> >  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)...

Re: [PATCH 2/5] fix "git add --ignore-errors" to ignore pathspec errors

[Luke-Jr]

On Thursday 13 August 2009 02:38:57 pm Junio C Hamano wrote:
> Luke Dashjr <luke-jr+git@utopios.org> writes:
> > Unmatched files are errors, and should be ignored with the rest of them.
>
> Why is this a "fix"?
>
> I would understand if it were "Make --ignore-errors imply --ignore-unmatch
> unconditionally".  But then I do not think I would necessarily agree it is
> a good change.
>
> The user may know that some files in the work tree are unreadable and
> cannot be indexed (hence he gives --ignore-errors) but he still may want
> to catch a typo on the command line.
>
> I do not think it is wise to make --ignore-errors imply --ignore-unmatch
> unconditionally like this patch does without any escape hatch.

Are unmatched files not errors? Perhaps the old flag should be renamed to
--ignore-read-errors and a new --ignore-errors that implies both added. Or 
maybe just a documentation change to preserve compatibility with anything that 
might assume that...

[PATCH] port --ignore-unmatch to "git add"

[Luke Dashjr]

"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.

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 builtin-add.c |   27 +++++++++++++++++++--------
 1 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 581a2a1..3882482 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -19,6 +19,7 @@ static const char * const builtin_add_usage[] = {
 };
 static int patch_interactive, add_interactive, edit_interactive;
 static int take_worktree_changes;
+static int ignore_unmatch = 0;
 
 static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
 {
@@ -60,13 +61,18 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
 			*dst++ = entry;
 	}
 	dir->nr = dst - dir->entries;
-	fill_pathspec_matches(pathspec, seen, specs);
 
-	for (i = 0; i < specs; i++) {
-		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]))
-			die("pathspec '%s' did not match any files",
-					pathspec[i]);
+	if (!ignore_unmatch)
+	{
+		fill_pathspec_matches(pathspec, seen, specs);
+
+		for (i = 0; i < specs; i++) {
+			if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]))
+				die("pathspec '%s' did not match any files",
+						pathspec[i]);
+		}
 	}
+
         free(seen);
 }
 
@@ -107,9 +113,12 @@ static void refresh(int verbose, const char **pathspec)
 	seen = xcalloc(specs, 1);
 	refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED : REFRESH_QUIET,
 		      pathspec, seen);
-	for (i = 0; i < specs; i++) {
-		if (!seen[i])
-			die("pathspec '%s' did not match any files", pathspec[i]);
+	if (!ignore_unmatch)
+	{
+		for (i = 0; i < specs; i++) {
+			if (!seen[i])
+				die("pathspec '%s' did not match any files", pathspec[i]);
+		}
 	}
         free(seen);
 }
@@ -226,6 +235,8 @@ static struct option builtin_add_options[] = {
 	OPT_BOOLEAN('A', "all", &addremove, "add all, noticing removal of tracked files"),
 	OPT_BOOLEAN( 0 , "refresh", &refresh_only, "don't add, only refresh the index"),
 	OPT_BOOLEAN( 0 , "ignore-errors", &ignore_add_errors, "just skip files which cannot be added because of errors"),
+	OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,
+				"exit with a zero status even if nothing matched"),
 	OPT_END(),
 };
 
-- 
1.6.3.3

[PATCH] Document --ignore-unmatch in git-add.txt

[Luke Dashjr]

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 Documentation/git-add.txt |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index e67b7e8..6e30ee9 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 [verse]
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
 	  [--edit | -e] [--all | [--update | -u]] [--intent-to-add | -N]
-	  [--refresh] [--ignore-errors] [--] [<filepattern>...]
+	  [--refresh] [--ignore-errors] [--ignore-unmatch] [--]
+	  [<filepattern>...]
 
 DESCRIPTION
 -----------
@@ -119,6 +120,9 @@ apply.
 	them, do not abort the operation, but continue adding the
 	others. The command shall still exit with non-zero status.
 
+--ignore-unmatch::
+	Exit with a zero status even if no files matched.
+
 \--::
 	This option can be used to separate command-line options from
 	the list of files, (useful when filenames might be mistaken
-- 
1.6.3.3

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

[Thomas Rast]

Junio C Hamano wrote:
> 
> Chould you refresh my memory a bit?
> 
> In what circumstance is "rm --ignore-unmatch" useful to begin with?

Not sure about add --ignore-unmatch myself, but there's even an
example of rm --ignore-unmatch in man git-filter-branch, along the
lines of

  git filter-branch --index-filter '
    rm --ignore-unmach some_file_that_shouldnt_be_in_history
  ' -- --all

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

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

[Alex Riesen]

On Thu, Aug 13, 2009 at 22:40, Luke-Jr<luke@dashjr.org> wrote:
> On Thursday 13 August 2009 02:36:13 pm Junio C Hamano wrote:
>> 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.

Yes. For statics.

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

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> Junio C Hamano wrote:
>> 
>> Chould you refresh my memory a bit?
>> 
>> In what circumstance is "rm --ignore-unmatch" useful to begin with?
>
> Not sure about add --ignore-unmatch myself, but there's even an
> example of rm --ignore-unmatch in man git-filter-branch, along the
> lines of
>
>   git filter-branch --index-filter '
>     rm --ignore-unmach some_file_that_shouldnt_be_in_history
>   ' -- --all

Ah, that makes sense.  I am not sure about "add --ignore-unmatch" myself
either, and an example similar to the above filter-branch would not apply
very easily (i.e. "add a file that should have been in history" would not
need --ignore-unmatch).

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

[Luke-Jr]

On Friday 14 August 2009 02:52:33 pm Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> > Junio C Hamano wrote:
> >> Chould you refresh my memory a bit?
> >>
> >> In what circumstance is "rm --ignore-unmatch" useful to begin with?
> >
> > Not sure about add --ignore-unmatch myself, but there's even an
> > example of rm --ignore-unmatch in man git-filter-branch, along the
> > lines of
> >
> >   git filter-branch --index-filter '
> >     rm --ignore-unmach some_file_that_shouldnt_be_in_history
> >   ' -- --all
>
> Ah, that makes sense.  I am not sure about "add --ignore-unmatch" myself
> either, and an example similar to the above filter-branch would not apply
> very easily (i.e. "add a file that should have been in history" would not
> need --ignore-unmatch).

The purpose of "add --ignore-unmatch" is to ignore race conditions where one 
of the files to be added has been deleted after git is executed, but before 
git scans it.

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

[Luke-Jr]

Specific use-case of "git add --ignore-unmatched":
	http://gitorious.org/gitbackup

Without this functionality, daily race conditions will break the 
synchronisation.

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

[Nanako Shiraishi]

Quoting "Luke-Jr" <luke@dashjr.org>

> On Friday 14 August 2009 02:52:33 pm Junio C Hamano wrote:
>> Thomas Rast <trast@student.ethz.ch> writes:
>> > Junio C Hamano wrote:
>> >> Chould you refresh my memory a bit?
>> >>
>> >> In what circumstance is "rm --ignore-unmatch" useful to begin with?
>> >
>> > Not sure about add --ignore-unmatch myself, but there's even an
>> > example of rm --ignore-unmatch in man git-filter-branch, along the
>> > lines of
>> >
>> >   git filter-branch --index-filter '
>> >     rm --ignore-unmach some_file_that_shouldnt_be_in_history
>> >   ' -- --all
>>
>> Ah, that makes sense.  I am not sure about "add --ignore-unmatch" myself
>> either, and an example similar to the above filter-branch would not apply
>> very easily (i.e. "add a file that should have been in history" would not
>> need --ignore-unmatch).
>
> The purpose of "add --ignore-unmatch" is to ignore race conditions where one 
> of the files to be added has been deleted after git is executed, but before 
> git scans it.

First of all, it should have been mentioned as part of your proposed commit log message.

Second of all, if a race condition makes an "add" fail, isn't it a good thing? If your "add" ignored such a failure, you'd be recording an inconsistent or incomplete state.

IMHO, fixing your racy script is a much cleaner solution to your problem than forcing "git add" to ignore errors.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

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

[Luke-Jr]

On Friday 14 August 2009 04:39:58 pm Nanako Shiraishi wrote:
> Second of all, if a race condition makes an "add" fail, isn't it a good
> thing? If your "add" ignored such a failure, you'd be recording an
> inconsistent or incomplete state.

If the file doesn't exist, it is no longer in the current state.

> IMHO, fixing your racy script is a much cleaner solution to your problem
> than forcing "git add" to ignore errors.

In this situation, the race condition is not a bug, it is normal operation.

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

[Luke-Jr]

Just for reference, since these fixes+functions have not yet been merged to 
mainline Git, I have pushed a fork with them rebased on up-to-date tags:
	http://repo.or.cz/w/git/ljr.git