[BUG] git add -u ignores --dry-run flag

[BUG] git add -u ignores --dry-run flag

[Gustaf Hendeby]

Hi!

I think this is a bug in the built in implementation of git add -u

$ git --version
git version 1.5.5.1.373.ga3200
$ git init
Initialized empty Git repository in /home/hendeby/bar/.git/
$ echo foo > foo
$ git add foo && git commit -m "Test"
Created initial commit 7477e8b: Test
  1 files changed, 1 insertions(+), 0 deletions(-)
  create mode 100644 foo
$ echo foo >> foo
$ git add -u --dry-run
$ git status
# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       modified:   foo
#

I was expecting to get foo listed, and not actually added to the index.

I've had a quick look in builtin-add.c, and the -u option is tested for 
before the --dry-run option, and in the process of handling the -u 
option the code makes a jump that bypasses the --dry-run handling. 
Unfortunately, I'm not familiar enough with the code to see how to best 
fix it.

/Gustaf

[PATCH] Make git add -u honor --dry-run

[Miklos Vajna]

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Thu, May 15, 2008 at 06:08:24PM +0200, Gustaf Hendeby <hendeby@isy.liu.se> wrote:
> I'm not familiar enough with the code to see how to best fix it.

Something like this?

 builtin-add.c         |    3 ++-
 t/t2200-add-update.sh |    7 +++++++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 4a91e3e..222497d 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -212,7 +212,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		if (read_cache() < 0)
 			die("index file corrupt");
 		pathspec = get_pathspec(prefix, argv);
-		add_files_to_cache(verbose, prefix, pathspec);
+		if(!show_only)
+			add_files_to_cache(verbose, prefix, pathspec);
 		goto finish;
 	}
 
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index b664341..13ad975 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -88,6 +88,13 @@ test_expect_success 'replace a file with a symlink' '
 
 '
 
+test_expect_success 'add everything changed with --dry-run' '
+
+	git add -u --dry-run &&
+	test -n "$(git diff-files)"
+
+'
+
 test_expect_success 'add everything changed' '
 
 	git add -u &&
-- 
1.5.5.1.211.g65ea3.dirty

Re: [PATCH] Make git add -u honor --dry-run

[Gustaf Hendeby]

On 2008-05-15 18:20, Miklos Vajna wrote:
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
> 
> On Thu, May 15, 2008 at 06:08:24PM +0200, Gustaf Hendeby <hendeby@isy.liu.se> wrote:
>> I'm not familiar enough with the code to see how to best fix it.
> 
> Something like this?

This fixes part of the problem.  Nothing gets written to the index now, 
however, I get no list of what files would have been added.  That is 
what I would have suspected.  Am I reading the docs incorrectly?

> 
>  builtin-add.c         |    3 ++-
>  t/t2200-add-update.sh |    7 +++++++
>  2 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/builtin-add.c b/builtin-add.c
> index 4a91e3e..222497d 100644
> --- a/builtin-add.c
> +++ b/builtin-add.c
> @@ -212,7 +212,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  		if (read_cache() < 0)
>  			die("index file corrupt");
>  		pathspec = get_pathspec(prefix, argv);
> -		add_files_to_cache(verbose, prefix, pathspec);
> +		if(!show_only)
> +			add_files_to_cache(verbose, prefix, pathspec);
>  		goto finish;
>  	}
Since the other code path for show_only, does not end up in finish but 
returns 0 directly, I'm assuming the same could be done here (after 
printing the changed files) to save some cycles.

>  
> diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
> index b664341..13ad975 100755
> --- a/t/t2200-add-update.sh
> +++ b/t/t2200-add-update.sh
> @@ -88,6 +88,13 @@ test_expect_success 'replace a file with a symlink' '
>  
>  '
>  
> +test_expect_success 'add everything changed with --dry-run' '
> +
> +	git add -u --dry-run &&
> +	test -n "$(git diff-files)"
Don't you need to validate the output from git add -u --dry-run too?

/Gustaf

> +
> +'
> +
>  test_expect_success 'add everything changed' '
>  
>  	git add -u &&

Re* [PATCH] Make git add -u honor --dry-run

[Junio C Hamano]

Miklos Vajna <vmiklos@frugalware.org> writes:

> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>
> On Thu, May 15, 2008 at 06:08:24PM +0200, Gustaf Hendeby <hendeby@isy.liu.se> wrote:
>> I'm not familiar enough with the code to see how to best fix it.
>
> Something like this?
>
>  builtin-add.c         |    3 ++-
>  t/t2200-add-update.sh |    7 +++++++
>  2 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-add.c b/builtin-add.c
> index 4a91e3e..222497d 100644
> --- a/builtin-add.c
> +++ b/builtin-add.c
> @@ -212,7 +212,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  		if (read_cache() < 0)
>  			die("index file corrupt");
>  		pathspec = get_pathspec(prefix, argv);
> -		add_files_to_cache(verbose, prefix, pathspec);
> +		if(!show_only)
> +			add_files_to_cache(verbose, prefix, pathspec);
>  		goto finish;
>  	}

That makes the whole thing noop, doesn't it?  We could do the surgery at a
bit lower layer, I guess.


 cache.h          |    8 +++++---
 builtin-add.c    |   22 +++++++++++++---------
 builtin-commit.c |    2 +-
 builtin-mv.c     |    2 +-
 read-cache.c     |   20 ++++++++++++++++----
 5 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/cache.h b/cache.h
index 98cfed6..cf30e53 100644
--- a/cache.h
+++ b/cache.h
@@ -173,7 +173,7 @@ extern struct index_state the_index;
 #define add_cache_entry(ce, option) add_index_entry(&the_index, (ce), (option))
 #define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))
 #define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))
-#define add_file_to_cache(path, verbose) add_file_to_index(&the_index, (path), (verbose))
+#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags))
 #define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL)
 #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
 #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
@@ -272,7 +272,9 @@ extern int add_index_entry(struct index_state *, struct cache_entry *ce, int opt
 extern struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
 extern int remove_index_entry_at(struct index_state *, int pos);
 extern int remove_file_from_index(struct index_state *, const char *path);
-extern int add_file_to_index(struct index_state *, const char *path, int verbose);
+#define ADD_CACHE_VERBOSE 1
+#define ADD_CACHE_PRETEND 2
+extern int add_file_to_index(struct index_state *, const char *path, int flags);
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
 extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
 
@@ -640,7 +642,7 @@ extern int convert_to_git(const char *path, const char *src, size_t len, struct
 extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);
 
 /* add */
-void add_files_to_cache(int verbose, const char *prefix, const char **pathspec);
+void add_files_to_cache(const char *prefix, const char **pathspec, int flags);
 
 /* diff.c */
 extern int diff_auto_refresh_index;
diff --git a/builtin-add.c b/builtin-add.c
index 4a91e3e..cb61366 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -82,9 +82,9 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec,
 static void update_callback(struct diff_queue_struct *q,
 			    struct diff_options *opt, void *cbdata)
 {
-	int i, verbose;
+	int i, flags;
 
-	verbose = *((int *)cbdata);
+	flags = *((int *)cbdata);
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
 		const char *path = p->one->path;
@@ -94,18 +94,19 @@ static void update_callback(struct diff_queue_struct *q,
 		case DIFF_STATUS_UNMERGED:
 		case DIFF_STATUS_MODIFIED:
 		case DIFF_STATUS_TYPE_CHANGED:
-			add_file_to_cache(path, verbose);
+			add_file_to_cache(path, flags);
 			break;
 		case DIFF_STATUS_DELETED:
-			remove_file_from_cache(path);
-			if (verbose)
+			if (!(flags & ADD_CACHE_PRETEND))
+				remove_file_from_cache(path);
+			if (flags)
 				printf("remove '%s'\n", path);
 			break;
 		}
 	}
 }
 
-void add_files_to_cache(int verbose, const char *prefix, const char **pathspec)
+void add_files_to_cache(const char *prefix, const char **pathspec, int flags)
 {
 	struct rev_info rev;
 	init_revisions(&rev, prefix);
@@ -113,7 +114,7 @@ void add_files_to_cache(int verbose, const char *prefix, const char **pathspec)
 	rev.prune_data = pathspec;
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
-	rev.diffopt.format_callback_data = &verbose;
+	rev.diffopt.format_callback_data = &flags;
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
 }
 
@@ -209,10 +210,13 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	if (take_worktree_changes) {
 		const char **pathspec;
+		int flags = ((verbose ? ADD_CACHE_VERBOSE : 0) |
+			     (show_only ? ADD_CACHE_PRETEND : 0));
+
 		if (read_cache() < 0)
 			die("index file corrupt");
 		pathspec = get_pathspec(prefix, argv);
-		add_files_to_cache(verbose, prefix, pathspec);
+		add_files_to_cache(prefix, pathspec, flags);
 		goto finish;
 	}
 
@@ -254,7 +258,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	}
 
 	for (i = 0; i < dir.nr; i++)
-		add_file_to_cache(dir.entries[i]->name, verbose);
+		add_file_to_cache(dir.entries[i]->name, ADD_CACHE_VERBOSE);
 
  finish:
 	if (active_cache_changed) {
diff --git a/builtin-commit.c b/builtin-commit.c
index 2f4d6cc..f31bf59 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -239,7 +239,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
 	 */
 	if (all || (also && pathspec && *pathspec)) {
 		int fd = hold_locked_index(&index_lock, 1);
-		add_files_to_cache(0, also ? prefix : NULL, pathspec);
+		add_files_to_cache(also ? prefix : NULL, pathspec, 0);
 		refresh_cache(REFRESH_QUIET);
 		if (write_cache(fd, active_cache, active_nr) ||
 		    close_lock_file(&index_lock))
diff --git a/builtin-mv.c b/builtin-mv.c
index 990e213..1ad3178 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -256,7 +256,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 
 		for (i = 0; i < added.nr; i++) {
 			const char *path = added.items[i].path;
-			add_file_to_cache(path, verbose);
+			add_file_to_cache(path, verbose ? ADD_CACHE_VERBOSE : 0);
 		}
 
 		for (i = 0; i < deleted.nr; i++)
diff --git a/read-cache.c b/read-cache.c
index 7db5588..a95861b 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -384,12 +384,14 @@ static int index_name_pos_also_unmerged(struct index_state *istate,
 	return pos;
 }
 
-int add_file_to_index(struct index_state *istate, const char *path, int verbose)
+int add_file_to_index(struct index_state *istate, const char *path, int flags)
 {
-	int size, namelen, pos;
+	int size, namelen, pos, was_same;
 	struct stat st;
 	struct cache_entry *ce;
 	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_RACY_IS_DIRTY;
+	int verbose = flags & (ADD_CACHE_VERBOSE|ADD_CACHE_PRETEND);
+	int pretend = flags & ADD_CACHE_PRETEND;
 
 	if (lstat(path, &st))
 		die("%s: unable to stat (%s)", path, strerror(errno));
@@ -432,9 +434,19 @@ int add_file_to_index(struct index_state *istate, const char *path, int verbose)
 
 	if (index_path(ce->sha1, path, &st, 1))
 		die("unable to index file %s", path);
-	if (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
+
+	/* It was suspected to be racily clean but it turns out to be Ok */
+	was_same = (0 <= pos &&
+		    !ce_stage(istate->cache[pos]) &&
+		    !hashcmp(ce->sha1, istate->cache[pos]->sha1) &&
+		    ce->ce_mode == istate->cache[pos]->ce_mode);
+
+	if (pretend)
+		;
+	else if (add_index_entry(istate, ce,
+				 ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
 		die("unable to add %s to index",path);
-	if (verbose)
+	if (verbose && !was_same)
 		printf("add '%s'\n", path);
 	return 0;
 }

Re: Re* [PATCH] Make git add -u honor --dry-run

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 305 bytes --]

On Thu, May 15, 2008 at 04:42:50PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> That makes the whole thing noop, doesn't it?

Exactly. I just considered git add -u --dry-run doing anything a bug.
Then Gustaf pointed out that it would be nice to print the to-be-added
files. Just forget my patch. :)

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Re* [PATCH] Make git add -u honor --dry-run

[Gustaf Hendeby]

On 2008-05-16 01:42, Junio C Hamano wrote:
> Miklos Vajna <vmiklos@frugalware.org> writes:
>> On Thu, May 15, 2008 at 06:08:24PM +0200, Gustaf Hendeby <hendeby@isy.liu.se> wrote:
>>> I'm not familiar enough with the code to see how to best fix it.
> 
> That makes the whole thing noop, doesn't it?  We could do the surgery at a
> bit lower layer, I guess.

What is the status on this one?  I've been away for a while and when I 
got back I didn't see any more than this suggestion.  I'm really in over 
my head here, but I think (judging from a compiler warning) the 
following needs to be amended:

diff --git a/builtin-checkout.c b/builtin-checkout.c
index cf9875c..39fef84 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -282,7 +282,7 @@ static int merge_working_tree(struct checkout_opts 
*opts,
                          * entries in the index.
                          */

-                       add_files_to_cache(0, NULL, NULL);
+                       add_files_to_cache(NULL, NULL, 0);
                         work = write_tree_from_memory();

                         ret = reset_to_new(new->commit->tree, opts->quiet);


I had started to think about a much less intrusive solution limited to 
builtin-add.c but, without really understanding all that is going on 
here, this seems to be a more flexible and better solution for the long run.

/Gustaf

> 
> 
>  cache.h          |    8 +++++---
>  builtin-add.c    |   22 +++++++++++++---------
>  builtin-commit.c |    2 +-
>  builtin-mv.c     |    2 +-
>  read-cache.c     |   20 ++++++++++++++++----
>  5 files changed, 36 insertions(+), 18 deletions(-)
> 
> diff --git a/cache.h b/cache.h
> index 98cfed6..cf30e53 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -173,7 +173,7 @@ extern struct index_state the_index;
>  #define add_cache_entry(ce, option) add_index_entry(&the_index, (ce), (option))
>  #define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))
>  #define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))
> -#define add_file_to_cache(path, verbose) add_file_to_index(&the_index, (path), (verbose))
> +#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags))
>  #define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL)
>  #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
>  #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
> @@ -272,7 +272,9 @@ extern int add_index_entry(struct index_state *, struct cache_entry *ce, int opt
>  extern struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
>  extern int remove_index_entry_at(struct index_state *, int pos);
>  extern int remove_file_from_index(struct index_state *, const char *path);
> -extern int add_file_to_index(struct index_state *, const char *path, int verbose);
> +#define ADD_CACHE_VERBOSE 1
> +#define ADD_CACHE_PRETEND 2
> +extern int add_file_to_index(struct index_state *, const char *path, int flags);
>  extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
>  extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
>  
> @@ -640,7 +642,7 @@ extern int convert_to_git(const char *path, const char *src, size_t len, struct
>  extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);
>  
>  /* add */
> -void add_files_to_cache(int verbose, const char *prefix, const char **pathspec);
> +void add_files_to_cache(const char *prefix, const char **pathspec, int flags);
>  
>  /* diff.c */
>  extern int diff_auto_refresh_index;
> diff --git a/builtin-add.c b/builtin-add.c
> index 4a91e3e..cb61366 100644
> --- a/builtin-add.c
> +++ b/builtin-add.c
> @@ -82,9 +82,9 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec,
>  static void update_callback(struct diff_queue_struct *q,
>  			    struct diff_options *opt, void *cbdata)
>  {
> -	int i, verbose;
> +	int i, flags;
>  
> -	verbose = *((int *)cbdata);
> +	flags = *((int *)cbdata);
>  	for (i = 0; i < q->nr; i++) {
>  		struct diff_filepair *p = q->queue[i];
>  		const char *path = p->one->path;
> @@ -94,18 +94,19 @@ static void update_callback(struct diff_queue_struct *q,
>  		case DIFF_STATUS_UNMERGED:
>  		case DIFF_STATUS_MODIFIED:
>  		case DIFF_STATUS_TYPE_CHANGED:
> -			add_file_to_cache(path, verbose);
> +			add_file_to_cache(path, flags);
>  			break;
>  		case DIFF_STATUS_DELETED:
> -			remove_file_from_cache(path);
> -			if (verbose)
> +			if (!(flags & ADD_CACHE_PRETEND))
> +				remove_file_from_cache(path);
> +			if (flags)
>  				printf("remove '%s'\n", path);
>  			break;
>  		}
>  	}
>  }
>  
> -void add_files_to_cache(int verbose, const char *prefix, const char **pathspec)
> +void add_files_to_cache(const char *prefix, const char **pathspec, int flags)
>  {
>  	struct rev_info rev;
>  	init_revisions(&rev, prefix);
> @@ -113,7 +114,7 @@ void add_files_to_cache(int verbose, const char *prefix, const char **pathspec)
>  	rev.prune_data = pathspec;
>  	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
>  	rev.diffopt.format_callback = update_callback;
> -	rev.diffopt.format_callback_data = &verbose;
> +	rev.diffopt.format_callback_data = &flags;
>  	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
>  }
>  
> @@ -209,10 +210,13 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  
>  	if (take_worktree_changes) {
>  		const char **pathspec;
> +		int flags = ((verbose ? ADD_CACHE_VERBOSE : 0) |
> +			     (show_only ? ADD_CACHE_PRETEND : 0));
> +
>  		if (read_cache() < 0)
>  			die("index file corrupt");
>  		pathspec = get_pathspec(prefix, argv);
> -		add_files_to_cache(verbose, prefix, pathspec);
> +		add_files_to_cache(prefix, pathspec, flags);
>  		goto finish;
>  	}
>  
> @@ -254,7 +258,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  	}
>  
>  	for (i = 0; i < dir.nr; i++)
> -		add_file_to_cache(dir.entries[i]->name, verbose);
> +		add_file_to_cache(dir.entries[i]->name, ADD_CACHE_VERBOSE);
>  
>   finish:
>  	if (active_cache_changed) {
> diff --git a/builtin-commit.c b/builtin-commit.c
> index 2f4d6cc..f31bf59 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -239,7 +239,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
>  	 */
>  	if (all || (also && pathspec && *pathspec)) {
>  		int fd = hold_locked_index(&index_lock, 1);
> -		add_files_to_cache(0, also ? prefix : NULL, pathspec);
> +		add_files_to_cache(also ? prefix : NULL, pathspec, 0);
>  		refresh_cache(REFRESH_QUIET);
>  		if (write_cache(fd, active_cache, active_nr) ||
>  		    close_lock_file(&index_lock))
> diff --git a/builtin-mv.c b/builtin-mv.c
> index 990e213..1ad3178 100644
> --- a/builtin-mv.c
> +++ b/builtin-mv.c
> @@ -256,7 +256,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>  
>  		for (i = 0; i < added.nr; i++) {
>  			const char *path = added.items[i].path;
> -			add_file_to_cache(path, verbose);
> +			add_file_to_cache(path, verbose ? ADD_CACHE_VERBOSE : 0);
>  		}
>  
>  		for (i = 0; i < deleted.nr; i++)
> diff --git a/read-cache.c b/read-cache.c
> index 7db5588..a95861b 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -384,12 +384,14 @@ static int index_name_pos_also_unmerged(struct index_state *istate,
>  	return pos;
>  }
>  
> -int add_file_to_index(struct index_state *istate, const char *path, int verbose)
> +int add_file_to_index(struct index_state *istate, const char *path, int flags)
>  {
> -	int size, namelen, pos;
> +	int size, namelen, pos, was_same;
>  	struct stat st;
>  	struct cache_entry *ce;
>  	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_RACY_IS_DIRTY;
> +	int verbose = flags & (ADD_CACHE_VERBOSE|ADD_CACHE_PRETEND);
> +	int pretend = flags & ADD_CACHE_PRETEND;
>  
>  	if (lstat(path, &st))
>  		die("%s: unable to stat (%s)", path, strerror(errno));
> @@ -432,9 +434,19 @@ int add_file_to_index(struct index_state *istate, const char *path, int verbose)
>  
>  	if (index_path(ce->sha1, path, &st, 1))
>  		die("unable to index file %s", path);
> -	if (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
> +
> +	/* It was suspected to be racily clean but it turns out to be Ok */
> +	was_same = (0 <= pos &&
> +		    !ce_stage(istate->cache[pos]) &&
> +		    !hashcmp(ce->sha1, istate->cache[pos]->sha1) &&
> +		    ce->ce_mode == istate->cache[pos]->ce_mode);
> +
> +	if (pretend)
> +		;
> +	else if (add_index_entry(istate, ce,
> +				 ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
>  		die("unable to add %s to index",path);
> -	if (verbose)
> +	if (verbose && !was_same)
>  		printf("add '%s'\n", path);
>  	return 0;
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Re* [PATCH] Make git add -u honor --dry-run

[Junio C Hamano]

Gustaf Hendeby <hendeby@isy.liu.se> writes:

> On 2008-05-16 01:42, Junio C Hamano wrote:
>> Miklos Vajna <vmiklos@frugalware.org> writes:
>>> On Thu, May 15, 2008 at 06:08:24PM +0200, Gustaf Hendeby <hendeby@isy.liu.se> wrote:
>>>> I'm not familiar enough with the code to see how to best fix it.
>>
>> That makes the whole thing noop, doesn't it?  We could do the surgery at a
>> bit lower layer, I guess.
>
> What is the status on this one?  I've been away for a while and when I
> got back I didn't see any more than this suggestion.

We need a slightly different patch for 'master' (for 1.5.6) and 'maint'
(for 1.5.5.X), due to recent introduction of add_to_index() API.  This is
an updated one for 'master'; I rolled your one-liner fix into it as well.

-- >8 --
Subject: "git-add -n -u" should not add but just report

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-add.c         |   22 +++++++++++++---------
 builtin-checkout.c    |    2 +-
 builtin-commit.c      |    2 +-
 builtin-mv.c          |    2 +-
 cache.h               |   12 +++++++-----
 read-cache.c          |   23 +++++++++++++++++------
 t/t2200-add-update.sh |   17 +++++++++++++++++
 7 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 4a91e3e..05af57f 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -82,9 +82,9 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec,
 static void update_callback(struct diff_queue_struct *q,
 			    struct diff_options *opt, void *cbdata)
 {
-	int i, verbose;
+	int i, flags;
 
-	verbose = *((int *)cbdata);
+	flags = *((int *)cbdata);
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
 		const char *path = p->one->path;
@@ -94,18 +94,19 @@ static void update_callback(struct diff_queue_struct *q,
 		case DIFF_STATUS_UNMERGED:
 		case DIFF_STATUS_MODIFIED:
 		case DIFF_STATUS_TYPE_CHANGED:
-			add_file_to_cache(path, verbose);
+			add_file_to_cache(path, flags);
 			break;
 		case DIFF_STATUS_DELETED:
-			remove_file_from_cache(path);
-			if (verbose)
+			if (!(flags & ADD_CACHE_PRETEND))
+				remove_file_from_cache(path);
+			if (flags)
 				printf("remove '%s'\n", path);
 			break;
 		}
 	}
 }
 
-void add_files_to_cache(int verbose, const char *prefix, const char **pathspec)
+void add_files_to_cache(const char *prefix, const char **pathspec, int flags)
 {
 	struct rev_info rev;
 	init_revisions(&rev, prefix);
@@ -113,7 +114,7 @@ void add_files_to_cache(int verbose, const char *prefix, const char **pathspec)
 	rev.prune_data = pathspec;
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
-	rev.diffopt.format_callback_data = &verbose;
+	rev.diffopt.format_callback_data = &flags;
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
 }
 
@@ -209,10 +210,13 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	if (take_worktree_changes) {
 		const char **pathspec;
+		int flags = ((verbose ? ADD_CACHE_VERBOSE : 0) |
+			     (show_only ? ADD_CACHE_PRETEND : 0));
+
 		if (read_cache() < 0)
 			die("index file corrupt");
 		pathspec = get_pathspec(prefix, argv);
-		add_files_to_cache(verbose, prefix, pathspec);
+		add_files_to_cache(prefix, pathspec, flags);
 		goto finish;
 	}
 
@@ -254,7 +258,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	}
 
 	for (i = 0; i < dir.nr; i++)
-		add_file_to_cache(dir.entries[i]->name, verbose);
+		add_file_to_cache(dir.entries[i]->name, verbose ? ADD_CACHE_VERBOSE : 0);
 
  finish:
 	if (active_cache_changed) {
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 10ec137..05c0642 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -282,7 +282,7 @@ static int merge_working_tree(struct checkout_opts *opts,
 			 * entries in the index.
 			 */
 
-			add_files_to_cache(0, NULL, NULL);
+			add_files_to_cache(NULL, NULL, 0);
 			work = write_tree_from_memory();
 
 			ret = reset_to_new(new->commit->tree, opts->quiet);
diff --git a/builtin-commit.c b/builtin-commit.c
index 0baec6d..924fca1 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -246,7 +246,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
 	 */
 	if (all || (also && pathspec && *pathspec)) {
 		int fd = hold_locked_index(&index_lock, 1);
-		add_files_to_cache(0, also ? prefix : NULL, pathspec);
+		add_files_to_cache(also ? prefix : NULL, pathspec, 0);
 		refresh_cache(REFRESH_QUIET);
 		if (write_cache(fd, active_cache, active_nr) ||
 		    close_lock_file(&index_lock))
diff --git a/builtin-mv.c b/builtin-mv.c
index 94f6dd2..df9ea97 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -256,7 +256,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 
 		for (i = 0; i < added.nr; i++) {
 			const char *path = added.items[i].path;
-			add_file_to_cache(path, verbose);
+			add_file_to_cache(path, verbose ? ADD_CACHE_VERBOSE : 0);
 		}
 
 		for (i = 0; i < deleted.nr; i++)
diff --git a/cache.h b/cache.h
index 093f04c..b1a8427 100644
--- a/cache.h
+++ b/cache.h
@@ -261,8 +261,8 @@ static inline void remove_name_hash(struct cache_entry *ce)
 #define add_cache_entry(ce, option) add_index_entry(&the_index, (ce), (option))
 #define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))
 #define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))
-#define add_to_cache(path, st, verbose) add_to_index(&the_index, (path), (st), (verbose))
-#define add_file_to_cache(path, verbose) add_file_to_index(&the_index, (path), (verbose))
+#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags))
+#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags))
 #define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL)
 #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
 #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
@@ -366,8 +366,10 @@ extern int add_index_entry(struct index_state *, struct cache_entry *ce, int opt
 extern struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
 extern int remove_index_entry_at(struct index_state *, int pos);
 extern int remove_file_from_index(struct index_state *, const char *path);
-extern int add_to_index(struct index_state *, const char *path, struct stat *, int verbose);
-extern int add_file_to_index(struct index_state *, const char *path, int verbose);
+#define ADD_CACHE_VERBOSE 1
+#define ADD_CACHE_PRETEND 2
+extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
+extern int add_file_to_index(struct index_state *, const char *path, int flags);
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
 extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
 
@@ -782,7 +784,7 @@ extern int convert_to_git(const char *path, const char *src, size_t len,
 extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);
 
 /* add */
-void add_files_to_cache(int verbose, const char *prefix, const char **pathspec);
+void add_files_to_cache(const char *prefix, const char **pathspec, int flags);
 
 /* diff.c */
 extern int diff_auto_refresh_index;
diff --git a/read-cache.c b/read-cache.c
index 0382804..5d967e8 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -462,12 +462,14 @@ static struct cache_entry *create_alias_ce(struct cache_entry *ce, struct cache_
 	return new;
 }
 
-int add_to_index(struct index_state *istate, const char *path, struct stat *st, int verbose)
+int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)
 {
-	int size, namelen;
+	int size, namelen, was_same;
 	mode_t st_mode = st->st_mode;
 	struct cache_entry *ce, *alias;
 	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_RACY_IS_DIRTY;
+	int verbose = flags & (ADD_CACHE_VERBOSE | ADD_CACHE_PRETEND);
+	int pretend = flags & ADD_CACHE_PRETEND;
 
 	if (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))
 		die("%s: can only add regular files, symbolic links or git-directories", path);
@@ -509,19 +511,28 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	if (ignore_case && alias && different_name(ce, alias))
 		ce = create_alias_ce(ce, alias);
 	ce->ce_flags |= CE_ADDED;
-	if (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
+
+	/* It was suspected to be recily clean, but it turns out to be Ok */
+	was_same = (alias &&
+		    !ce_stage(alias) &&
+		    !hashcmp(alias->sha1, ce->sha1) &&
+		    ce->ce_mode == alias->ce_mode);
+
+	if (pretend)
+		;
+	else if (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
 		die("unable to add %s to index",path);
-	if (verbose)
+	if (verbose && !was_same)
 		printf("add '%s'\n", path);
 	return 0;
 }
 
-int add_file_to_index(struct index_state *istate, const char *path, int verbose)
+int add_file_to_index(struct index_state *istate, const char *path, int flags)
 {
 	struct stat st;
 	if (lstat(path, &st))
 		die("%s: unable to stat (%s)", path, strerror(errno));
-	return add_to_index(istate, path, &st, verbose);
+	return add_to_index(istate, path, &st, flags);
 }
 
 struct cache_entry *make_cache_entry(unsigned int mode,
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index b664341..f57a6e0 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -111,4 +111,21 @@ test_expect_success 'touch and then add explicitly' '
 
 '
 
+test_expect_success 'add -n -u should not add but just report' '
+
+	(
+		echo "add '\''check'\''" &&
+		echo "remove '\''top'\''"
+	) >expect &&
+	before=$(git ls-files -s check top) &&
+	echo changed >>check &&
+	rm -f top &&
+	git add -n -u >actual &&
+	after=$(git ls-files -s check top) &&
+
+	test "$before" = "$after" &&
+	test_cmp expect actual
+
+'
+
 test_done

Re: Re* [PATCH] Make git add -u honor --dry-run

[Gustaf Hendeby]

On 2008-05-22 20:16, Junio C Hamano wrote:
> Gustaf Hendeby <hendeby@isy.liu.se> writes:
> 
>> On 2008-05-16 01:42, Junio C Hamano wrote:
>>> Miklos Vajna <vmiklos@frugalware.org> writes:
>>>> On Thu, May 15, 2008 at 06:08:24PM +0200, Gustaf Hendeby <hendeby@isy.liu.se> wrote:
>>>>> I'm not familiar enough with the code to see how to best fix it.
>>> That makes the whole thing noop, doesn't it?  We could do the surgery at a
>>> bit lower layer, I guess.
>> What is the status on this one?  I've been away for a while and when I
>> got back I didn't see any more than this suggestion.
> 
> We need a slightly different patch for 'master' (for 1.5.6) and 'maint'
> (for 1.5.5.X), due to recent introduction of add_to_index() API.  This is
> an updated one for 'master'; I rolled your one-liner fix into it as well.

I'm sorry, but I don't get this to apply cleanly at all.  Did I 
misunderstand you, or is it actually meant for master (1af8bca)?  The 
merge is too complicated for me to do anything sensible with.

/Gustaf

> 
> -- >8 --
> Subject: "git-add -n -u" should not add but just report
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin-add.c         |   22 +++++++++++++---------
>  builtin-checkout.c    |    2 +-
>  builtin-commit.c      |    2 +-
>  builtin-mv.c          |    2 +-
>  cache.h               |   12 +++++++-----
>  read-cache.c          |   23 +++++++++++++++++------
>  t/t2200-add-update.sh |   17 +++++++++++++++++
>  7 files changed, 57 insertions(+), 23 deletions(-)
> 
> diff --git a/builtin-add.c b/builtin-add.c
> index 4a91e3e..05af57f 100644
> --- a/builtin-add.c
> +++ b/builtin-add.c
> @@ -82,9 +82,9 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec,
>  static void update_callback(struct diff_queue_struct *q,
>  			    struct diff_options *opt, void *cbdata)
>  {
> -	int i, verbose;
> +	int i, flags;
>  
> -	verbose = *((int *)cbdata);
> +	flags = *((int *)cbdata);
>  	for (i = 0; i < q->nr; i++) {
>  		struct diff_filepair *p = q->queue[i];
>  		const char *path = p->one->path;
> @@ -94,18 +94,19 @@ static void update_callback(struct diff_queue_struct *q,
>  		case DIFF_STATUS_UNMERGED:
>  		case DIFF_STATUS_MODIFIED:
>  		case DIFF_STATUS_TYPE_CHANGED:
> -			add_file_to_cache(path, verbose);
> +			add_file_to_cache(path, flags);
>  			break;
>  		case DIFF_STATUS_DELETED:
> -			remove_file_from_cache(path);
> -			if (verbose)
> +			if (!(flags & ADD_CACHE_PRETEND))
> +				remove_file_from_cache(path);
> +			if (flags)
>  				printf("remove '%s'\n", path);
>  			break;
>  		}
>  	}
>  }
>  
> -void add_files_to_cache(int verbose, const char *prefix, const char **pathspec)
> +void add_files_to_cache(const char *prefix, const char **pathspec, int flags)
>  {
>  	struct rev_info rev;
>  	init_revisions(&rev, prefix);
> @@ -113,7 +114,7 @@ void add_files_to_cache(int verbose, const char *prefix, const char **pathspec)
>  	rev.prune_data = pathspec;
>  	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
>  	rev.diffopt.format_callback = update_callback;
> -	rev.diffopt.format_callback_data = &verbose;
> +	rev.diffopt.format_callback_data = &flags;
>  	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
>  }
>  
> @@ -209,10 +210,13 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  
>  	if (take_worktree_changes) {
>  		const char **pathspec;
> +		int flags = ((verbose ? ADD_CACHE_VERBOSE : 0) |
> +			     (show_only ? ADD_CACHE_PRETEND : 0));
> +
>  		if (read_cache() < 0)
>  			die("index file corrupt");
>  		pathspec = get_pathspec(prefix, argv);
> -		add_files_to_cache(verbose, prefix, pathspec);
> +		add_files_to_cache(prefix, pathspec, flags);
>  		goto finish;
>  	}
>  
> @@ -254,7 +258,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  	}
>  
>  	for (i = 0; i < dir.nr; i++)
> -		add_file_to_cache(dir.entries[i]->name, verbose);
> +		add_file_to_cache(dir.entries[i]->name, verbose ? ADD_CACHE_VERBOSE : 0);
>  
>   finish:
>  	if (active_cache_changed) {
> diff --git a/builtin-checkout.c b/builtin-checkout.c
> index 10ec137..05c0642 100644
> --- a/builtin-checkout.c
> +++ b/builtin-checkout.c
> @@ -282,7 +282,7 @@ static int merge_working_tree(struct checkout_opts *opts,
>  			 * entries in the index.
>  			 */
>  
> -			add_files_to_cache(0, NULL, NULL);
> +			add_files_to_cache(NULL, NULL, 0);
>  			work = write_tree_from_memory();
>  
>  			ret = reset_to_new(new->commit->tree, opts->quiet);
> diff --git a/builtin-commit.c b/builtin-commit.c
> index 0baec6d..924fca1 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -246,7 +246,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
>  	 */
>  	if (all || (also && pathspec && *pathspec)) {
>  		int fd = hold_locked_index(&index_lock, 1);
> -		add_files_to_cache(0, also ? prefix : NULL, pathspec);
> +		add_files_to_cache(also ? prefix : NULL, pathspec, 0);
>  		refresh_cache(REFRESH_QUIET);
>  		if (write_cache(fd, active_cache, active_nr) ||
>  		    close_lock_file(&index_lock))
> diff --git a/builtin-mv.c b/builtin-mv.c
> index 94f6dd2..df9ea97 100644
> --- a/builtin-mv.c
> +++ b/builtin-mv.c
> @@ -256,7 +256,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>  
>  		for (i = 0; i < added.nr; i++) {
>  			const char *path = added.items[i].path;
> -			add_file_to_cache(path, verbose);
> +			add_file_to_cache(path, verbose ? ADD_CACHE_VERBOSE : 0);
>  		}
>  
>  		for (i = 0; i < deleted.nr; i++)
> diff --git a/cache.h b/cache.h
> index 093f04c..b1a8427 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -261,8 +261,8 @@ static inline void remove_name_hash(struct cache_entry *ce)
>  #define add_cache_entry(ce, option) add_index_entry(&the_index, (ce), (option))
>  #define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))
>  #define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))
> -#define add_to_cache(path, st, verbose) add_to_index(&the_index, (path), (st), (verbose))
> -#define add_file_to_cache(path, verbose) add_file_to_index(&the_index, (path), (verbose))
> +#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags))
> +#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags))
>  #define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL)
>  #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
>  #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
> @@ -366,8 +366,10 @@ extern int add_index_entry(struct index_state *, struct cache_entry *ce, int opt
>  extern struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
>  extern int remove_index_entry_at(struct index_state *, int pos);
>  extern int remove_file_from_index(struct index_state *, const char *path);
> -extern int add_to_index(struct index_state *, const char *path, struct stat *, int verbose);
> -extern int add_file_to_index(struct index_state *, const char *path, int verbose);
> +#define ADD_CACHE_VERBOSE 1
> +#define ADD_CACHE_PRETEND 2
> +extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
> +extern int add_file_to_index(struct index_state *, const char *path, int flags);
>  extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
>  extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
>  
> @@ -782,7 +784,7 @@ extern int convert_to_git(const char *path, const char *src, size_t len,
>  extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);
>  
>  /* add */
> -void add_files_to_cache(int verbose, const char *prefix, const char **pathspec);
> +void add_files_to_cache(const char *prefix, const char **pathspec, int flags);
>  
>  /* diff.c */
>  extern int diff_auto_refresh_index;
> diff --git a/read-cache.c b/read-cache.c
> index 0382804..5d967e8 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -462,12 +462,14 @@ static struct cache_entry *create_alias_ce(struct cache_entry *ce, struct cache_
>  	return new;
>  }
>  
> -int add_to_index(struct index_state *istate, const char *path, struct stat *st, int verbose)
> +int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)
>  {
> -	int size, namelen;
> +	int size, namelen, was_same;
>  	mode_t st_mode = st->st_mode;
>  	struct cache_entry *ce, *alias;
>  	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_RACY_IS_DIRTY;
> +	int verbose = flags & (ADD_CACHE_VERBOSE | ADD_CACHE_PRETEND);
> +	int pretend = flags & ADD_CACHE_PRETEND;
>  
>  	if (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))
>  		die("%s: can only add regular files, symbolic links or git-directories", path);
> @@ -509,19 +511,28 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
>  	if (ignore_case && alias && different_name(ce, alias))
>  		ce = create_alias_ce(ce, alias);
>  	ce->ce_flags |= CE_ADDED;
> -	if (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
> +
> +	/* It was suspected to be recily clean, but it turns out to be Ok */
> +	was_same = (alias &&
> +		    !ce_stage(alias) &&
> +		    !hashcmp(alias->sha1, ce->sha1) &&
> +		    ce->ce_mode == alias->ce_mode);
> +
> +	if (pretend)
> +		;
> +	else if (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
>  		die("unable to add %s to index",path);
> -	if (verbose)
> +	if (verbose && !was_same)
>  		printf("add '%s'\n", path);
>  	return 0;
>  }
>  
> -int add_file_to_index(struct index_state *istate, const char *path, int verbose)
> +int add_file_to_index(struct index_state *istate, const char *path, int flags)
>  {
>  	struct stat st;
>  	if (lstat(path, &st))
>  		die("%s: unable to stat (%s)", path, strerror(errno));
> -	return add_to_index(istate, path, &st, verbose);
> +	return add_to_index(istate, path, &st, flags);
>  }
>  
>  struct cache_entry *make_cache_entry(unsigned int mode,
> diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
> index b664341..f57a6e0 100755
> --- a/t/t2200-add-update.sh
> +++ b/t/t2200-add-update.sh
> @@ -111,4 +111,21 @@ test_expect_success 'touch and then add explicitly' '
>  
>  '
>  
> +test_expect_success 'add -n -u should not add but just report' '
> +
> +	(
> +		echo "add '\''check'\''" &&
> +		echo "remove '\''top'\''"
> +	) >expect &&
> +	before=$(git ls-files -s check top) &&
> +	echo changed >>check &&
> +	rm -f top &&
> +	git add -n -u >actual &&
> +	after=$(git ls-files -s check top) &&
> +
> +	test "$before" = "$after" &&
> +	test_cmp expect actual
> +
> +'
> +
>  test_done

Re: Re* [PATCH] Make git add -u honor --dry-run

[Junio C Hamano]

Gustaf Hendeby <hendeby@isy.liu.se> writes:

> On 2008-05-22 20:16, Junio C Hamano wrote:
>> ...
>> We need a slightly different patch for 'master' (for 1.5.6) and 'maint'
>> (for 1.5.5.X), due to recent introduction of add_to_index() API.  This is
>> an updated one for 'master'; I rolled your one-liner fix into it as well.
>
> I'm sorry, but I don't get this to apply cleanly at all.  Did I
> misunderstand you, or is it actually meant for master (1af8bca)?  The
> merge is too complicated for me to do anything sensible with.

Oh, sorry about that.  I prepared the patch and the message before
yesterday's merge binge, and one of the topics graduated to 'master'
somewhat conflicts with the patch.

The attached patch should apply cleanly to 1af8bca (Merge branch 'maint',
2008-05-21).

Side note.  The patch is already merged to 'next', so if you can try it to
see if it fixes your issue, we are hopefully in a good shape.

Side note2.  I applied the patch with "git-am -3" to 'master', which reused
the recorded resolution I previously had to make when I merged the topic
to 'next' to resolve the conflict fully.  This patch was created by taking
the diff between that result and 'master'.

---

 builtin-add.c         |   22 ++++++++++------------
 builtin-mv.c          |    2 +-
 cache.h               |   13 +++++++------
 read-cache.c          |   23 +++++++++++++++++------
 t/t2200-add-update.sh |   17 +++++++++++++++++
 5 files changed, 52 insertions(+), 25 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 73235ed..dd2ca4b 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -100,15 +100,16 @@ static void update_callback(struct diff_queue_struct *q,
 		case DIFF_STATUS_UNMERGED:
 		case DIFF_STATUS_MODIFIED:
 		case DIFF_STATUS_TYPE_CHANGED:
-			if (add_file_to_cache(path, data->flags & ADD_FILES_VERBOSE)) {
-				if (!(data->flags & ADD_FILES_IGNORE_ERRORS))
+			if (add_file_to_cache(path, data->flags)) {
+				if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
 					die("updating files failed");
 				data->add_errors++;
 			}
 			break;
 		case DIFF_STATUS_DELETED:
-			remove_file_from_cache(path);
-			if (data->flags & ADD_FILES_VERBOSE)
+			if (!(data->flags & ADD_CACHE_PRETEND))
+				remove_file_from_cache(path);
+			if (data->flags & (ADD_CACHE_PRETEND|ADD_CACHE_VERBOSE))
 				printf("remove '%s'\n", path);
 			break;
 		}
@@ -234,17 +235,14 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	newfd = hold_locked_index(&lock_file, 1);
 
 	if (take_worktree_changes) {
-		int flags = 0;
 		const char **pathspec;
+		int flags = ((verbose ? ADD_CACHE_VERBOSE : 0) |
+			     (show_only ? ADD_CACHE_PRETEND : 0) |
+			     (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0));
+
 		if (read_cache() < 0)
 			die("index file corrupt");
 		pathspec = get_pathspec(prefix, argv);
-
-		if (verbose)
-			flags |= ADD_FILES_VERBOSE;
-		if (ignore_add_errors)
-			flags |= ADD_FILES_IGNORE_ERRORS;
-
 		exit_status = add_files_to_cache(prefix, pathspec, flags);
 		goto finish;
 	}
@@ -287,7 +285,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	}
 
 	for (i = 0; i < dir.nr; i++)
-		if (add_file_to_cache(dir.entries[i]->name, verbose)) {
+		if (add_file_to_cache(dir.entries[i]->name, verbose ? ADD_CACHE_VERBOSE : 0)) {
 			if (!ignore_add_errors)
 				die("adding files failed");
 			exit_status = 1;
diff --git a/builtin-mv.c b/builtin-mv.c
index fb8ffb4..fb906b3 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -256,7 +256,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 
 		for (i = 0; i < added.nr; i++) {
 			const char *path = added.items[i].path;
-			if (add_file_to_cache(path, verbose))
+			if (add_file_to_cache(path, verbose ? ADD_CACHE_VERBOSE : 0))
 				die("updating index entries failed");
 		}
 
diff --git a/cache.h b/cache.h
index 0f89f15..123d6cc 100644
--- a/cache.h
+++ b/cache.h
@@ -261,8 +261,8 @@ static inline void remove_name_hash(struct cache_entry *ce)
 #define add_cache_entry(ce, option) add_index_entry(&the_index, (ce), (option))
 #define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))
 #define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))
-#define add_to_cache(path, st, verbose) add_to_index(&the_index, (path), (st), (verbose))
-#define add_file_to_cache(path, verbose) add_file_to_index(&the_index, (path), (verbose))
+#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags))
+#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags))
 #define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL)
 #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
 #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
@@ -366,8 +366,11 @@ extern int add_index_entry(struct index_state *, struct cache_entry *ce, int opt
 extern struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
 extern int remove_index_entry_at(struct index_state *, int pos);
 extern int remove_file_from_index(struct index_state *, const char *path);
-extern int add_to_index(struct index_state *, const char *path, struct stat *, int verbose);
-extern int add_file_to_index(struct index_state *, const char *path, int verbose);
+#define ADD_CACHE_VERBOSE 1
+#define ADD_CACHE_PRETEND 2
+#define ADD_CACHE_IGNORE_ERRORS	4
+extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
+extern int add_file_to_index(struct index_state *, const char *path, int flags);
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
 extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
 
@@ -782,8 +785,6 @@ extern int convert_to_git(const char *path, const char *src, size_t len,
 extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);
 
 /* add */
-#define ADD_FILES_VERBOSE	01
-#define ADD_FILES_IGNORE_ERRORS	02
 /*
  * return 0 if success, 1 - if addition of a file failed and
  * ADD_FILES_IGNORE_ERRORS was specified in flags
diff --git a/read-cache.c b/read-cache.c
index 8b467f8..c90cbb9 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -462,12 +462,14 @@ static struct cache_entry *create_alias_ce(struct cache_entry *ce, struct cache_
 	return new;
 }
 
-int add_to_index(struct index_state *istate, const char *path, struct stat *st, int verbose)
+int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)
 {
-	int size, namelen;
+	int size, namelen, was_same;
 	mode_t st_mode = st->st_mode;
 	struct cache_entry *ce, *alias;
 	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_RACY_IS_DIRTY;
+	int verbose = flags & (ADD_CACHE_VERBOSE | ADD_CACHE_PRETEND);
+	int pretend = flags & ADD_CACHE_PRETEND;
 
 	if (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))
 		return error("%s: can only add regular files, symbolic links or git-directories", path);
@@ -509,19 +511,28 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	if (ignore_case && alias && different_name(ce, alias))
 		ce = create_alias_ce(ce, alias);
 	ce->ce_flags |= CE_ADDED;
-	if (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
+
+	/* It was suspected to be recily clean, but it turns out to be Ok */
+	was_same = (alias &&
+		    !ce_stage(alias) &&
+		    !hashcmp(alias->sha1, ce->sha1) &&
+		    ce->ce_mode == alias->ce_mode);
+
+	if (pretend)
+		;
+	else if (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
 		return error("unable to add %s to index",path);
-	if (verbose)
+	if (verbose && !was_same)
 		printf("add '%s'\n", path);
 	return 0;
 }
 
-int add_file_to_index(struct index_state *istate, const char *path, int verbose)
+int add_file_to_index(struct index_state *istate, const char *path, int flags)
 {
 	struct stat st;
 	if (lstat(path, &st))
 		die("%s: unable to stat (%s)", path, strerror(errno));
-	return add_to_index(istate, path, &st, verbose);
+	return add_to_index(istate, path, &st, flags);
 }
 
 struct cache_entry *make_cache_entry(unsigned int mode,
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index b664341..f57a6e0 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -111,4 +111,21 @@ test_expect_success 'touch and then add explicitly' '
 
 '
 
+test_expect_success 'add -n -u should not add but just report' '
+
+	(
+		echo "add '\''check'\''" &&
+		echo "remove '\''top'\''"
+	) >expect &&
+	before=$(git ls-files -s check top) &&
+	echo changed >>check &&
+	rm -f top &&
+	git add -n -u >actual &&
+	after=$(git ls-files -s check top) &&
+
+	test "$before" = "$after" &&
+	test_cmp expect actual
+
+'
+
 test_done

Re: Re* [PATCH] Make git add -u honor --dry-run

[Gustaf Hendeby]

On 2008-05-22 22:38, Junio C Hamano wrote:
> Gustaf Hendeby <hendeby@isy.liu.se> writes:
>> On 2008-05-22 20:16, Junio C Hamano wrote:
>>> ...
>>> We need a slightly different patch for 'master' (for 1.5.6) and 'maint'
>>> (for 1.5.5.X), due to recent introduction of add_to_index() API.  This is
>>> an updated one for 'master'; I rolled your one-liner fix into it as well.
>> I'm sorry, but I don't get this to apply cleanly at all.  Did I
>> misunderstand you, or is it actually meant for master (1af8bca)?  The
>> merge is too complicated for me to do anything sensible with.
> 
> Oh, sorry about that.  I prepared the patch and the message before
> yesterday's merge binge, and one of the topics graduated to 'master'
> somewhat conflicts with the patch.

No problem!

> The attached patch should apply cleanly to 1af8bca (Merge branch 'maint',
> 2008-05-21).

Applies and fixes my problem (master, and the fix in next works too). 
Thank you!  However, in my process of testing I noticed something else, 
not really a bug but an inconsistency in how things are reported to the 
user:

$ git --version
git version 1.5.5.1.501.gefb4
$ git status
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   bar/bar
#       modified:   foo
#
no changes added to commit (use "git add" and/or "git commit -a")
$ git add -u -n
add 'bar/bar'
add 'foo'
$ git add -n foo bar/bar
bar/bar foo

I would have expected the output to be the same in this case.

/Gustaf

PS
Sorry for the duplicate Junio, I hit the wrong reply button, and lost 
the whole reply-list.

> Side note.  The patch is already merged to 'next', so if you can try it to
> see if it fixes your issue, we are hopefully in a good shape.
> 
> Side note2.  I applied the patch with "git-am -3" to 'master', which reused
> the recorded resolution I previously had to make when I merged the topic
> to 'next' to resolve the conflict fully.  This patch was created by taking
> the diff between that result and 'master'.
> 
> ---
> 
>  builtin-add.c         |   22 ++++++++++------------
>  builtin-mv.c          |    2 +-
>  cache.h               |   13 +++++++------
>  read-cache.c          |   23 +++++++++++++++++------
>  t/t2200-add-update.sh |   17 +++++++++++++++++
>  5 files changed, 52 insertions(+), 25 deletions(-)
> 
> diff --git a/builtin-add.c b/builtin-add.c
> index 73235ed..dd2ca4b 100644
> --- a/builtin-add.c
> +++ b/builtin-add.c
> @@ -100,15 +100,16 @@ static void update_callback(struct diff_queue_struct *q,
>  		case DIFF_STATUS_UNMERGED:
>  		case DIFF_STATUS_MODIFIED:
>  		case DIFF_STATUS_TYPE_CHANGED:
> -			if (add_file_to_cache(path, data->flags & ADD_FILES_VERBOSE)) {
> -				if (!(data->flags & ADD_FILES_IGNORE_ERRORS))
> +			if (add_file_to_cache(path, data->flags)) {
> +				if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
>  					die("updating files failed");
>  				data->add_errors++;
>  			}
>  			break;
>  		case DIFF_STATUS_DELETED:
> -			remove_file_from_cache(path);
> -			if (data->flags & ADD_FILES_VERBOSE)
> +			if (!(data->flags & ADD_CACHE_PRETEND))
> +				remove_file_from_cache(path);
> +			if (data->flags & (ADD_CACHE_PRETEND|ADD_CACHE_VERBOSE))
>  				printf("remove '%s'\n", path);
>  			break;
>  		}
> @@ -234,17 +235,14 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  	newfd = hold_locked_index(&lock_file, 1);
>  
>  	if (take_worktree_changes) {
> -		int flags = 0;
>  		const char **pathspec;
> +		int flags = ((verbose ? ADD_CACHE_VERBOSE : 0) |
> +			     (show_only ? ADD_CACHE_PRETEND : 0) |
> +			     (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0));
> +
>  		if (read_cache() < 0)
>  			die("index file corrupt");
>  		pathspec = get_pathspec(prefix, argv);
> -
> -		if (verbose)
> -			flags |= ADD_FILES_VERBOSE;
> -		if (ignore_add_errors)
> -			flags |= ADD_FILES_IGNORE_ERRORS;
> -
>  		exit_status = add_files_to_cache(prefix, pathspec, flags);
>  		goto finish;
>  	}
> @@ -287,7 +285,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  	}
>  
>  	for (i = 0; i < dir.nr; i++)
> -		if (add_file_to_cache(dir.entries[i]->name, verbose)) {
> +		if (add_file_to_cache(dir.entries[i]->name, verbose ? ADD_CACHE_VERBOSE : 0)) {
>  			if (!ignore_add_errors)
>  				die("adding files failed");
>  			exit_status = 1;
> diff --git a/builtin-mv.c b/builtin-mv.c
> index fb8ffb4..fb906b3 100644
> --- a/builtin-mv.c
> +++ b/builtin-mv.c
> @@ -256,7 +256,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>  
>  		for (i = 0; i < added.nr; i++) {
>  			const char *path = added.items[i].path;
> -			if (add_file_to_cache(path, verbose))
> +			if (add_file_to_cache(path, verbose ? ADD_CACHE_VERBOSE : 0))
>  				die("updating index entries failed");
>  		}
>  
> diff --git a/cache.h b/cache.h
> index 0f89f15..123d6cc 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -261,8 +261,8 @@ static inline void remove_name_hash(struct cache_entry *ce)
>  #define add_cache_entry(ce, option) add_index_entry(&the_index, (ce), (option))
>  #define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))
>  #define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))
> -#define add_to_cache(path, st, verbose) add_to_index(&the_index, (path), (st), (verbose))
> -#define add_file_to_cache(path, verbose) add_file_to_index(&the_index, (path), (verbose))
> +#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags))
> +#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags))
>  #define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL)
>  #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
>  #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
> @@ -366,8 +366,11 @@ extern int add_index_entry(struct index_state *, struct cache_entry *ce, int opt
>  extern struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
>  extern int remove_index_entry_at(struct index_state *, int pos);
>  extern int remove_file_from_index(struct index_state *, const char *path);
> -extern int add_to_index(struct index_state *, const char *path, struct stat *, int verbose);
> -extern int add_file_to_index(struct index_state *, const char *path, int verbose);
> +#define ADD_CACHE_VERBOSE 1
> +#define ADD_CACHE_PRETEND 2
> +#define ADD_CACHE_IGNORE_ERRORS	4
> +extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
> +extern int add_file_to_index(struct index_state *, const char *path, int flags);
>  extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
>  extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
>  
> @@ -782,8 +785,6 @@ extern int convert_to_git(const char *path, const char *src, size_t len,
>  extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);
>  
>  /* add */
> -#define ADD_FILES_VERBOSE	01
> -#define ADD_FILES_IGNORE_ERRORS	02
>  /*
>   * return 0 if success, 1 - if addition of a file failed and
>   * ADD_FILES_IGNORE_ERRORS was specified in flags
> diff --git a/read-cache.c b/read-cache.c
> index 8b467f8..c90cbb9 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -462,12 +462,14 @@ static struct cache_entry *create_alias_ce(struct cache_entry *ce, struct cache_
>  	return new;
>  }
>  
> -int add_to_index(struct index_state *istate, const char *path, struct stat *st, int verbose)
> +int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)
>  {
> -	int size, namelen;
> +	int size, namelen, was_same;
>  	mode_t st_mode = st->st_mode;
>  	struct cache_entry *ce, *alias;
>  	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_RACY_IS_DIRTY;
> +	int verbose = flags & (ADD_CACHE_VERBOSE | ADD_CACHE_PRETEND);
> +	int pretend = flags & ADD_CACHE_PRETEND;
>  
>  	if (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))
>  		return error("%s: can only add regular files, symbolic links or git-directories", path);
> @@ -509,19 +511,28 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
>  	if (ignore_case && alias && different_name(ce, alias))
>  		ce = create_alias_ce(ce, alias);
>  	ce->ce_flags |= CE_ADDED;
> -	if (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
> +
> +	/* It was suspected to be recily clean, but it turns out to be Ok */
> +	was_same = (alias &&
> +		    !ce_stage(alias) &&
> +		    !hashcmp(alias->sha1, ce->sha1) &&
> +		    ce->ce_mode == alias->ce_mode);
> +
> +	if (pretend)
> +		;
> +	else if (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
>  		return error("unable to add %s to index",path);
> -	if (verbose)
> +	if (verbose && !was_same)
>  		printf("add '%s'\n", path);
>  	return 0;
>  }
>  
> -int add_file_to_index(struct index_state *istate, const char *path, int verbose)
> +int add_file_to_index(struct index_state *istate, const char *path, int flags)
>  {
>  	struct stat st;
>  	if (lstat(path, &st))
>  		die("%s: unable to stat (%s)", path, strerror(errno));
> -	return add_to_index(istate, path, &st, verbose);
> +	return add_to_index(istate, path, &st, flags);
>  }
>  
>  struct cache_entry *make_cache_entry(unsigned int mode,
> diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
> index b664341..f57a6e0 100755
> --- a/t/t2200-add-update.sh
> +++ b/t/t2200-add-update.sh
> @@ -111,4 +111,21 @@ test_expect_success 'touch and then add explicitly' '
>  
>  '
>  
> +test_expect_success 'add -n -u should not add but just report' '
> +
> +	(
> +		echo "add '\''check'\''" &&
> +		echo "remove '\''top'\''"
> +	) >expect &&
> +	before=$(git ls-files -s check top) &&
> +	echo changed >>check &&
> +	rm -f top &&
> +	git add -n -u >actual &&
> +	after=$(git ls-files -s check top) &&
> +
> +	test "$before" = "$after" &&
> +	test_cmp expect actual
> +
> +'
> +
>  test_done

[PATCH] Make git add -n and git -u -n output consistent

[Gustaf Hendeby]

Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
---

This would be one way to go to get a more coherent behavior or the -n
switch to git add.  It would also unify the implementation somewhat.
I'd suggest amending this to your patch, or would the output be likely
to be used by scrips?  In that case I'd vote for changing the output
of git add -n -u,

/Gustaf


 builtin-add.c |   19 ++++++-------------
 1 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index dd2ca4b..e8dce30 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -261,17 +261,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	fill_directory(&dir, pathspec, ignored_too);
 
-	if (show_only) {
-		const char *sep = "", *eof = "";
-		for (i = 0; i < dir.nr; i++) {
-			printf("%s%s", sep, dir.entries[i]->name);
-			sep = " ";
-			eof = "\n";
-		}
-		fputs(eof, stdout);
-		return 0;
-	}
-
 	if (read_cache() < 0)
 		die("index file corrupt");
 
@@ -284,12 +273,16 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		die("no files added");
 	}
 
-	for (i = 0; i < dir.nr; i++)
-		if (add_file_to_cache(dir.entries[i]->name, verbose ? ADD_CACHE_VERBOSE : 0)) {
+	for (i = 0; i < dir.nr; i++) {
+		int flags = ((verbose ? ADD_CACHE_VERBOSE : 0) |
+								 (show_only ? ADD_CACHE_PRETEND : 0) |
+								 (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0));
+		if (add_file_to_cache(dir.entries[i]->name, flags)) {
 			if (!ignore_add_errors)
 				die("adding files failed");
 			exit_status = 1;
 		}
+	}
 
  finish:
 	if (active_cache_changed) {
-- 
1.5.5.1.501.gefb4

Re: [PATCH] Make git add -n and git -u -n output consistent

[Junio C Hamano]

Gustaf Hendeby <hendeby@isy.liu.se> writes:

> Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
> ---
>
> This would be one way to go to get a more coherent behavior or the -n
> switch to git add.  It would also unify the implementation somewhat.
> I'd suggest amending this to your patch, or would the output be likely
> to be used by scrips?  In that case I'd vote for changing the output
> of git add -n -u,

The commit log message should describe what kind of consistency you are
talking about to people who are not familiar with the topic.

Output format from "git add -n $path" lists path to blobs that are going
to be added on a single line, separated with SP.  On the other hand, the
suggested "git add -u -n" shows one path per line, like "add '<file>'\n".
Of course, these two are inconsistent.

Plain "git add -n" can afford to only say names of paths, as all it does
is to add (update).  However, "git add -u" needs to be able to express
"remove" somehow.  So if we need to have them formatted the same way, we
need to unify with the "git add -n -u" format.  Incidentally, this is
consistent with how 'update-index' says it.

I do not think we need to worry about people who wrote script around
output from "git add -n".  Output from Porcelain commands is a fair game
for improvements.

Re: [PATCH] Make git add -n and git -u -n output consistent

[Gustaf Hendeby]

On 05/23/2008 06:40 AM, Junio C Hamano wrote:
> Gustaf Hendeby <hendeby@isy.liu.se> writes:
> 
>> Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
>> ---
>>
>> This would be one way to go to get a more coherent behavior or the -n
>> switch to git add.  It would also unify the implementation somewhat.
>> I'd suggest amending this to your patch, or would the output be likely
>> to be used by scrips?  In that case I'd vote for changing the output
>> of git add -n -u,
> 
> The commit log message should describe what kind of consistency you are
> talking about to people who are not familiar with the topic.

Point taken, I will try to do better next time.

> 
> Output format from "git add -n $path" lists path to blobs that are going
> to be added on a single line, separated with SP.  On the other hand, the
> suggested "git add -u -n" shows one path per line, like "add '<file>'\n".
> Of course, these two are inconsistent.
> 
> Plain "git add -n" can afford to only say names of paths, as all it does
> is to add (update).  However, "git add -u" needs to be able to express
> "remove" somehow.  So if we need to have them formatted the same way, we
> need to unify with the "git add -n -u" format.  Incidentally, this is
> consistent with how 'update-index' says it.
> 
> I do not think we need to worry about people who wrote script around
> output from "git add -n".  Output from Porcelain commands is a fair game
> for improvements.

Thanks for the explaination.

/Gustaf