Git development
 help / color / mirror / Atom feed
* Re: [PATCH 10/10] push: teach push to be quiet if local ref is strict subset of remote ref
From: Junio C Hamano @ 2007-10-30 19:19 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git
In-Reply-To: <52171BF7-50E2-473E-A0BD-CB64D38FD502@zib.de>

Steffen Prohaska <prohaska@zib.de> writes:

> On Oct 30, 2007, at 9:29 AM, Junio C Hamano wrote:
>
>> It simply is insane to make this strange rule 10/10 introduces
>> the default behaviour.  It is too specific to a particular
>> workflow (that is, working with a shared central repository,
>> having many locally tracking branches that are not often used
>> and become stale, and working on only things to completion
>> between pushes).
>
> I don't think its very strange behaviour if you see it in the
> light of what the user wants to achieve. We are talking about
> the case were only fast forward pushes are allowed. So, we
> only talk about a push that has the goal of adding new local
> changes to the remote. The user says "git push" and means
> push my new local changes to the remote.

If you want to push a specific subset of branches, you should
not be invoking the "matching refs" to begin with.  And breaking
the "matching refs" behaviour is not the way to fix it.

You can rewind a wrong branch by mistake locally and run push.
With your change you would not notice that mistake.

        $ git checkout bar
        $ work work work; commit commit commit
	$ git checkout test
        $ git merge bar
	... integrate, build, test
        ... notice that the tip commit of bar is not ready
        $ git checkout foo ;# oops, mistake
        $ git reset --hard HEAD^
	$ git push

If you checked out foo instead of bar by mistake at the last
"git checkout" step like this, your change will make 'foo' an
ancestor of the other side of the connection, and push silently
ignores it instead of failing.

Also, the behaviour is too specific to your workflow of working
on things only to completion between pushes.  If you work a bit
on branch 'foo' (but not complete), and work much on branch
'bar', 'baz', and 'boo' making all of them ready to be
published, you cannot say "git push" anyway.  Instead you have
to say "git push $remote bar baz boo".

This discourages people from making commits that are not ready
to be published, which is a very wrong thing to do, as a major
selling point of distributed revision control is the
dissociation between committing and publishing.

You work and commit freely, and at any point some of your
branches are ready to be published while some others
aren't. Inconvenience of "matching refs" may need to be worked
around.  I liked your "current branch only", with "git push
$remote HEAD" (I presume that "remote.$remote.push = HEAD" and
"branch.$current.remote = $remote" would let you do that with
"git push"), exactly because the way it specifies which branch
is to be published is very clearly defined and easy to
understand.  This "matching but only ff" does not have that
attractive clarity.

^ permalink raw reply

* [PATCH] Fixed a command line option type for builtin-fsck.c
From: Emil Medve @ 2007-10-30 19:15 UTC (permalink / raw)
  To: madcoder, spearce, gitster, git; +Cc: Emil Medve

The typo was introduced by this commit: 5ac0a2063e8f824f6e8ffb4d18de74c55aae7131

Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---

This patch applies to the 'next' branch

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

diff --git a/builtin-fsck.c b/builtin-fsck.c
index 64da3bd..e4874f6 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -680,7 +680,7 @@ static struct option fsck_opts[] = {
 	OPT_BOOLEAN(0, "cache", &keep_cache_objects, "make index objects head nodes"),
 	OPT_BOOLEAN(0, "reflogs", &include_reflogs, "make reflogs head nodes (default)"),
 	OPT_BOOLEAN(0, "full", &check_full, "also consider alternate objects"),
-	OPT_BOOLEAN(0, "struct", &check_strict, "enable more strict checking"),
+	OPT_BOOLEAN(0, "strict", &check_strict, "enable more strict checking"),
 	OPT_BOOLEAN(0, "lost-found", &write_lost_and_found,
 				"write dangling objects in .git/lost-found"),
 	OPT_END(),
-- 
1.5.3.4.1458.g3e72e-dirty

^ permalink raw reply related

* [PATCH 4/5] add throughput to progress display
From: Nicolas Pitre @ 2007-10-30 18:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1193770655-20492-4-git-send-email-nico@cam.org>

This adds the ability for the progress code to also display transfer
throughput when that makes sense.

The math was inspired by commit c548cf4ee0737a321ffe94f6a97c65baf87281be
from Linus.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 progress.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 progress.h |    1 +
 2 files changed, 77 insertions(+), 4 deletions(-)

diff --git a/progress.c b/progress.c
index 7f9b00e..23ee9f3 100644
--- a/progress.c
+++ b/progress.c
@@ -1,6 +1,19 @@
 #include "git-compat-util.h"
 #include "progress.h"
 
+#define TP_IDX_MAX      8
+
+struct throughput {
+	struct timeval prev_tv;
+	unsigned long count;
+	unsigned long avg_bytes;
+	unsigned long last_bytes[TP_IDX_MAX];
+	unsigned int avg_misecs;
+	unsigned int last_misecs[TP_IDX_MAX];
+	unsigned int idx;
+	char display[20];
+};
+
 struct progress {
 	const char *title;
 	int last_value;
@@ -8,6 +21,7 @@ struct progress {
 	unsigned last_percent;
 	unsigned delay;
 	unsigned delayed_percent_treshold;
+	struct throughput *throughput;
 };
 
 static volatile sig_atomic_t progress_update;
@@ -46,7 +60,7 @@ static void clear_progress_signal(void)
 
 static int display(struct progress *progress, unsigned n, int done)
 {
-	char *eol;
+	char *eol, *tp;
 
 	if (progress->delay) {
 		if (!progress_update || --progress->delay)
@@ -64,18 +78,20 @@ static int display(struct progress *progress, unsigned n, int done)
 	}
 
 	progress->last_value = n;
+	tp = (progress->throughput) ? progress->throughput->display : "";
 	eol = done ? ", done.   \n" : "   \r";
 	if (progress->total) {
 		unsigned percent = n * 100 / progress->total;
 		if (percent != progress->last_percent || progress_update) {
 			progress->last_percent = percent;
-			fprintf(stderr, "%s: %3u%% (%u/%u)%s", progress->title,
-				percent, n, progress->total, eol);
+			fprintf(stderr, "%s: %3u%% (%u/%u)%s%s",
+				progress->title, percent, n,
+				progress->total, tp, eol);
 			progress_update = 0;
 			return 1;
 		}
 	} else if (progress_update) {
-		fprintf(stderr, "%s: %u%s", progress->title, n, eol);
+		fprintf(stderr, "%s: %u%s%s", progress->title, n, tp, eol);
 		progress_update = 0;
 		return 1;
 	}
@@ -83,6 +99,60 @@ static int display(struct progress *progress, unsigned n, int done)
 	return 0;
 }
 
+void display_throughput(struct progress *progress, unsigned long n)
+{
+	struct throughput *tp;
+	struct timeval tv;
+	unsigned int misecs;
+
+	if (!progress)
+		return;
+	tp = progress->throughput;
+
+	gettimeofday(&tv, NULL);
+
+	if (!tp) {
+		progress->throughput = tp = calloc(1, sizeof(*tp));
+		if (tp)
+			tp->prev_tv = tv;
+		return;
+	}
+
+	tp->count += n;
+
+	/*
+	 * We have x = bytes and y = microsecs.  We want z = KiB/s:
+	 *
+	 *	z = (x / 1024) / (y / 1000000)
+	 *	z = x / y * 1000000 / 1024
+	 *	z = x / (y * 1024 / 1000000)
+	 *	z = x / y'
+	 *
+	 * To simplify things we'll keep track of misecs, or 1024th of a sec
+	 * obtained with:
+	 *
+	 *	y' = y * 1024 / 1000000
+	 *	y' = y / (1000000 / 1024)
+	 *	y' = y / 977
+	 */
+	misecs = (tv.tv_sec - tp->prev_tv.tv_sec) * 1024;
+	misecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;
+
+	if (misecs > 512) {
+		tp->prev_tv = tv;
+		tp->avg_bytes += tp->count;
+		tp->avg_misecs += misecs;
+		snprintf(tp->display, sizeof(tp->display),
+			 ", %lu KiB/s", tp->avg_bytes / tp->avg_misecs);
+		tp->avg_bytes -= tp->last_bytes[tp->idx];
+		tp->avg_misecs -= tp->last_misecs[tp->idx];
+		tp->last_bytes[tp->idx] = tp->count;
+		tp->last_misecs[tp->idx] = misecs;
+		tp->idx = (tp->idx + 1) % TP_IDX_MAX;
+		tp->count = 0;
+	}
+}
+
 int display_progress(struct progress *progress, unsigned n)
 {
 	return progress ? display(progress, n, 0) : 0;
@@ -103,6 +173,7 @@ struct progress * start_progress_delay(const char *title, unsigned total,
 	progress->last_percent = -1;
 	progress->delayed_percent_treshold = percent_treshold;
 	progress->delay = delay;
+	progress->throughput = NULL;
 	set_progress_signal();
 	return progress;
 }
@@ -124,5 +195,6 @@ void stop_progress(struct progress **p_progress)
 		display(progress, progress->last_value, 1);
 	}
 	clear_progress_signal();
+	free(progress->throughput);
 	free(progress);
 }
diff --git a/progress.h b/progress.h
index 29780ce..7902ca5 100644
--- a/progress.h
+++ b/progress.h
@@ -3,6 +3,7 @@
 
 struct progress;
 
+void display_throughput(struct progress *progress, unsigned long n);
 int display_progress(struct progress *progress, unsigned n);
 struct progress * start_progress(const char *title, unsigned total);
 struct progress * start_progress_delay(const char *title, unsigned total,
-- 
1.5.3.4.1463.gf79ad2

^ permalink raw reply related

* [PATCH 5/5] add throughput display to index-pack
From: Nicolas Pitre @ 2007-10-30 18:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1193770655-20492-5-git-send-email-nico@cam.org>

... and call it "Receiving objects" when over stdin to look clearer
to end users.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 index-pack.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/index-pack.c b/index-pack.c
index 879ea15..61ea762 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -87,6 +87,8 @@ static void *fill(int min)
 				die("early EOF");
 			die("read error on input: %s", strerror(errno));
 		}
+		if (from_stdin)
+			display_throughput(progress, ret);
 		input_len += ret;
 	} while (input_len < min);
 	return input_buffer;
@@ -406,7 +408,9 @@ static void parse_pack_objects(unsigned char *sha1)
 	 * - remember base (SHA1 or offset) for all deltas.
 	 */
 	if (verbose)
-		progress = start_progress("Indexing objects", nr_objects);
+		progress = start_progress(
+				from_stdin ? "Receiving objects" : "Indexing objects",
+				nr_objects);
 	for (i = 0; i < nr_objects; i++) {
 		struct object_entry *obj = &objects[i];
 		data = unpack_raw_entry(obj, &delta->base);
-- 
1.5.3.4.1463.gf79ad2

^ permalink raw reply related

* [PATCH 3/5] relax usage of the progress API
From: Nicolas Pitre @ 2007-10-30 18:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1193770655-20492-3-git-send-email-nico@cam.org>

Since it is now OK to pass a null pointer to display_progress() and
stop_progress() resulting in a no-op, then we can simplify the code
and remove a bunch of lines by not making those calls conditional all
the time.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 builtin-pack-objects.c   |   18 ++++++------------
 builtin-prune-packed.c   |    6 ++----
 builtin-unpack-objects.c |    6 ++----
 index-pack.c             |   20 +++++++-------------
 unpack-trees.c           |    8 +++-----
 5 files changed, 20 insertions(+), 38 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 3ca5cc7..52a26a2 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -629,8 +629,7 @@ static void write_pack_file(void)
 			if (!offset_one)
 				break;
 			offset = offset_one;
-			if (do_progress)
-				display_progress(progress_state, written);
+			display_progress(progress_state, written);
 		}
 
 		/*
@@ -684,8 +683,7 @@ static void write_pack_file(void)
 	} while (nr_remaining && i < nr_objects);
 
 	free(written_list);
-	if (do_progress)
-		stop_progress(&progress_state);
+	stop_progress(&progress_state);
 	if (written != nr_result)
 		die("wrote %u objects while expecting %u", written, nr_result);
 	/*
@@ -853,8 +851,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
 	else
 		object_ix[-1 - ix] = nr_objects;
 
-	if (progress)
-		display_progress(progress_state, nr_objects);
+	display_progress(progress_state, nr_objects);
 
 	if (name && no_try_delta(name))
 		entry->no_try_delta = 1;
@@ -1517,8 +1514,7 @@ static void find_deltas(struct object_entry **list, unsigned list_size,
 
 		progress_lock();
 		(*processed)++;
-		if (progress)
-			display_progress(progress_state, *processed);
+		display_progress(progress_state, *processed);
 		progress_unlock();
 
 		/*
@@ -1722,8 +1718,7 @@ static void prepare_pack(int window, int depth)
 							nr_deltas);
 		qsort(delta_list, n, sizeof(*delta_list), type_size_sort);
 		ll_find_deltas(delta_list, n, window+1, depth, &nr_done);
-		if (progress)
-			stop_progress(&progress_state);
+		stop_progress(&progress_state);
 		if (nr_done != nr_deltas)
 			die("inconsistency with delta count");
 	}
@@ -2142,8 +2137,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 		rp_av[rp_ac] = NULL;
 		get_object_list(rp_ac, rp_av);
 	}
-	if (progress)
-		stop_progress(&progress_state);
+	stop_progress(&progress_state);
 
 	if (non_empty && !nr_result)
 		return 0;
diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c
index c66fb03..f4287da 100644
--- a/builtin-prune-packed.c
+++ b/builtin-prune-packed.c
@@ -15,8 +15,7 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts)
 	struct dirent *de;
 	char hex[40];
 
-	if (opts == VERBOSE)
-		display_progress(progress, i + 1);
+	display_progress(progress, i + 1);
 
 	sprintf(hex, "%02x", i);
 	while ((de = readdir(dir)) != NULL) {
@@ -64,8 +63,7 @@ void prune_packed_objects(int opts)
 		prune_dir(i, d, pathname, len + 3, opts);
 		closedir(d);
 	}
-	if (opts == VERBOSE)
-		stop_progress(&progress);
+	stop_progress(&progress);
 }
 
 int cmd_prune_packed(int argc, const char **argv, const char *prefix)
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index e0ecbc5..1e51865 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -326,11 +326,9 @@ static void unpack_all(void)
 	obj_list = xmalloc(nr_objects * sizeof(*obj_list));
 	for (i = 0; i < nr_objects; i++) {
 		unpack_one(i);
-		if (!quiet)
-			display_progress(progress, i + 1);
+		display_progress(progress, i + 1);
 	}
-	if (!quiet)
-		stop_progress(&progress);
+	stop_progress(&progress);
 
 	if (delta_list)
 		die("unresolved deltas left after unpacking");
diff --git a/index-pack.c b/index-pack.c
index b4543a4..879ea15 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -418,12 +418,10 @@ static void parse_pack_objects(unsigned char *sha1)
 		} else
 			sha1_object(data, obj->size, obj->type, obj->idx.sha1);
 		free(data);
-		if (verbose)
-			display_progress(progress, i+1);
+		display_progress(progress, i+1);
 	}
 	objects[i].idx.offset = consumed_bytes;
-	if (verbose)
-		stop_progress(&progress);
+	stop_progress(&progress);
 
 	/* Check pack integrity */
 	flush();
@@ -486,8 +484,7 @@ static void parse_pack_objects(unsigned char *sha1)
 						      obj->size, obj->type);
 			}
 		free(data);
-		if (verbose)
-			display_progress(progress, nr_resolved_deltas);
+		display_progress(progress, nr_resolved_deltas);
 	}
 }
 
@@ -594,8 +591,7 @@ static void fix_unresolved_deltas(int nr_unresolved)
 			die("local object %s is corrupt", sha1_to_hex(d->base.sha1));
 		append_obj_to_pack(d->base.sha1, data, size, type);
 		free(data);
-		if (verbose)
-			display_progress(progress, nr_resolved_deltas);
+		display_progress(progress, nr_resolved_deltas);
 	}
 	free(sorted_by_pos);
 }
@@ -774,8 +770,7 @@ int main(int argc, char **argv)
 	deltas = xmalloc(nr_objects * sizeof(struct delta_entry));
 	parse_pack_objects(sha1);
 	if (nr_deltas == nr_resolved_deltas) {
-		if (verbose)
-			stop_progress(&progress);
+		stop_progress(&progress);
 		/* Flush remaining pack final 20-byte SHA1. */
 		flush();
 	} else {
@@ -788,11 +783,10 @@ int main(int argc, char **argv)
 					   (nr_objects + nr_unresolved + 1)
 					   * sizeof(*objects));
 			fix_unresolved_deltas(nr_unresolved);
-			if (verbose) {
-				stop_progress(&progress);
+			stop_progress(&progress);
+			if (verbose)
 				fprintf(stderr, "%d objects were added to complete this thin pack.\n",
 					nr_objects - nr_objects_initial);
-			}
 			fixup_pack_header_footer(output_fd, sha1,
 				curr_pack, nr_objects);
 		}
diff --git a/unpack-trees.c b/unpack-trees.c
index 6776c29..c527d7d 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -316,9 +316,8 @@ static void check_updates(struct cache_entry **src, int nr,
 	while (nr--) {
 		struct cache_entry *ce = *src++;
 
-		if (total)
-			if (!ce->ce_mode || ce->ce_flags & mask)
-				display_progress(progress, ++cnt);
+		if (!ce->ce_mode || ce->ce_flags & mask)
+			display_progress(progress, ++cnt);
 		if (!ce->ce_mode) {
 			if (o->update)
 				unlink_entry(ce->name, last_symlink);
@@ -332,8 +331,7 @@ static void check_updates(struct cache_entry **src, int nr,
 			}
 		}
 	}
-	if (total)
-		stop_progress(&progress);
+	stop_progress(&progress);
 }
 
 int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
-- 
1.5.3.4.1463.gf79ad2

^ permalink raw reply related

* [PATCH 2/5] make struct progress an opaque type
From: Nicolas Pitre @ 2007-10-30 18:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1193770655-20492-2-git-send-email-nico@cam.org>

This allows for better management of progress "object" existence,
as well as making the progress display implementation more independent
from its callers.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 builtin-pack-objects.c   |   16 ++++++++--------
 builtin-prune-packed.c   |    7 +++----
 builtin-unpack-objects.c |    6 +++---
 index-pack.c             |   12 ++++++------
 progress.c               |   33 +++++++++++++++++++++++++++------
 progress.h               |   18 +++++-------------
 unpack-trees.c           |   10 +++++-----
 7 files changed, 57 insertions(+), 45 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 21ba977..3ca5cc7 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -73,7 +73,7 @@ static int depth = 50;
 static int delta_search_threads = 1;
 static int pack_to_stdout;
 static int num_preferred_base;
-static struct progress progress_state;
+static struct progress *progress_state;
 static int pack_compression_level = Z_DEFAULT_COMPRESSION;
 static int pack_compression_seen;
 
@@ -598,7 +598,7 @@ static void write_pack_file(void)
 	uint32_t nr_remaining = nr_result;
 
 	if (do_progress)
-		start_progress(&progress_state, "Writing objects", nr_result);
+		progress_state = start_progress("Writing objects", nr_result);
 	written_list = xmalloc(nr_objects * sizeof(struct object_entry *));
 
 	do {
@@ -630,7 +630,7 @@ static void write_pack_file(void)
 				break;
 			offset = offset_one;
 			if (do_progress)
-				display_progress(&progress_state, written);
+				display_progress(progress_state, written);
 		}
 
 		/*
@@ -854,7 +854,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
 		object_ix[-1 - ix] = nr_objects;
 
 	if (progress)
-		display_progress(&progress_state, nr_objects);
+		display_progress(progress_state, nr_objects);
 
 	if (name && no_try_delta(name))
 		entry->no_try_delta = 1;
@@ -1518,7 +1518,7 @@ static void find_deltas(struct object_entry **list, unsigned list_size,
 		progress_lock();
 		(*processed)++;
 		if (progress)
-			display_progress(&progress_state, *processed);
+			display_progress(progress_state, *processed);
 		progress_unlock();
 
 		/*
@@ -1718,8 +1718,8 @@ static void prepare_pack(int window, int depth)
 	if (nr_deltas && n > 1) {
 		unsigned nr_done = 0;
 		if (progress)
-			start_progress(&progress_state, "Compressing objects",
-					nr_deltas);
+			progress_state = start_progress("Compressing objects",
+							nr_deltas);
 		qsort(delta_list, n, sizeof(*delta_list), type_size_sort);
 		ll_find_deltas(delta_list, n, window+1, depth, &nr_done);
 		if (progress)
@@ -2135,7 +2135,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	prepare_packed_git();
 
 	if (progress)
-		start_progress(&progress_state, "Counting objects", 0);
+		progress_state = start_progress("Counting objects", 0);
 	if (!use_internal_rev_list)
 		read_object_list_from_stdin();
 	else {
diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c
index 907e368..c66fb03 100644
--- a/builtin-prune-packed.c
+++ b/builtin-prune-packed.c
@@ -8,7 +8,7 @@ static const char prune_packed_usage[] =
 #define DRY_RUN 01
 #define VERBOSE 02
 
-static struct progress progress;
+static struct progress *progress;
 
 static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts)
 {
@@ -16,7 +16,7 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts)
 	char hex[40];
 
 	if (opts == VERBOSE)
-		display_progress(&progress, i + 1);
+		display_progress(progress, i + 1);
 
 	sprintf(hex, "%02x", i);
 	while ((de = readdir(dir)) != NULL) {
@@ -46,8 +46,7 @@ void prune_packed_objects(int opts)
 	int len = strlen(dir);
 
 	if (opts == VERBOSE)
-		start_progress_delay(&progress,
-			"Removing duplicate objects",
+		progress = start_progress_delay("Removing duplicate objects",
 			256, 95, 2);
 
 	if (len > PATH_MAX - 42)
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 2317b8f..e0ecbc5 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -311,7 +311,7 @@ static void unpack_one(unsigned nr)
 static void unpack_all(void)
 {
 	int i;
-	struct progress progress;
+	struct progress *progress = NULL;
 	struct pack_header *hdr = fill(sizeof(struct pack_header));
 	unsigned nr_objects = ntohl(hdr->hdr_entries);
 
@@ -322,12 +322,12 @@ static void unpack_all(void)
 	use(sizeof(struct pack_header));
 
 	if (!quiet)
-		start_progress(&progress, "Unpacking objects", nr_objects);
+		progress = start_progress("Unpacking objects", nr_objects);
 	obj_list = xmalloc(nr_objects * sizeof(*obj_list));
 	for (i = 0; i < nr_objects; i++) {
 		unpack_one(i);
 		if (!quiet)
-			display_progress(&progress, i + 1);
+			display_progress(progress, i + 1);
 	}
 	if (!quiet)
 		stop_progress(&progress);
diff --git a/index-pack.c b/index-pack.c
index 2f149a4..b4543a4 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -46,7 +46,7 @@ static int nr_resolved_deltas;
 static int from_stdin;
 static int verbose;
 
-static struct progress progress;
+static struct progress *progress;
 
 /* We always read in 4kB chunks. */
 static unsigned char input_buffer[4096];
@@ -406,7 +406,7 @@ static void parse_pack_objects(unsigned char *sha1)
 	 * - remember base (SHA1 or offset) for all deltas.
 	 */
 	if (verbose)
-		start_progress(&progress, "Indexing objects", nr_objects);
+		progress = start_progress("Indexing objects", nr_objects);
 	for (i = 0; i < nr_objects; i++) {
 		struct object_entry *obj = &objects[i];
 		data = unpack_raw_entry(obj, &delta->base);
@@ -419,7 +419,7 @@ static void parse_pack_objects(unsigned char *sha1)
 			sha1_object(data, obj->size, obj->type, obj->idx.sha1);
 		free(data);
 		if (verbose)
-			display_progress(&progress, i+1);
+			display_progress(progress, i+1);
 	}
 	objects[i].idx.offset = consumed_bytes;
 	if (verbose)
@@ -455,7 +455,7 @@ static void parse_pack_objects(unsigned char *sha1)
 	 *   for some more deltas.
 	 */
 	if (verbose)
-		start_progress(&progress, "Resolving deltas", nr_deltas);
+		progress = start_progress("Resolving deltas", nr_deltas);
 	for (i = 0; i < nr_objects; i++) {
 		struct object_entry *obj = &objects[i];
 		union delta_base base;
@@ -487,7 +487,7 @@ static void parse_pack_objects(unsigned char *sha1)
 			}
 		free(data);
 		if (verbose)
-			display_progress(&progress, nr_resolved_deltas);
+			display_progress(progress, nr_resolved_deltas);
 	}
 }
 
@@ -595,7 +595,7 @@ static void fix_unresolved_deltas(int nr_unresolved)
 		append_obj_to_pack(d->base.sha1, data, size, type);
 		free(data);
 		if (verbose)
-			display_progress(&progress, nr_resolved_deltas);
+			display_progress(progress, nr_resolved_deltas);
 	}
 	free(sorted_by_pos);
 }
diff --git a/progress.c b/progress.c
index 7629e05..7f9b00e 100644
--- a/progress.c
+++ b/progress.c
@@ -1,6 +1,15 @@
 #include "git-compat-util.h"
 #include "progress.h"
 
+struct progress {
+	const char *title;
+	int last_value;
+	unsigned total;
+	unsigned last_percent;
+	unsigned delay;
+	unsigned delayed_percent_treshold;
+};
+
 static volatile sig_atomic_t progress_update;
 
 static void progress_interval(int signum)
@@ -76,12 +85,18 @@ static int display(struct progress *progress, unsigned n, int done)
 
 int display_progress(struct progress *progress, unsigned n)
 {
-	return display(progress, n, 0);
+	return progress ? display(progress, n, 0) : 0;
 }
 
-void start_progress_delay(struct progress *progress, const char *title,
-			  unsigned total, unsigned percent_treshold, unsigned delay)
+struct progress * start_progress_delay(const char *title, unsigned total,
+				       unsigned percent_treshold, unsigned delay)
 {
+	struct progress *progress = malloc(sizeof(*progress));
+	if (!progress) {
+		/* unlikely, but here's a good fallback */
+		fprintf(stderr, "%s...\n", title);
+		return NULL;
+	}
 	progress->title = title;
 	progress->total = total;
 	progress->last_value = -1;
@@ -89,19 +104,25 @@ void start_progress_delay(struct progress *progress, const char *title,
 	progress->delayed_percent_treshold = percent_treshold;
 	progress->delay = delay;
 	set_progress_signal();
+	return progress;
 }
 
-void start_progress(struct progress *progress, const char *title, unsigned total)
+struct progress * start_progress(const char *title, unsigned total)
 {
-	start_progress_delay(progress, title, total, 0, 0);
+	return start_progress_delay(title, total, 0, 0);
 }
 
-void stop_progress(struct progress *progress)
+void stop_progress(struct progress **p_progress)
 {
+	struct progress *progress = *p_progress;
+	if (!progress)
+		return;
+	*p_progress = NULL;
 	if (progress->last_value != -1) {
 		/* Force the last update */
 		progress_update = 1;
 		display(progress, progress->last_value, 1);
 	}
 	clear_progress_signal();
+	free(progress);
 }
diff --git a/progress.h b/progress.h
index 07b56bd..29780ce 100644
--- a/progress.h
+++ b/progress.h
@@ -1,20 +1,12 @@
 #ifndef PROGRESS_H
 #define PROGRESS_H
 
-struct progress {
-	const char *title;
-	int last_value;
-	unsigned total;
-	unsigned last_percent;
-	unsigned delay;
-	unsigned delayed_percent_treshold;
-};
+struct progress;
 
 int display_progress(struct progress *progress, unsigned n);
-void start_progress(struct progress *progress, const char *title,
-		    unsigned total);
-void start_progress_delay(struct progress *progress, const char *title,
-			  unsigned total, unsigned percent_treshold, unsigned delay);
-void stop_progress(struct progress *progress);
+struct progress * start_progress(const char *title, unsigned total);
+struct progress * start_progress_delay(const char *title, unsigned total,
+				       unsigned percent_treshold, unsigned delay);
+void stop_progress(struct progress **progress);
 
 #endif
diff --git a/unpack-trees.c b/unpack-trees.c
index 3225101..6776c29 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -297,7 +297,7 @@ static void check_updates(struct cache_entry **src, int nr,
 {
 	unsigned short mask = htons(CE_UPDATE);
 	unsigned cnt = 0, total = 0;
-	struct progress progress;
+	struct progress *progress = NULL;
 	char last_symlink[PATH_MAX];
 
 	if (o->update && o->verbose_update) {
@@ -307,8 +307,8 @@ static void check_updates(struct cache_entry **src, int nr,
 				total++;
 		}
 
-		start_progress_delay(&progress, "Checking out files",
-				     total, 50, 2);
+		progress = start_progress_delay("Checking out files",
+						total, 50, 2);
 		cnt = 0;
 	}
 
@@ -318,7 +318,7 @@ static void check_updates(struct cache_entry **src, int nr,
 
 		if (total)
 			if (!ce->ce_mode || ce->ce_flags & mask)
-				display_progress(&progress, ++cnt);
+				display_progress(progress, ++cnt);
 		if (!ce->ce_mode) {
 			if (o->update)
 				unlink_entry(ce->name, last_symlink);
@@ -333,7 +333,7 @@ static void check_updates(struct cache_entry **src, int nr,
 		}
 	}
 	if (total)
-		stop_progress(&progress);;
+		stop_progress(&progress);
 }
 
 int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
-- 
1.5.3.4.1463.gf79ad2

^ permalink raw reply related

* [PATCH 1/5] prune-packed: don't call display_progress() for every file
From: Nicolas Pitre @ 2007-10-30 18:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1193770655-20492-1-git-send-email-nico@cam.org>

The progress count is per fanout directory, so it is useless to call
it for every file as the count doesn't change that often.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 builtin-prune-packed.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c
index 015c8bb..907e368 100644
--- a/builtin-prune-packed.c
+++ b/builtin-prune-packed.c
@@ -15,6 +15,9 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts)
 	struct dirent *de;
 	char hex[40];
 
+	if (opts == VERBOSE)
+		display_progress(&progress, i + 1);
+
 	sprintf(hex, "%02x", i);
 	while ((de = readdir(dir)) != NULL) {
 		unsigned char sha1[20];
@@ -26,8 +29,6 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts)
 		if (!has_sha1_pack(sha1, NULL))
 			continue;
 		memcpy(pathname + len, de->d_name, 38);
-		if (opts == VERBOSE)
-			display_progress(&progress, i + 1);
 		if (opts & DRY_RUN)
 			printf("rm -f %s\n", pathname);
 		else if (unlink(pathname) < 0)
-- 
1.5.3.4.1463.gf79ad2

^ permalink raw reply related

* [PATCH 0/5] more progress display stuff
From: Nicolas Pitre @ 2007-10-30 18:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This is the result of some changes and cleanup I did to nicely support
progress with throughput display, and avoid those random crashes the
previous patches introduced.  All tests are OK now after every patch.

This is meant to replace my latest 2 patches currently in pu.


Nicolas

^ permalink raw reply

* Re: git-merge: inconsistent manual page.
From: Junio C Hamano @ 2007-10-30 18:54 UTC (permalink / raw)
  To: Sergei Organov; +Cc: git
In-Reply-To: <7vsl3sla5q.fsf@gitster.siamese.dyndns.org>

Subject: git-merge: document but discourage the historical syntax

Historically "git merge" took its command line arguments in a
rather strange order.  Document the historical syntax, and also
document clearly that it is not encouraged in new scripts.

There is no reason to deprecate the historical syntax, as the
current code can sanely tell which syntax the caller is using,
and existing scripts by people do use the historical syntax.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

  Junio C Hamano <gitster@pobox.com> writes:

  > See above.  We do not want to advertise the crazy syntax.

  By the way, I kept saying "crazy" above, but the thing is that
  when Linus did that "crazy" syntax, it was not crazy at all.
  Simply it did not matter, as nobody was supposed to use "git
  merge" from the command line.

  Instead, when merging a local branch, you would just have said:

          $ git pull . $my_other_branch

  It became a "crazy historical syntax" only after people started
  talking about using it from the command line, giving it other
  command line options.  And at that point, we introduced -m flag
  and stopped requiring the second token 'HEAD'.

  With that in mind, this might be a better alternative.

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

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index bca4212..a056b40 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 [verse]
 'git-merge' [-n] [--summary] [--no-commit] [--squash] [-s <strategy>]...
 	[-m <msg>] <remote> <remote>...
+'git-merge' <msg> HEAD <remote>...
 
 DESCRIPTION
 -----------
@@ -43,6 +44,11 @@ If you tried a merge which resulted in a complex conflicts and
 would want to start over, you can recover with
 gitlink:git-reset[1].
 
+The second syntax (<msg> `HEAD` <remote>) is supported for
+historical reasons.  Do not use it from the command line or in
+new scripts.
+
+
 CONFIGURATION
 -------------
 

^ permalink raw reply related

* Re: [PATCH] Documentation/git-cvsexportcommit.txt: s/mgs/msg/ in example
From: Junio C Hamano @ 2007-10-30 18:39 UTC (permalink / raw)
  To: Michael W. Olson; +Cc: git
In-Reply-To: <1193752427-21280-1-git-send-email-mwolson@gnu.org>

Thanks.

^ permalink raw reply

* Re: git-merge: inconsistent manual page.
From: Junio C Hamano @ 2007-10-30 18:39 UTC (permalink / raw)
  To: Sergei Organov; +Cc: git
In-Reply-To: <fg7b6o$k1f$1@ger.gmane.org>

Sergei Organov <osv@javad.com> writes:

> git-merge has inconsistent manual page:
>
> 1. In the SYNOPSIS, there is [-m <msg>], but OPTIONS section lacks it.
>    The latter has just  description of <msg>, suggesting it could be
>    used without -m, but SYNOPSIS doesn't reflect this. 

This is not quite right, I am afraid.  "git merge" used to take
its parameters in quite an exotic order, and we still do support
it as we do not break people's existing setups:

	git-merge "$message" HEAD $list_of_merged_refs

where the second parameter is always HEAD.  This is the only
form that accepts the message without -m (see ll.230- in the
git-merge script).

But we do not _want_ to advertise this funny syntax.  Humans and
newly written scripts should express the above as:

	git merge -m "$message" $list_of_merged_refs


>    BTW, git-merge options are described in
>    Documentation/merge-options.txt that is also used fot git-pull
>    options, and it's not clear for me if git-pull supports [-m
>    <msg>]. Does it?

It shouldn't; the merge message is prepared by git-pull to
describe what got merged from where for you (see the last few
lines in the git-pull script).  

> 2. In the SYNOPSIS, there is no <head> that is described in the
>    OPTIONS.

See above.  We do not want to advertise the crazy syntax.

^ permalink raw reply

* Re: [BUG] remote.c/match_explicit() ... NULL pointer dereferenciation (git 1.5.3.4)
From: Junio C Hamano @ 2007-10-30 18:30 UTC (permalink / raw)
  To: Melchior FRANZ; +Cc: git, spearce
In-Reply-To: <200710301144.32528@rk-nord.at>

Subject: Prevent send-pack from segfaulting (backport from 'master')

4491e62ae932d5774f628d1bd3be663c11058a73 (Prevent send-pack from
segfaulting when a branch doesn't match) 

If we can't find a source match, and we have no destination, we
need to abort the match function early before we try to match
the destination against the remote.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

  Thanks.  Shawn fixed it on the 'master' side but 'maint' is
  still using the old code.

 remote.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/remote.c b/remote.c
index cdbbdcb..9a88917 100644
--- a/remote.c
+++ b/remote.c
@@ -504,8 +504,11 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	if (!matched_src)
 		errs = 1;
 
-	if (dst_value == NULL)
+	if (!dst_value) {
+		if (!matched_src)
+			return errs;
 		dst_value = matched_src->name;
+	}
 
 	switch (count_refspec_match(dst_value, dst, &matched_dst)) {
 	case 1:

^ permalink raw reply related

* Re: remote#branch
From: Linus Torvalds @ 2007-10-30 18:19 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Tom Prince, Theodore Tso, Junio C Hamano, Jan Hudec,
	Johannes Schindelin, Petr Baudis, Paolo Ciarrocchi, git
In-Reply-To: <alpine.LFD.0.999.0710301056070.30120@woody.linux-foundation.org>



On Tue, 30 Oct 2007, Linus Torvalds wrote:
> 
> So if you want to make that RFC have any relevance what-so-ever, then show 
> some interoperability issue. Which is why I'm bringing up a web browser: 
> that interop issue simply *does*not*exist*.

Btw, the reason I care is that the whole ".. but it's a standard" thinking 
really is dangerous. It's how you create absolute and utter crap.

It's why I have also brought up XML in this discussion, because XML is a 
classic example of something where this ".. but it's a standard" thinking 
has resulted in really bad solutions. It's pushed as a way to 
"interoperate", but then, since everybody does different things, the only 
thing you can actually interoperate with is by having a common parsing 
library for a REALLY HORRIBLY BAD FORMAT!

The same is true of a URL. What is there to interoperate with? The whole 
(and only) point of the git remote URL is to point git to a different git 
repository. There's no other reason to use it. So the only possible case 
we care about interoperability is with other git uses.

And those other git uses are not RFC1738-quoted, are they? 

			Linus

^ permalink raw reply

* Re: [PATCH] gitweb : disambiguate heads and tags withs the same name
From: Junio C Hamano @ 2007-10-30 18:19 UTC (permalink / raw)
  To: Guillaume Seguin; +Cc: pasky, git
In-Reply-To: <e877c31c0710280612l5d783ab0o50ce00c70b3311db@mail.gmail.com>

"Guillaume Seguin" <guillaume@segu.in> writes:

> Avoid wrong disambiguation that would link logs/trees of tags and heads which
> share the same name to the same page, leading to a disambiguation that would
> prefer the tag, thus making it impossible to access the corresponding
> head log and
> tree without hacking the url by hand.
>
> ---
>  gitweb/gitweb.perl |   14 ++++++++------
>  1 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 48e21da..f918c00 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3534,6 +3534,7 @@ sub git_tags_body {
>  	for (my $i = $from; $i <= $to; $i++) {
>  		my $entry = $taglist->[$i];
>  		my %tag = %$entry;
> +		my $name = "refs/tags/$tag{'name'}";
>  		my $comment = $tag{'subject'};
>  		my $comment_short;
>  		if (defined $comment) {
> @@ -3570,8 +3571,8 @@ sub git_tags_body {
>  		      "<td class=\"link\">" . " | " .
>  		      $cgi->a({-href => href(action=>$tag{'reftype'},
> hash=>$tag{'refid'})}, $tag{'reftype'});
>  		if ($tag{'reftype'} eq "commit") {
> -			print " | " . $cgi->a({-href => href(action=>"shortlog",
> hash=>$tag{'name'})}, "shortlog") .
> -			      " | " . $cgi->a({-href => href(action=>"log",
> hash=>$tag{'name'})}, "log");
> +			print " | " . $cgi->a({-href => href(action=>"shortlog",
> hash=>$name)}, "shortlog") .
> ...

Just in case anybody is wondering, the patch is whitespace
mangled and lacks a sign-off.

I suspect what the patch does may be a good idea, although I
haven't followed the existing code closely to verify it.

^ permalink raw reply

* Re: [PATCH 10/10] push: teach push to be quiet if local ref is strict subset of remote ref
From: Daniel Barkalow @ 2007-10-30 18:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steffen Prohaska, git
In-Reply-To: <7vfxztm2dx.fsf@gitster.siamese.dyndns.org>

On Tue, 30 Oct 2007, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
> 
> > git push now allows you pushing a couple of branches that have
> > advanced, while ignoring all branches that have no local changes,
> > but are lagging behind their matching remote refs. This is done
> > without reporting errors.
> >
> > Thanks to Junio C. Hamano <gitster@pobox.com> for suggesting to
> > report in the summary that refs have been ignored.
> 
> I do not think this is a good idea at all.  Furthermore, I never
> suggested anything about summary.  You are robbing the
> information from the pusher which ones are pushed and which ones
> are left behind.

I think this case should be a warning rather than an error, though. It is 
certainly true that the user isn't intending to update those remote refs, 
because there is no local change to update them with. And it is also true 
that those local refs being stale is no impediment to updating the refs 
which are not stale, which is what the user does intend to do. I can't see 
a workflow which would be hurt by this change, because we know that, if 
the user follows the instructions and then tries the push again, it will 
have no effect.

If the concern is robbing the user of information, we should simply 
provide the information, rather than interrupting the user's work to make 
them act on the information before completing the essentially independant 
operation they're attempting.

In any case, it's misleading to suggest that the user "pull first", 
because we know that there would be no effect to pushing again after 
merging. In this case, it would be more accurate to suggest that the user 
"pull instead". Perhaps the message should be
"%s: nothing to push to %s, but you are not up-to-date and may want to 
pull"

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: remote#branch
From: Linus Torvalds @ 2007-10-30 17:58 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Tom Prince, Theodore Tso, Junio C Hamano, Jan Hudec,
	Johannes Schindelin, Petr Baudis, Paolo Ciarrocchi, git
In-Reply-To: <vpq8x5kh4rr.fsf@bauges.imag.fr>



On Tue, 30 Oct 2007, Matthieu Moy wrote:
>
> Linus Torvalds <torvalds@linux-foundation.org> writes:
> 
> > Nobody cares about git being consistent with a web browser.
> 
> Why do you keep talking about web browser?
> 
> URLs are _not_ a web-browser thing. A web browser is just _one_
> example of program which uses URLs.

I keep talking about a web browser, because THE ONLY POINT of following a 
standard is to interoperate.

So if you cannot find something to interoperate with, why the hell would 
you care about the standard?

So here's a question: why do people bother to quote irrelevant RFC's?

Following those RFC's would make git not interoperate WITH ITSELF, and use 
illogically different formats for the same things.

So if you want to make that RFC have any relevance what-so-ever, then show 
some interoperability issue. Which is why I'm bringing up a web browser: 
that interop issue simply *does*not*exist*.

Why is that so hard to understand?

			Linus

^ permalink raw reply

* Re: remote#branch
From: Matthieu Moy @ 2007-10-30 17:49 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Tom Prince, Theodore Tso, Junio C Hamano, Jan Hudec,
	Johannes Schindelin, Petr Baudis, Paolo Ciarrocchi, git
In-Reply-To: <alpine.LFD.0.999.0710301037120.30120@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Nobody cares about git being consistent with a web browser.

Why do you keep talking about web browser?

URLs are _not_ a web-browser thing. A web browser is just _one_
example of program which uses URLs.

-- 
Matthieu

^ permalink raw reply

* Re: remote#branch
From: Linus Torvalds @ 2007-10-30 17:39 UTC (permalink / raw)
  To: Tom Prince
  Cc: Theodore Tso, Junio C Hamano, Jan Hudec, Johannes Schindelin,
	Petr Baudis, Paolo Ciarrocchi, git
In-Reply-To: <20071030160232.GB2640@hermes.priv>



On Tue, 30 Oct 2007, Tom Prince wrote:
> >
> >  - <remote shorthand> ("origin")
> >  - <path> ("../git.git")
> >  - <host>:<path> ("master.kernel.org:/pub/scm/...")
> >  - <protocol>://<host>/<path> ("git://repo.or.cz/...")
> >
> > See? We may not follow RFC's, but we follow "easy to use".
>
> Well, only the last one actually looks like a URL, so that is the only this
> discussion is about.

NO.

The thing is, we'd be much better off being consistent with OURSELVES than
with something else!

Nobody cares about git being consistent with a web browser. There is
nothing in common.

But I *do* care about git being consistent with itself. If I do

	git clone /some/directory

and then decide that I want to generate a new pack and change it into

	git clone file:///some/directory

I don't want to have to re-write the thing to quote differently!

The same very much goes for a path like

	git://git.kernel.org/<path>

vs

	master.kernel.org:<path>

because I will use the two interchangably. They *are* the same address,
except:

 - the "git://" protocol is a bit faster, since the ssh connection
   overhead is actually big enough to be quite noticeable.

 - but I often use the master.kernel.org:<path> thing because there's a
   mirroring delay that means that accessing it directly is sometimes
   preferable.

See? THAT is where we need to be consistent: with our own paths!

[ And yes, I literally really do switch things around exactly like that 
  between ssh accesses and the git:// protocol. That was not a made-up 
  example, but real usage! ]

In contrast, nobody has _ever_ given a real technical reason to care about
the Web URL RFC at all.

Really. It's that simple: if you cannot argue for something without
pointing to an irrelevant standard, you really shouldn't argue for it in
the first place.

People who make decisions based on "it's a standard" make *sub*standard
decisions. The fact is, most standards are not worth even using as toilet
paper, because they were designed by some committee that wanted to reach
"consensus". That's just crap.

                        Linus

^ permalink raw reply

* Re: [PATCH 0/4] Build in some more things
From: Daniel Barkalow @ 2007-10-30 16:49 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <4726DC3D.2030202@viscovery.net>

On Tue, 30 Oct 2007, Johannes Sixt wrote:

> Daniel Barkalow schrieb:
> > The main effect of this series is removing the fork/exec from pushing via
> > the git protocol (aside from the later fork/exec in connect.c of course).
> > 
> > It also heads off some tempting transport-related fetch bugs, which I will
> > not introduce in a later patch.
> > 
> > * Miscellaneous const changes and utilities
> >   Adds two small utility functions, and marks a bunch of stuff as const; the
> >   const stuff is to keep builtin-fetch from getting messed up without a
> >   warning, because it wants some lists not to change.
> > 
> > * Build-in peek-remote, using transport infrastructure.
> > * Build-in send-pack, with an API for other programs to call.
> > * Use built-in send-pack.
> 
> I assume this goes on top of current master or db/fetch-pack. The patches have
> some conflicts with js/forkexec (nothing serious, though). Maybe it makes
> sense to rebase on top of that.

Current master. As I said to Junio a moment ago (and forgot to cc you, 
oops), I think 1/4 should go before js/forkexec, being trivial, and 2/4 
should also, since it simply removes duplicate code that js/forkexec 
updates; I should redo 3/4 after the code settles down, and 4/4 is trivial 
but depends on 3/4.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH 0/4] Build in some more things
From: Daniel Barkalow @ 2007-10-30 16:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzly1nhmn.fsf@gitster.siamese.dyndns.org>

On Tue, 30 Oct 2007, Junio C Hamano wrote:

> I've merged this to 'pu', but honestly speaking, the conflicts
> are geting a bit more "interesting" than I care to keep
> repeating even with help from rerere, with four people
> simultaneously touching the neighbouring code in their topics.

I mostly actually care about [PATCH 1/4] which is also probably the 
easiest to validate (since it really shouldn't change anything that 
matters except to make things work in more cases). If I could get that in 
early, I'd be fine with pushing the rest off until other people are out of 
that code. The reason I care about 1/4 is that it also sets up an 
extensive patch to make the fetch side of transport use the same git 
connection for getting the initial list of remote refs and for fetching 
their content (still with a second connection for auto-following tags, 
since I couldn't figure out how to request more content after examining 
some fetched content); this patch miraculously doesn't seem to 
significantly conflict with anything currently in pu, and actually 
provides a user-visible benefit (ssh transport with no special 
infrastructure doesn't require typing the password multiple times for a 
single logical operation most of the time).

On the other hand, [PATCH 2/4] might be worth floating earlier in pu, 
since its conflicts are to simply remove all of the code that the other 
patches update (since that code duplicates code in transport.c that can be 
used instead).

For the builtin-send-pack stuff, I might as well redo the same logical 
changes to the code once it settles down, since the edit sequence for 3/4 
is really not all that well represented by diff, being very nearly "rename 
a bunch of static globals; mark a bunch of things const; move a 
function call down; split a function in half", and I can just redo that 
after all the other changes.

> Topics involved are:
> 
> ** db/remote-builtin (Mon Oct 29 22:03:42 2007 -0400) 4 commits
>  . Use built-in send-pack.
>  . Build-in send-pack, with an API for other programs to call.
>  . Build-in peek-remote, using transport infrastructure.
>  . Miscellaneous const changes and utilities
> 
> *  jk/send-pack (Thu Oct 18 02:17:46 2007 -0400) 2 commits
>  + t5516: test update of local refs on push
>  + send-pack: don't update tracking refs on error
> 
> *  js/forkexec (Fri Oct 19 21:48:06 2007 +0200)
>  + Use start_command() in git_connect() instead of explicit
>    fork/exec.
>  + Change git_connect() to return a struct child_process instead of a
>    pid_t.
> 
> ** sp/push-refspec (Sun Oct 28 18:46:21 2007 +0100)
>  . push: teach push to pass --verbose option to transport layer
>  . push: use same rules as git-rev-parse to resolve refspecs
>  . add ref_abbrev_matches_full_with_rev_parse_rules() comparing
>    abbrev with full ref name
>  . rename ref_matches_abbrev() to
>    ref_abbrev_matches_full_with_fetch_rules()
> 
> Could you please check the result after I push it out?

I agree with all of the conflict resolutions, although I didn't make sure 
that parts that didn't get conflicts merged correctly (but, if it builds, 
it's almost certainly right).

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: remote#branch
From: Tom Prince @ 2007-10-30 16:02 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Theodore Tso, Junio C Hamano, Jan Hudec, Johannes Schindelin,
	Petr Baudis, Paolo Ciarrocchi, git
In-Reply-To: <alpine.LFD.0.999.0710300738550.30120@woody.linux-foundation.org>

On Tue, Oct 30, 2007 at 07:59:45AM -0700, Linus Torvalds wrote:
> > Not that I care, but git should probably handle things consistently.
> 
> Git has been, and *is* entirely consistent. It uses convenient repo names. 
> If you don't want to call them url's, then call them "repository name". 
> Call them whatever. But they are 100% obvious, even if there are multiple 
> forms of them (and *none* of the forms do any quoting at all):
> 
>  - <remote shorthand> ("origin")
>  - <path> ("../git.git")
>  - <host>:<path> ("master.kernel.org:/pub/scm/...")
>  - <protocol>://<host>/<path> ("git://repo.or.cz/...")
> 
> See? We may not follow RFC's, but we follow "easy to use".

Well, only the last one actually looks like a URL, so that is the only this
discussion is about. I don't think anyone is suggesting that the first three
be changed at all. So, to use your terminology, git has a variety of ways to
specify a repo name, one of which happens to be a URL (or looks like one). The
suggestion is that we should make that way (and only that way) behave like a
RFC URL.

And git should be consistent with web browsers, automatically quoting things
it gets passed. I think the only point of contention is probably how to deal
with URLs that git receives that are already quoted.

1. We ignore the quoting and re-encode everything for the http transport.
2. We honour the encoding and decode everything for the git transport.
3. We handle git:// and http:// different, so that the three git:// URLs below
refer to different repositories, while the three http:// URLs give refer to
the same repository.

> > git://repo.or.cz/linux-2.6/linux acpi-2.6/ibm-acpi-2.6.git
> > git://repo.or.cz/linux-2.6/linux+acpi-2.6/ibm-acpi-2.6.git
> > git://repo.or.cz/linux-2.6/linux%20acpi-2.6/ibm-acpi-2.6.git
> 
> > http://repo.or.cz/linux-2.6/linux acpi-2.6/ibm-acpi-2.6.git
> > http://repo.or.cz/linux-2.6/linux+acpi-2.6/ibm-acpi-2.6.git
> > http://repo.or.cz/linux-2.6/linux%20acpi-2.6/ibm-acpi-2.6.git

The third possibility is probably what we do now, which is why I am suggesting
git is inconsistent. The first will fall down when using a repository that is
colocated, and somebody copies a URL from the web browsers location bar (which
will be properly encoded). Which leaves the second.

  Tom

^ permalink raw reply

* Re: [PATCH/RFC 0/3] faster inexact rename handling
From: Linus Torvalds @ 2007-10-30 15:38 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Andy C, Junio C Hamano
In-Reply-To: <20071030134355.GA21342@coredump.intra.peff.net>



On Tue, 30 Oct 2007, Jeff King wrote:
>
> On Mon, Oct 29, 2007 at 10:06:11PM -0700, Linus Torvalds wrote:
> 
> > Have you compared the results? IOW, does it find the *same* renames?
> 
> From my limited testing, it generally finds the same pairs. However,
> there are a number of renames that it _doesn't_ find, because they are
> composed of "uninteresting" lines, dropping them below the minimum
> score. Try (in git.git):
> 
>   git-show --raw -M -l0 :/'Big tool rename'
> 
> with the old and new code. Pairs like Documentation/git-add-script.txt
> -> Documentation/git-add.txt are not found, because the file is composed
> almost entirely of boilerplate.

Ok, that does imply to me that we cannot just drop boilerplate text, 
because the fact is, lots of files contain boilerplate, but people still 
think they are "similar".

We do actually depend on the similarity analysis being "good" - because it 
matters a lot for things like merging. The old code was actually very 
careful indeed, and while it didn't care about things like the exact 
*ordering* of lines (ie moving functions around in the same file resulted 
in the *exact* same fingerprint for the file!) it cared about everything 
else.

> Moving the size normalization into the similarity engine should probably
> fix that, and will let us compare old and new results more accurately.
> I'll try to work on that.

Hmm. I hope that is sufficient. But I suspect it may well not be. 
Especially since you ignore boiler-plate lines for *some* files but not 
others (ie it depends on which file you happen to find it in first).

			Linus

^ permalink raw reply

* Re: Recording merges after repo conversion
From: Johannes Schindelin @ 2007-10-30 15:05 UTC (permalink / raw)
  To: Peter Karlsson; +Cc: Benoit SIGOURE, git
In-Reply-To: <Pine.LNX.4.62.0710301433150.652@perkele.intern.softwolves.pp.se>

Hi,

On Tue, 30 Oct 2007, Peter Karlsson wrote:

> Benoit SIGOURE:
> 
> > I think you can use grafts do achieve this.
> 
> That seems to work, but the grafts list doesn't seem to propagate when I 
> push/pull/clone. Is it possible to get that to work?

No.  Use filter-branch, and publish the cleaned up history (possibly as a 
new branch/repo).

Ciao,
Dscho

^ permalink raw reply

* Re: remote#branch
From: Linus Torvalds @ 2007-10-30 14:59 UTC (permalink / raw)
  To: Tom Prince
  Cc: Theodore Tso, Junio C Hamano, Jan Hudec, Johannes Schindelin,
	Petr Baudis, Paolo Ciarrocchi, git
In-Reply-To: <20071030053732.GA16963@hermes.priv>



On Tue, 30 Oct 2007, Tom Prince wrote:
> > The push url is generally written as
> > 
> > 	repo.or.cz:/srv/git/linux-2.6/linux-acpi-2.6/ibm-acpi-2.6.git
> > 
> > Tough.
> 
> But gitweb (on git.kernel.org and repo.or.cz) both give git:// locators.

Yes, for anonymous pulling.

The thing is, git takes something much more extended than a "RFC url".

We use pathnames. Not a "quoted mess". Regular, bog-standard, normal unix 
pathnames.

On Windows, I assume (and hope) that git uses the native-style Windows 
pathnames, and you can do "git pull d:\system\ugh" if you want to.

And I think we should care a whole lot about interacting with the *user* 
and actual programs that git shares code with, than interacting with some 
idiotic RFC that has absolutely zero to do with us, regardless of whether 
we use the name "url" or not.

For example, the reason we use "host:/path" for pushing over ssh (and 
pulling, for that matter), is that that's the same syntax that scp uses. 
It's a natural fit. And I hope to God nobody seriously argues that we 
shouldn't use regular pathnames on the local disk? 

> > Quick! WHO THE F*CK CARES?

[ Btw, sorry for the french. I blame being tired and ina bad mood, but I 
  also blame the fact that I absolutely *detest* arguments based on 
  standards. If you cannot back it up with a real usage scenario, you 
  shouldn't even mention the standard ]

> So, how should git deal with
> 
> git://repo.or.cz/linux-2.6/linux acpi-2.6/ibm-acpi-2.6.git
> git://repo.or.cz/linux-2.6/linux+acpi-2.6/ibm-acpi-2.6.git
> git://repo.or.cz/linux-2.6/linux%20acpi-2.6/ibm-acpi-2.6.git

The way it has always cared. Git itself does no quoting what-so-ever 
(except for the *argument* quoting etc that it needs).

Now, the *transport* back-end may end up quoting it, of course, the same 
way it may end up using some random protocol. The user shouldn't care 
about the implementation details!

In the case of the git transport, there is no quoting even by the 
transport protocol. In the case of http, libcurl would hopefully quote for 
us.

> compared to 
> 
> http://repo.or.cz/linux-2.6/linux acpi-2.6/ibm-acpi-2.6.git
> http://repo.or.cz/linux-2.6/linux+acpi-2.6/ibm-acpi-2.6.git
> http://repo.or.cz/linux-2.6/linux%20acpi-2.6/ibm-acpi-2.6.git

No difference, what-so-ever, that I can see. Git doesn't quote it.

Notice how the fact that we use http:// doesn't actually mean that you can 
feed the result to a web browser anyway? It's not like you get a "link" 
that git follows. You get a name.

Yes, you can try to "co-locate" things so that something smart 
disambiguates (maybe have an "index.html" file, so a web browser gets a 
gitweb page, and git gets the raw data). But even then, notice how even 
web browser will do the quoting for you: try

	firefox "http://www.google.com/search?q=Html spaces"

just for fun.

Notice? The thing is, "strict RFC following" makes no sense:

 - the git syntax is (and HAS TO BE to be user friendly) a real extension 
   on any "strict RFC URL" anyway, since it is a lot more important to 
   interact well with normal unix tools (ie use regular filenames, and use 
   the same syntax as "cp", "mv" and "find" etc uses!)

   Ergo: we cannot and MUST NOT care about the "URL RFC" too deeply 
   anyway.

 - 

> Not that I care, but git should probably handle things consistently.

Git has been, and *is* entirely consistent. It uses convenient repo names. 
If you don't want to call them url's, then call them "repository name". 
Call them whatever. But they are 100% obvious, even if there are multiple 
forms of them (and *none* of the forms do any quoting at all):

 - <remote shorthand> ("origin")
 - <path> ("../git.git")
 - <host>:<path> ("master.kernel.org:/pub/scm/...")
 - <protocol>://<host>/<path> ("git://repo.or.cz/...")

See? We may not follow RFC's, but we follow "easy to use".

And btw, that's really much much MUCH more important. It's why the git 
config file is in a "ini"-like format. It's readable. It's not the insane 
RFC crap that would result if somebody had decided that standards are more 
important than being sane.

			Linus

^ permalink raw reply

* Re: Recording merges after repo conversion
From: Lars Hjemli @ 2007-10-30 14:29 UTC (permalink / raw)
  To: Peter Karlsson; +Cc: Benoit SIGOURE, git
In-Reply-To: <Pine.LNX.4.62.0710301433150.652@perkele.intern.softwolves.pp.se>

On 10/30/07, Peter Karlsson <peter@softwolves.pp.se> wrote:
> Benoit SIGOURE:
>
> > I think you can use grafts do achieve this.
>
> That seems to work, but the grafts list doesn't seem to propagate when I
> push/pull/clone. Is it possible to get that to work?
>

No, the grafts file is purely local. To achieve your goal, you'd have
to 'git filter-branch' before pushing/cloning. But beware: this _will_
rewrite your current branch(es).

--
larsh

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox