threads / patch / 7753

patchgit-add -u: match the index with working tree.

Subject: [PATCH] git-add -u: match the index with working tree.

## tl;dr

13 messages between Apr 20, 2007 and Apr 20, 2007. Diffs are folded; open one to read it.

replies: 12people: 9as markdown or json

Junio C Hamano· Apr 20, 2007, 08:42 UTC · lore

This is a shorthand of what "git commit -a" does in preparation for making a commit, which is:

    git diff-files --name-only -z | git update-index --remove -z --stdin
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-add.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 57 insertions(+), 1 deletions(-)
Show changes to builtin-add.c +57 −1
diff --git a/builtin-add.c b/builtin-add.c
index 9ec2925..5e6748f 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -8,10 +8,15 @@
 #include "dir.h"
 #include "exec_cmd.h"
 #include "cache-tree.h"
+#include "diff.h"
+#include "diffcore.h"
+#include "commit.h"
+#include "revision.h"
 
 static const char builtin_add_usage[] =
-"git-add [-n] [-v] [-f] [--interactive | -i] [--] <filepattern>...";
+"git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--] <filepattern>...";
 
+static int take_all_worktree_changes;
 static const char *excludes_file;
 
 static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
@@ -92,6 +97,44 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec)
 		prune_directory(dir, pathspec, baselen);
 }
 
+static void update_callback(struct diff_queue_struct *q,
+			    struct diff_options *opt, void *cbdata)
+{
+	int i, verbose;
+
+	verbose = *((int *)cbdata);
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+		const char *path = p->one->path;
+		switch (p->status) {
+		default:
+			die("unexpacted diff status %c", p->status);
+		case DIFF_STATUS_UNMERGED:
+		case DIFF_STATUS_MODIFIED:
+			add_file_to_cache(path, verbose);
+			break;
+		case DIFF_STATUS_DELETED:
+			remove_file_from_cache(path);
+			if (verbose)
+				printf("remove '%s'\n", path);
+			break;
+		}
+	}
+}
+
+static void update_all(int verbose)
+{
+	struct rev_info rev;
+	init_revisions(&rev, "");
+	setup_revisions(0, NULL, &rev, NULL);
+	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
+	rev.diffopt.format_callback = update_callback;
+	rev.diffopt.format_callback_data = &verbose;
+	if (read_cache() < 0)
+		die("index file corrupt");
+	run_diff_files(&rev, 0);
+}
+
 static int git_add_config(const char *var, const char *value)
 {
 	if (!strcmp(var, "core.excludesfile")) {
@@ -156,8 +199,20 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 			verbose = 1;
 			continue;
 		}
+		if (!strcmp(arg, "-u")) {
+			take_all_worktree_changes = 1;
+			continue;
+		}
 		usage(builtin_add_usage);
 	}
+
+	if (take_all_worktree_changes) {
+		if (i < argc)
+			die("-u and explicit paths are incompatible");
+		update_all(verbose);
+		goto finish;
+	}
+
 	if (argc <= i) {
 		fprintf(stderr, "Nothing specified, nothing added.\n");
 		fprintf(stderr, "Maybe you wanted to say 'git add .'?\n");
@@ -207,6 +262,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);
 
+ finish:
 	if (active_cache_changed) {
 		if (write_cache(newfd, active_cache, active_nr) ||
 		    close(newfd) || commit_locked_index(&lock_file))
-- 
1.5.1.1.942.g0a20
Junio C Hamano· Apr 20, 2007, 08:42 UTC · re: Junio C Hamano · lore

[PATCH] Do not show progress meter while checking files out.

Originally I thought it would take too long to check out many files and to prevent people from getting bored, I added progress meter. But it feels a bit too noisy; let's disable it.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 unpack-trees.c |   62 --------------------------------------------------------
 1 files changed, 0 insertions(+), 62 deletions(-)
Show changes to unpack-trees.c +0 −62
diff --git a/unpack-trees.c b/unpack-trees.c
index 5139481..1419653 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -288,73 +288,15 @@ static void unlink_entry(char *name)
 	}
 }
 
-static volatile sig_atomic_t progress_update;
-
-static void progress_interval(int signum)
-{
-	progress_update = 1;
-}
-
-static void setup_progress_signal(void)
-{
-	struct sigaction sa;
-	struct itimerval v;
-
-	memset(&sa, 0, sizeof(sa));
-	sa.sa_handler = progress_interval;
-	sigemptyset(&sa.sa_mask);
-	sa.sa_flags = SA_RESTART;
-	sigaction(SIGALRM, &sa, NULL);
-
-	v.it_interval.tv_sec = 1;
-	v.it_interval.tv_usec = 0;
-	v.it_value = v.it_interval;
-	setitimer(ITIMER_REAL, &v, NULL);
-}
-
 static struct checkout state;
 static void check_updates(struct cache_entry **src, int nr,
 		struct unpack_trees_options *o)
 {
 	unsigned short mask = htons(CE_UPDATE);
-	unsigned last_percent = 200, cnt = 0, total = 0;
-
-	if (o->update && o->verbose_update) {
-		for (total = cnt = 0; cnt < nr; cnt++) {
-			struct cache_entry *ce = src[cnt];
-			if (!ce->ce_mode || ce->ce_flags & mask)
-				total++;
-		}
-
-		/* Don't bother doing this for very small updates */
-		if (total < 250)
-			total = 0;
-
-		if (total) {
-			fprintf(stderr, "Checking files out...\n");
-			setup_progress_signal();
-			progress_update = 1;
-		}
-		cnt = 0;
-	}
 
 	while (nr--) {
 		struct cache_entry *ce = *src++;
 
-		if (total) {
-			if (!ce->ce_mode || ce->ce_flags & mask) {
-				unsigned percent;
-				cnt++;
-				percent = (cnt * 100) / total;
-				if (percent != last_percent ||
-				    progress_update) {
-					fprintf(stderr, "%4u%% (%u/%u) done\r",
-						percent, cnt, total);
-					last_percent = percent;
-					progress_update = 0;
-				}
-			}
-		}
 		if (!ce->ce_mode) {
 			if (o->update)
 				unlink_entry(ce->name);
@@ -366,10 +308,6 @@ static void check_updates(struct cache_entry **src, int nr,
 				checkout_entry(ce, &state, NULL);
 		}
 	}
-	if (total) {
-		signal(SIGALRM, SIG_IGN);
-		fputc('\n', stderr);
-	}
 }
 
 int unpack_trees(struct object_list *trees, struct unpack_trees_options *o)
-- 
1.5.1.1.942.g0a20
Johannes Sixt· Apr 20, 2007, 09:37 UTC · re: Junio C Hamano · lore

Re: [PATCH] Do not show progress meter while checking files out.

Junio C Hamano wrote:
> 
> Originally I thought it would take too long to check out many
> files and to prevent people from getting bored, I added progress
> meter.  But it feels a bit too noisy; let's disable it.

Hm. The only place where I've seen this particular progress meter is after a clone. Which is quite noisy anyway.

Here on my windows box, this progress meter *is* entertaining. My project has ~2400 files, and it takes its time. If you really want to get rid of it, then please leave the message "Checking out files...", which together with a thrashing disk should give "progress" enough for people who do their first clone.

-- Hannes
Andy Parkins· Apr 20, 2007, 12:37 UTC · re: Junio C Hamano · lore

Re: [PATCH] Do not show progress meter while checking files out.

On Friday 2007 April 20 09:42, Junio C Hamano wrote:
> Originally I thought it would take too long to check out many
> files and to prevent people from getting bored, I added progress
> meter.  But it feels a bit too noisy; let's disable it.
Don't do that.  I like it.

I think you need output like this not to prevent boredom but to reassure that nothing has crashed or gone wild. A checkout of a big project really can take time, so it's nice to have the feedback.

Incidentally, the other place that needs some sort of feedback is the beginning of git-fetch/git-push. I think they are off listing remote branches, but they don't output anything before they do it, so I have no idea where the delay is (actually I know that the delay is from forming the ssh connection - which seems to be unable to understand that I'm connecting over a VPN and my IP has no reverse DNS so don't check for it - grr - but the point is that nothing is (apparently) happening and I have no feedback to know why).

I'll submit a patch...
Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com
Matthieu Moy· Apr 20, 2007, 12:46 UTC · re: Junio C Hamano · lore

Re: [PATCH] Do not show progress meter while checking files out.

Junio C Hamano <junkio@cox.net> writes:
> Originally I thought it would take too long to check out many
> files and to prevent people from getting bored, I added progress
> meter.  But it feels a bit too noisy; let's disable it.
[...]
> -		/* Don't bother doing this for very small updates */
> -		if (total < 250)
> -			total = 0;
[...]
Andy Parkins <andyparkins@gmail.com> writes:
> Don't do that.  I like it.

If it's just to make it less noisy, perhaps adjusting the magic value above would do it.

-- 
Matthieu
Nicolas Pitre· Apr 20, 2007, 13:04 UTC · re: Junio C Hamano · lore

Re: [PATCH] Do not show progress meter while checking files out.

On Fri, 20 Apr 2007, Junio C Hamano wrote:
> Originally I thought it would take too long to check out many
> files and to prevent people from getting bored, I added progress
> meter.  But it feels a bit too noisy; let's disable it.

For large checkouts, say after a big clone, that might be quite long to check out large amount of files.

What about looking at the number of files checked out after say 2 seconds, and if it is still below 50% of the total then turn on the progress display?

Nicolas
Shawn O. Pearce· Apr 20, 2007, 14:46 UTC · re: Nicolas Pitre · lore

Re: [PATCH] Do not show progress meter while checking files out.

Nicolas Pitre <nico@cam.org> wrote:
Show 5 quoted lines
> On Fri, 20 Apr 2007, Junio C Hamano wrote:
> 
> > Originally I thought it would take too long to check out many
> > files and to prevent people from getting bored, I added progress
> > meter.  But it feels a bit too noisy; let's disable it.
...
> What about looking at the number of files checked out after say 2 
> seconds, and if it is still below 50% of the total then turn on the 
> progress display?
I agree completely with Nico, and everyone else.

Nico's approach is the right way to handle that particular progress meter. It should also be enabled the same way in git-checkout.sh and git-merge.sh (for the fast-forward case).

On Windows, with a cheap+slow 5400 RPM IDE drive, a slow processor and a virus scanner that has higher priority than the mouse driver, a simple branch switch that updates only 500 files (out of almost 10,000) can take 30 seconds. Ok, sure, maybe I shouldn't switch branches on such horrid hardware[*1*], but a progress meter would be very nice for when I do.

On the other hand, the one I removed from merge-recursive was braindamaged. It only knew the amount of work remaining once it had finished it. That meant the meter was completely useless. Though maybe something based on a 2 second timer like Nico is proposing for read-tree might still be useful in merge-recursive.

*1*: Of course my Solaris 9 system does that switch so fast it makes
     my head spin.  Ahh, what a good system modern UNIXes are...
-- 
Shawn.
J. Bruce Fields· Apr 20, 2007, 13:12 UTC · re: Junio C Hamano · lore

Re: [PATCH] Do not show progress meter while checking files out.

On Fri, Apr 20, 2007 at 01:42:19AM -0700, Junio C Hamano wrote:
> Originally I thought it would take too long to check out many
> files and to prevent people from getting bored, I added progress
> meter.  But it feels a bit too noisy; let's disable it.

I was just doing a kernel-tree clone on an NFS filesystem yesterday, and creating 20000 new files over NFS does take some time, so the progress meter was appreciated.

--b.
Karl Hasselström· Apr 20, 2007, 10:49 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-add -u: match the index with working tree.

On 2007-04-20 01:42:18 -0700, Junio C Hamano wrote:
> This is a shorthand of what "git commit -a" does in preparation
> for making a commit,
[snip]
> -"git-add [-n] [-v] [-f] [--interactive | -i] [--] <filepattern>...";
> +"git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--] <filepattern>...";

Any particular reason for choosing a different letter than the -a/--all that git commit uses?

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle
Junio C Hamano· Apr 20, 2007, 11:16 UTC · re: Karl Hasselström · lore

Re: [PATCH] git-add -u: match the index with working tree.

Karl Hasselström <kha@treskal.com> writes:
Show 10 quoted lines
> On 2007-04-20 01:42:18 -0700, Junio C Hamano wrote:
>
>> This is a shorthand of what "git commit -a" does in preparation
>> for making a commit,
> [snip]
>> -"git-add [-n] [-v] [-f] [--interactive | -i] [--] <filepattern>...";
>> +"git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--] <filepattern>...";
>
> Any particular reason for choosing a different letter than the
> -a/--all that git commit uses?
Haven't I explain that elsewhere already?
Nikolai Weibull· Apr 20, 2007, 11:18 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-add -u: match the index with working tree.

On 4/20/07, Junio C Hamano <junkio@cox.net> wrote:
Show 14 quoted lines
> Karl Hasselström <kha@treskal.com> writes:
>
> > On 2007-04-20 01:42:18 -0700, Junio C Hamano wrote:
> >
> >> This is a shorthand of what "git commit -a" does in preparation
> >> for making a commit,
> > [snip]
> >> -"git-add [-n] [-v] [-f] [--interactive | -i] [--] <filepattern>...";
> >> +"git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--] <filepattern>...";
> >
> > Any particular reason for choosing a different letter than the
> > -a/--all that git commit uses?
>
> Haven't I explain that elsewhere already?
Yes, in <7vzm531ly3.fsf@assigned-by-dhcp.cox.net>.
Specifically:
> I picked "-u" instead of "-a" because I wanted to stress that
> this is about "updating" (which has connotation that it is
> relative to something, and in this case it is relative to the
> current "index"), and not about "all", which "-a" would imply.
  nikolai
Karl Hasselström· Apr 20, 2007, 11:39 UTC · re: Nikolai Weibull · lore

Re: [PATCH] git-add -u: match the index with working tree.

On 2007-04-20 13:18:25 +0200, Nikolai Weibull wrote:
Show 10 quoted lines
> On 4/20/07, Junio C Hamano <junkio@cox.net> wrote:
>
> > Karl Hasselström <kha@treskal.com> writes:
>
> > > Any particular reason for choosing a different letter than the
> > > -a/--all that git commit uses?
> >
> > Haven't I explain that elsewhere already?
>
> Yes, in <7vzm531ly3.fsf@assigned-by-dhcp.cox.net>.
Ah, I missed that. Sorry.
-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle
Junio C Hamano· Apr 20, 2007, 11:17 UTC · re: Karl Hasselström · lore

Re: [PATCH] git-add -u: match the index with working tree.

Karl Hasselström <kha@treskal.com> writes:
Show 10 quoted lines
> On 2007-04-20 01:42:18 -0700, Junio C Hamano wrote:
>
>> This is a shorthand of what "git commit -a" does in preparation
>> for making a commit,
> [snip]
>> -"git-add [-n] [-v] [-f] [--interactive | -i] [--] <filepattern>...";
>> +"git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--] <filepattern>...";
>
> Any particular reason for choosing a different letter than the
> -a/--all that git commit uses?

I picked "-u" instead of "-a" because I wanted to stress that this is about "updating" (which has connotation that it is relative to something, and in this case it is relative to the current "index"), and not about "all", which "-a" would imply.

← back to recent threads