Git development
 help / color / mirror / Atom feed
* Re: [PATCH 3/3] Add a command "fix" to rebase --interactive.
From: Johannes Schindelin @ 2009-12-04 16:57 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git, gitster
In-Reply-To: <6d779d2c244bf5d5b7924cdc5daf66a8186e4bc7.1259934977.git.mhagger@alum.mit.edu>

Hi,

On Fri, 4 Dec 2009, Michael Haggerty wrote:

> The command is like "squash", except that it discards the commit message
> of the corresponding commit.
> 
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>

The only part which made me a bit uneasy was this one:

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0bd3bf7..539413d 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -302,7 +302,7 @@ nth_string () {
>  
>  make_squash_message () {
>  	if test -f "$SQUASH_MSG"; then
> -		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
> +		COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)\(th\|st\|nd\|rd\) commit message.*:/\1/p" \
>  			< "$SQUASH_MSG" | sed -ne '$p')+1))
>  		echo "# This is a combination of $COUNT commits."
>  		sed -e 1d -e '2,/^./{

But it is fine.  (Took this idiot a couple of seconds to figure out why; 
I'd have put "th" last, being a mathematician instead of a computer 
scientist, so my natural numbers start with 1, not 0 ;-)

To be honest, I never thought of "fix" as something I want, but patiently 
deleted the respective commit messages of my fixup commits (usually called 
"amend" or "amend.<something helpful>").

This is brilliant.

ACK

^ permalink raw reply

* Re: git gsoc money
From: Brandon Casey @ 2009-12-04 16:57 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20091204121417.GA20682@coredump.intra.peff.net>

Jeff King wrote:
> On Thu, Dec 03, 2009 at 07:39:35AM -0800, Shawn O. Pearce wrote:
> 
>>>   1. Become an affiliated project of an organization like The Software
>>>      Freedom Conservancy or Software in the Public Interest.
>> Try to join the Software Freedom Conservancy and retain the funds
>> for Git's use?  Maybe you can slide in before the Dec 31st deadline.
> 
> Everybody who responded seems to think that is a good idea, so I've
> contacted the SFC about joining. I'll keep the list up to date as things
> develop there.
> 
> I expect they'll need a liaison from the project. I can do that, but if
> somebody else is interested, let me know.

Now that I've read the links that you mentioned, I too think this is a
good idea.  I haven't been involved in SoC (though I benefited and much
appreciated having my travel expenses reimbursed), so my opinion should
be weighted accordingly.

If there's anything I can do to help, Jeff, please let me know.

-brandon

^ permalink raw reply

* Re: git-blame.el: what is format-spec?
From: Sergei Organov @ 2009-12-04 16:59 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: git
In-Reply-To: <m2fx7qae49.fsf@igel.home>

Andreas Schwab <schwab@linux-m68k.org> writes:
> Sergei Organov <osv@javad.com> writes:
>
>> What is format-spec function in current git-blame.el? Neither my GNU
>> Emacs 22.2.1 nor Google knows anything about it.
>
> It's part of Emacs since more than 9 years, imported from Gnus.
>

Thanks, I now see it in Gnus on my own computer, in
lisp/gnus/format-spec.el.gz.

GNU Emacs 22.2.1 (i486-pc-linux-gnu, GTK+ Version 2.12.11) of 2008-11-10
on raven, modified by Debian 

However, isn't it a bad idea to require Gnus(!) for git-blame to run? Gnus
is not installed on our server where I've encountered the problem. Was
format-spec actually moved to core emacs recently?

-- Sergei.

^ permalink raw reply

* [PATCH v8 0/7] fast-import: add new feature and option command
From: Sverre Rabbelier @ 2009-12-04 17:06 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	vcs-fast-import-devs

This reroll addressess comments from Shawn and the fast-import
mailing list. It also adds a new feature, "relative-marks", which
will be very useful for remote helpers.

Apologies for taking so long with the reroll, hopefully this version
is good for next and I can finish my git-remote-hg work :).

Sverre Rabbelier (7):
      fast-import: put option parsing code in separate functions
      fast-import: put marks reading in it's own function
      fast-import: add feature command
      fast-import: add option command
      fast-import: test the new option command
      fast-import: allow for multiple --import-marks= arguments
      fast-import: add (non-)relative-marks feature

 Documentation/git-fast-import.txt |   79 +++++++++
 fast-import.c                     |  317 +++++++++++++++++++++++++++----------
 t/t9300-fast-import.sh            |  152 ++++++++++++++++++
 3 files changed, 462 insertions(+), 86 deletions(-)

^ permalink raw reply

* [PATCH v8 2/7] fast-import: put marks reading in it's own function
From: Sverre Rabbelier @ 2009-12-04 17:06 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	vcs-fast-import-devs
  Cc: Sverre Rabbelier
In-Reply-To: <1259946420-8845-2-git-send-email-srabbelier@gmail.com>

All options do nothing but set settings, with the exception of the
--input-marks option. Delay the reading of the marks file till after
all options have been parsed.

Also, rename mark_file to export_marks_file as it is now ambiguous.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

	Renamed mark_fiel to export_marks_file and added import_marks_file as
	per Shawn's comments.

 fast-import.c |   93 +++++++++++++++++++++++++++++++-------------------------
 1 files changed, 51 insertions(+), 42 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index fcd9e1e..0458b03 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -318,7 +318,8 @@ static unsigned int object_entry_alloc = 5000;
 static struct object_entry_pool *blocks;
 static struct object_entry *object_table[1 << 16];
 static struct mark_set *marks;
-static const char *mark_file;
+static const char *export_marks_file;
+static const char *import_marks_file;
 
 /* Our last blob */
 static struct last_object last_blob = { STRBUF_INIT, 0, 0, 0 };
@@ -455,8 +456,8 @@ static void write_crash_report(const char *err)
 	fputc('\n', rpt);
 	fputs("Marks\n", rpt);
 	fputs("-----\n", rpt);
-	if (mark_file)
-		fprintf(rpt, "  exported to %s\n", mark_file);
+	if (export_marks_file)
+		fprintf(rpt, "  exported to %s\n", export_marks_file);
 	else
 		dump_marks_helper(rpt, 0, marks);
 
@@ -1603,13 +1604,13 @@ static void dump_marks(void)
 	int mark_fd;
 	FILE *f;
 
-	if (!mark_file)
+	if (!export_marks_file)
 		return;
 
-	mark_fd = hold_lock_file_for_update(&mark_lock, mark_file, 0);
+	mark_fd = hold_lock_file_for_update(&mark_lock, export_marks_file, 0);
 	if (mark_fd < 0) {
 		failure |= error("Unable to write marks file %s: %s",
-			mark_file, strerror(errno));
+			export_marks_file, strerror(errno));
 		return;
 	}
 
@@ -1618,7 +1619,7 @@ static void dump_marks(void)
 		int saved_errno = errno;
 		rollback_lock_file(&mark_lock);
 		failure |= error("Unable to write marks file %s: %s",
-			mark_file, strerror(saved_errno));
+			export_marks_file, strerror(saved_errno));
 		return;
 	}
 
@@ -1634,7 +1635,7 @@ static void dump_marks(void)
 		int saved_errno = errno;
 		rollback_lock_file(&mark_lock);
 		failure |= error("Unable to write marks file %s: %s",
-			mark_file, strerror(saved_errno));
+			export_marks_file, strerror(saved_errno));
 		return;
 	}
 
@@ -1642,11 +1643,47 @@ static void dump_marks(void)
 		int saved_errno = errno;
 		rollback_lock_file(&mark_lock);
 		failure |= error("Unable to commit marks file %s: %s",
-			mark_file, strerror(saved_errno));
+			export_marks_file, strerror(saved_errno));
 		return;
 	}
 }
 
+static void read_marks(void)
+{
+	char line[512];
+	FILE *f = fopen(import_marks_file, "r");
+	if (!f)
+		die_errno("cannot read '%s'", import_marks_file);
+	while (fgets(line, sizeof(line), f)) {
+		uintmax_t mark;
+		char *end;
+		unsigned char sha1[20];
+		struct object_entry *e;
+
+		end = strchr(line, '\n');
+		if (line[0] != ':' || !end)
+			die("corrupt mark line: %s", line);
+		*end = 0;
+		mark = strtoumax(line + 1, &end, 10);
+		if (!mark || end == line + 1
+			|| *end != ' ' || get_sha1(end + 1, sha1))
+			die("corrupt mark line: %s", line);
+		e = find_object(sha1);
+		if (!e) {
+			enum object_type type = sha1_object_info(sha1, NULL);
+			if (type < 0)
+				die("object not found: %s", sha1_to_hex(sha1));
+			e = insert_object(sha1);
+			e->type = type;
+			e->pack_id = MAX_PACK_ID;
+			e->offset = 1; /* just not zero! */
+		}
+		insert_mark(mark, e);
+	}
+	fclose(f);
+}
+
+
 static int read_next_command(void)
 {
 	static int stdin_eof = 0;
@@ -2421,39 +2458,9 @@ static void parse_progress(void)
 	skip_optional_lf();
 }
 
-static void option_import_marks(const char *input_file)
+static void option_import_marks(const char *marks)
 {
-	char line[512];
-	FILE *f = fopen(input_file, "r");
-	if (!f)
-		die_errno("cannot read '%s'", input_file);
-	while (fgets(line, sizeof(line), f)) {
-		uintmax_t mark;
-		char *end;
-		unsigned char sha1[20];
-		struct object_entry *e;
-
-		end = strchr(line, '\n');
-		if (line[0] != ':' || !end)
-			die("corrupt mark line: %s", line);
-		*end = 0;
-		mark = strtoumax(line + 1, &end, 10);
-		if (!mark || end == line + 1
-			|| *end != ' ' || get_sha1(end + 1, sha1))
-			die("corrupt mark line: %s", line);
-		e = find_object(sha1);
-		if (!e) {
-			enum object_type type = sha1_object_info(sha1, NULL);
-			if (type < 0)
-				die("object not found: %s", sha1_to_hex(sha1));
-			e = insert_object(sha1);
-			e->type = type;
-			e->pack_id = MAX_PACK_ID;
-			e->offset = 1; /* just not zero! */
-		}
-		insert_mark(mark, e);
-	}
-	fclose(f);
+	import_marks_file = xstrdup(marks);
 }
 
 static void option_date_format(const char *fmt)
@@ -2487,7 +2494,7 @@ static void option_active_branches(const char *branches)
 
 static void option_export_marks(const char *marks)
 {
-	mark_file = xstrdup(marks);
+	export_marks_file = xstrdup(marks);
 }
 
 static void option_export_pack_edges(const char *edges)
@@ -2581,6 +2588,8 @@ int main(int argc, const char **argv)
 	}
 	if (i != argc)
 		usage(fast_import_usage);
+	if (import_marks_file)
+		read_marks();
 
 	rc_free = pool_alloc(cmd_save * sizeof(*rc_free));
 	for (i = 0; i < (cmd_save - 1); i++)
-- 
1.6.5.3.164.g07b0c

^ permalink raw reply related

* [PATCH v8 1/7] fast-import: put option parsing code in separate functions
From: Sverre Rabbelier @ 2009-12-04 17:06 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	vcs-fast-import-devs
  Cc: Sverre Rabbelier
In-Reply-To: <1259946420-8845-1-git-send-email-srabbelier@gmail.com>

Putting the options in their own functions increases readability of
the option parsing block and makes it easier to reuse the option
parsing code later on.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

	Rebased against current master, no other changes.

 fast-import.c |  115 +++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 75 insertions(+), 40 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index dd3c99d..fcd9e1e 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -295,6 +295,7 @@ static unsigned long branch_count;
 static unsigned long branch_load_count;
 static int failure;
 static FILE *pack_edges;
+static unsigned int show_stats = 1;
 
 /* Memory pools */
 static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);
@@ -2420,7 +2421,7 @@ static void parse_progress(void)
 	skip_optional_lf();
 }
 
-static void import_marks(const char *input_file)
+static void option_import_marks(const char *input_file)
 {
 	char line[512];
 	FILE *f = fopen(input_file, "r");
@@ -2455,6 +2456,76 @@ static void import_marks(const char *input_file)
 	fclose(f);
 }
 
+static void option_date_format(const char *fmt)
+{
+	if (!strcmp(fmt, "raw"))
+		whenspec = WHENSPEC_RAW;
+	else if (!strcmp(fmt, "rfc2822"))
+		whenspec = WHENSPEC_RFC2822;
+	else if (!strcmp(fmt, "now"))
+		whenspec = WHENSPEC_NOW;
+	else
+		die("unknown --date-format argument %s", fmt);
+}
+
+static void option_max_pack_size(const char *packsize)
+{
+	max_packsize = strtoumax(packsize, NULL, 0) * 1024 * 1024;
+}
+
+static void option_depth(const char *depth)
+{
+	max_depth = strtoul(depth, NULL, 0);
+	if (max_depth > MAX_DEPTH)
+		die("--depth cannot exceed %u", MAX_DEPTH);
+}
+
+static void option_active_branches(const char *branches)
+{
+	max_active_branches = strtoul(branches, NULL, 0);
+}
+
+static void option_export_marks(const char *marks)
+{
+	mark_file = xstrdup(marks);
+}
+
+static void option_export_pack_edges(const char *edges)
+{
+	if (pack_edges)
+		fclose(pack_edges);
+	pack_edges = fopen(edges, "a");
+	if (!pack_edges)
+		die_errno("Cannot open '%s'", edges);
+}
+
+static void parse_one_option(const char *option)
+{
+	if (!prefixcmp(option, "date-format=")) {
+		option_date_format(option + 12);
+	} else if (!prefixcmp(option, "max-pack-size=")) {
+		option_max_pack_size(option + 14);
+	} else if (!prefixcmp(option, "depth=")) {
+		option_depth(option + 6);
+	} else if (!prefixcmp(option, "active-branches=")) {
+		option_active_branches(option + 16);
+	} else if (!prefixcmp(option, "import-marks=")) {
+		option_import_marks(option + 13);
+	} else if (!prefixcmp(option, "export-marks=")) {
+		option_export_marks(option + 13);
+	} else if (!prefixcmp(option, "export-pack-edges=")) {
+		option_export_pack_edges(option + 18);
+	} else if (!prefixcmp(option, "force")) {
+		force_update = 1;
+	} else if (!prefixcmp(option, "quiet")) {
+		show_stats = 0;
+	} else if (!prefixcmp(option, "stats")) {
+		show_stats = 1;
+	} else {
+		die("Unsupported option: %s", option);
+	}
+}
+
 static int git_pack_config(const char *k, const char *v, void *cb)
 {
 	if (!strcmp(k, "pack.depth")) {
@@ -2481,7 +2552,7 @@ static const char fast_import_usage[] =
 
 int main(int argc, const char **argv)
 {
-	unsigned int i, show_stats = 1;
+	unsigned int i;
 
 	git_extract_argv0_path(argv[0]);
 
@@ -2505,44 +2576,8 @@ int main(int argc, const char **argv)
 
 		if (*a != '-' || !strcmp(a, "--"))
 			break;
-		else if (!prefixcmp(a, "--date-format=")) {
-			const char *fmt = a + 14;
-			if (!strcmp(fmt, "raw"))
-				whenspec = WHENSPEC_RAW;
-			else if (!strcmp(fmt, "rfc2822"))
-				whenspec = WHENSPEC_RFC2822;
-			else if (!strcmp(fmt, "now"))
-				whenspec = WHENSPEC_NOW;
-			else
-				die("unknown --date-format argument %s", fmt);
-		}
-		else if (!prefixcmp(a, "--max-pack-size="))
-			max_packsize = strtoumax(a + 16, NULL, 0) * 1024 * 1024;
-		else if (!prefixcmp(a, "--depth=")) {
-			max_depth = strtoul(a + 8, NULL, 0);
-			if (max_depth > MAX_DEPTH)
-				die("--depth cannot exceed %u", MAX_DEPTH);
-		}
-		else if (!prefixcmp(a, "--active-branches="))
-			max_active_branches = strtoul(a + 18, NULL, 0);
-		else if (!prefixcmp(a, "--import-marks="))
-			import_marks(a + 15);
-		else if (!prefixcmp(a, "--export-marks="))
-			mark_file = a + 15;
-		else if (!prefixcmp(a, "--export-pack-edges=")) {
-			if (pack_edges)
-				fclose(pack_edges);
-			pack_edges = fopen(a + 20, "a");
-			if (!pack_edges)
-				die_errno("Cannot open '%s'", a + 20);
-		} else if (!strcmp(a, "--force"))
-			force_update = 1;
-		else if (!strcmp(a, "--quiet"))
-			show_stats = 0;
-		else if (!strcmp(a, "--stats"))
-			show_stats = 1;
-		else
-			die("unknown option %s", a);
+
+		parse_one_option(a + 2);
 	}
 	if (i != argc)
 		usage(fast_import_usage);
-- 
1.6.5.3.164.g07b0c

^ permalink raw reply related

* [PATCH v8 3/7] fast-import: add feature command
From: Sverre Rabbelier @ 2009-12-04 17:06 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	vcs-fast-import-devs
  Cc: Sverre Rabbelier
In-Reply-To: <1259946420-8845-3-git-send-email-srabbelier@gmail.com>

This allows the fronted to require a specific feature to be supported
by the frontend, or abort.

Also add support for four initial feature, date-format=, force=,
import-marks=, export-marks=.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

	Updated documentation, reject feature commands after a data command,
	as per Shawn's comments. Also factor out parse_one_feature from
	parse_feature so that we can re-use it in patch 4/7.

	This also has the previous 4/6 squashed in (which added tests)

 Documentation/git-fast-import.txt |   25 +++++++++++++
 fast-import.c                     |   38 ++++++++++++++++++++
 t/t9300-fast-import.sh            |   70 +++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 288032c..4357c21 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -303,6 +303,10 @@ and control the current import process.  More detailed discussion
 	standard output.  This command is optional and is not needed
 	to perform an import.
 
+`feature`::
+	Require that fast-import supports the specified feature, or
+	abort if it does not.
+
 `commit`
 ~~~~~~~~
 Create or update a branch with a new commit, recording one logical
@@ -846,6 +850,27 @@ Placing a `progress` command immediately after a `checkpoint` will
 inform the reader when the `checkpoint` has been completed and it
 can safely access the refs that fast-import updated.
 
+`feature`
+~~~~~~~~~
+Require that fast-import supports the specified feature, or abort if
+it does not.
+
+....
+	'feature' SP <feature> LF
+....
+
+The <feature> part of the command may be any string matching
+^[a-zA-Z][a-zA-Z-]*$ and should be understood by fast-import.
+
+Feature work identical as their option counterparts.
+
+The following features are currently supported:
+
+* date-format
+* import-marks
+* export-marks
+* force
+
 Crash Reports
 -------------
 If fast-import is supplied invalid input it will terminate with a
diff --git a/fast-import.c b/fast-import.c
index 0458b03..ce0cd4e 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -353,6 +353,7 @@ static struct recent_command *rc_free;
 static unsigned int cmd_save = 100;
 static uintmax_t next_mark;
 static struct strbuf new_data = STRBUF_INIT;
+static int seen_data_command;
 
 static void write_branch_report(FILE *rpt, struct branch *b)
 {
@@ -1704,6 +1705,11 @@ static int read_next_command(void)
 			if (stdin_eof)
 				return EOF;
 
+			if (!seen_data_command
+				&& prefixcmp(command_buf.buf, "feature ")) {
+				seen_data_command = 1;
+			}
+
 			rc = rc_free;
 			if (rc)
 				rc_free = rc->next;
@@ -2533,6 +2539,36 @@ static void parse_one_option(const char *option)
 	}
 }
 
+static int parse_one_feature(const char *feature)
+{
+	if (!prefixcmp(feature, "date-format=")) {
+		option_date_format(feature + 12);
+	} else if (!prefixcmp(feature, "import-marks=")) {
+		option_import_marks(feature + 13);
+	} else if (!prefixcmp(feature, "export-marks=")) {
+		option_export_marks(feature + 13);
+	} else if (!prefixcmp(feature, "force")) {
+		force_update = 1;
+	} else {
+		return 0;
+	}
+
+	return 1;
+}
+
+static void parse_feature(void)
+{
+	char *feature = command_buf.buf + 8;
+
+	if (seen_data_command)
+		die("Got feature command '%s' after data command", feature);
+
+	if (parse_one_feature(feature))
+		return;
+
+	die("This version of fast-import does not support feature %s.", feature);
+}
+
 static int git_pack_config(const char *k, const char *v, void *cb)
 {
 	if (!strcmp(k, "pack.depth")) {
@@ -2612,6 +2648,8 @@ int main(int argc, const char **argv)
 			parse_checkpoint();
 		else if (!prefixcmp(command_buf.buf, "progress "))
 			parse_progress();
+		else if (!prefixcmp(command_buf.buf, "feature "))
+			parse_feature();
 		else
 			die("Unsupported command: %s", command_buf.buf);
 	}
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index b49815d..b2c521f 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1254,4 +1254,74 @@ test_expect_success \
 	'Q: verify note for third commit' \
 	'git cat-file blob refs/notes/foobar:$commit3 >actual && test_cmp expect actual'
 
+###
+### series R (feature)
+###
+
+cat >input <<EOF
+feature no-such-feature-exists
+EOF
+
+test_expect_success 'R: abort on unsupported feature' '
+	test_must_fail git fast-import <input
+'
+
+cat >input <<EOF
+feature date-format=now
+EOF
+
+test_expect_success 'R: supported feature is accepted' '
+	git fast-import <input
+'
+
+cat >input << EOF
+blob
+data 3
+hi
+feature date-format=now
+EOF
+
+test_expect_success 'R: abort on receiving feature after data command' '
+	test_must_fail git fast-import <input
+'
+
+cat >input << EOF
+feature export-marks=git.marks
+blob
+mark :1
+data 3
+hi
+
+EOF
+
+test_expect_success \
+    'R: export-marks feature results in a marks file being created' \
+    'cat input | git fast-import &&
+    grep :1 git.marks'
+
+test_expect_success \
+    'R: export-marks options can be overriden by commandline options' \
+    'cat input | git fast-import --export-marks=other.marks &&
+    grep :1 other.marks'
+
+cat >input << EOF
+feature import-marks=marks.out
+feature export-marks=marks.new
+EOF
+
+test_expect_success \
+    'R: import to output marks works without any content' \
+    'cat input | git fast-import &&
+    test_cmp marks.out marks.new'
+
+cat >input <<EOF
+feature import-marks=nonexistant.marks
+feature export-marks=marks.new
+EOF
+
+test_expect_success \
+    'R: import marks prefers commandline marks file over the stream' \
+    'cat input | git fast-import --import-marks=marks.out &&
+    test_cmp marks.out marks.new'
+
 test_done
-- 
1.6.5.3.164.g07b0c

^ permalink raw reply related

* [PATCH v8 4/7] fast-import: add option command
From: Sverre Rabbelier @ 2009-12-04 17:06 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	vcs-fast-import-devs
  Cc: Sverre Rabbelier
In-Reply-To: <1259946420-8845-4-git-send-email-srabbelier@gmail.com>

This allows the frontend to specify any of the supported options as
long as no non-option command has been given. This way the
user does not have to include any frontend-specific options, but
instead she can rely on the frontend to tell fast-import what it
needs.

Also factor out parsing of argv and have it execute when we reach the
first non-option command, or after all commands have been read and
no non-option command has been encountered.

Non-git options are ignored, unrecognised options result in an error.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

	Reject unknown options, update documentation, and some style fixes.

 Documentation/git-fast-import.txt |   32 +++++++++++++
 fast-import.c                     |   87 ++++++++++++++++++++++++++-----------
 2 files changed, 94 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 4357c21..2d5f533 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -307,6 +307,11 @@ and control the current import process.  More detailed discussion
 	Require that fast-import supports the specified feature, or
 	abort if it does not.
 
+`option`::
+	Specify any of the options listed under OPTIONS that do not
+	change stream semantic to suit the frontend's needs. This
+	command is optional and is not needed to perform an import.
+
 `commit`
 ~~~~~~~~
 Create or update a branch with a new commit, recording one logical
@@ -871,6 +876,33 @@ The following features are currently supported:
 * export-marks
 * force
 
+`option`
+~~~~~~~~
+Processes the specified option so that git fast-import behaves in a
+way that suits the frontend's needs.
+Note that options specified by the frontend are overridden by any
+options the user may specify to git fast-import itself.
+
+....
+    'option' SP <option> LF
+....
+
+The `<option>` part of the command may contain any of the options
+listed in the OPTIONS section that do not change import semantics,
+without the leading '--' and is treated in the same way.
+
+Option commands must be the first commands on the input (not counting
+feature commands), to give an option command after any non-option
+command is an error.
+
+The following commandline options change import semantics and may therefore
+not be passed as option:
+
+* date-format
+* import-marks
+* export-marks
+* force
+
 Crash Reports
 -------------
 If fast-import is supplied invalid input it will terminate with a
diff --git a/fast-import.c b/fast-import.c
index ce0cd4e..ab099b6 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -296,6 +296,8 @@ static unsigned long branch_load_count;
 static int failure;
 static FILE *pack_edges;
 static unsigned int show_stats = 1;
+static int global_argc;
+static const char **global_argv;
 
 /* Memory pools */
 static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);
@@ -355,6 +357,8 @@ static uintmax_t next_mark;
 static struct strbuf new_data = STRBUF_INIT;
 static int seen_data_command;
 
+static void parse_argv(void);
+
 static void write_branch_report(FILE *rpt, struct branch *b)
 {
 	fprintf(rpt, "%s:\n", b->name);
@@ -1706,8 +1710,9 @@ static int read_next_command(void)
 				return EOF;
 
 			if (!seen_data_command
-				&& prefixcmp(command_buf.buf, "feature ")) {
-				seen_data_command = 1;
+				&& prefixcmp(command_buf.buf, "feature ")
+				&& prefixcmp(command_buf.buf, "option ")) {
+				parse_argv();
 			}
 
 			rc = rc_free;
@@ -2512,31 +2517,25 @@ static void option_export_pack_edges(const char *edges)
 		die_errno("Cannot open '%s'", edges);
 }
 
-static void parse_one_option(const char *option)
+static int parse_one_option(const char *option)
 {
-	if (!prefixcmp(option, "date-format=")) {
-		option_date_format(option + 12);
-	} else if (!prefixcmp(option, "max-pack-size=")) {
+	if (!prefixcmp(option, "max-pack-size=")) {
 		option_max_pack_size(option + 14);
 	} else if (!prefixcmp(option, "depth=")) {
 		option_depth(option + 6);
 	} else if (!prefixcmp(option, "active-branches=")) {
 		option_active_branches(option + 16);
-	} else if (!prefixcmp(option, "import-marks=")) {
-		option_import_marks(option + 13);
-	} else if (!prefixcmp(option, "export-marks=")) {
-		option_export_marks(option + 13);
 	} else if (!prefixcmp(option, "export-pack-edges=")) {
 		option_export_pack_edges(option + 18);
-	} else if (!prefixcmp(option, "force")) {
-		force_update = 1;
 	} else if (!prefixcmp(option, "quiet")) {
 		show_stats = 0;
 	} else if (!prefixcmp(option, "stats")) {
 		show_stats = 1;
 	} else {
-		die("Unsupported option: %s", option);
+		return 0;
 	}
+
+	return 1;
 }
 
 static int parse_one_feature(const char *feature)
@@ -2569,6 +2568,19 @@ static void parse_feature(void)
 	die("This version of fast-import does not support feature %s.", feature);
 }
 
+static void parse_option(void)
+{
+	char *option = command_buf.buf + 11;
+
+	if (seen_data_command)
+		die("Got option command '%s' after data command", option);
+
+	if (parse_one_option(option))
+		return;
+
+	die("This version of fast-import does not support option: %s", option);
+}
+
 static int git_pack_config(const char *k, const char *v, void *cb)
 {
 	if (!strcmp(k, "pack.depth")) {
@@ -2593,6 +2605,32 @@ static int git_pack_config(const char *k, const char *v, void *cb)
 static const char fast_import_usage[] =
 "git fast-import [--date-format=f] [--max-pack-size=n] [--depth=n] [--active-branches=n] [--export-marks=marks.file]";
 
+static void parse_argv(void)
+{
+	unsigned int i;
+
+	for (i = 1; i < global_argc; i++) {
+		const char *a = global_argv[i];
+
+		if (*a != '-' || !strcmp(a, "--"))
+			break;
+
+		if (parse_one_option(a + 2))
+			continue;
+
+		if (parse_one_feature(a + 2))
+			continue;
+
+		die("unknown option %s", a);
+	}
+	if (i != global_argc)
+		usage(fast_import_usage);
+
+	seen_data_command = 1;
+	if (import_marks_file)
+		read_marks();
+}
+
 int main(int argc, const char **argv)
 {
 	unsigned int i;
@@ -2614,18 +2652,8 @@ int main(int argc, const char **argv)
 	avail_tree_table = xcalloc(avail_tree_table_sz, sizeof(struct avail_tree_content*));
 	marks = pool_calloc(1, sizeof(struct mark_set));
 
-	for (i = 1; i < argc; i++) {
-		const char *a = argv[i];
-
-		if (*a != '-' || !strcmp(a, "--"))
-			break;
-
-		parse_one_option(a + 2);
-	}
-	if (i != argc)
-		usage(fast_import_usage);
-	if (import_marks_file)
-		read_marks();
+	global_argc = argc;
+	global_argv = argv;
 
 	rc_free = pool_alloc(cmd_save * sizeof(*rc_free));
 	for (i = 0; i < (cmd_save - 1); i++)
@@ -2650,9 +2678,18 @@ int main(int argc, const char **argv)
 			parse_progress();
 		else if (!prefixcmp(command_buf.buf, "feature "))
 			parse_feature();
+		else if (!prefixcmp(command_buf.buf, "option git "))
+			parse_option();
+		else if (!prefixcmp(command_buf.buf, "option "))
+			/* ignore non-git options*/;
 		else
 			die("Unsupported command: %s", command_buf.buf);
 	}
+
+	/* argv hasn't been parsed yet, do so */
+	if (!seen_data_command)
+		parse_argv();
+
 	end_packfile();
 
 	dump_branches();
-- 
1.6.5.3.164.g07b0c

^ permalink raw reply related

* [PATCH v8 5/7] fast-import: test the new option command
From: Sverre Rabbelier @ 2009-12-04 17:06 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	vcs-fast-import-devs
  Cc: Sverre Rabbelier
In-Reply-To: <1259946420-8845-5-git-send-email-srabbelier@gmail.com>

Test the quiet option and verify that the commandline options
override it.

Also make sure that an unknown option command is rejected and that
non-git options are ignored.

Lastly, show that unknown options are rejected when parsed on the
commandline.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

	Tests updated to match the new behavior.

 t/t9300-fast-import.sh |   37 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index b2c521f..74dff11 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1255,7 +1255,7 @@ test_expect_success \
 	'git cat-file blob refs/notes/foobar:$commit3 >actual && test_cmp expect actual'
 
 ###
-### series R (feature)
+### series R (feature and option)
 ###
 
 cat >input <<EOF
@@ -1324,4 +1324,39 @@ test_expect_success \
     'cat input | git fast-import --import-marks=marks.out &&
     test_cmp marks.out marks.new'
 
+cat >input << EOF
+option git quiet
+blob
+data 3
+hi
+
+EOF
+
+touch empty
+
+test_expect_success 'R: quiet option results in no stats being output' '
+    cat input | git fast-import 2> output &&
+    test_cmp empty output
+'
+
+cat >input <<EOF
+option git non-existing-option
+EOF
+
+test_expect_success 'R: die on unknown option' '
+    test_must_fail git fast-import <input
+'
+
+test_expect_success 'R: unknown commandline options are rejected' '\
+    test_must_fail git fast-import --non-existing-option < /dev/null
+'
+
+cat >input <<EOF
+option non-existing-vcs non-existing-option
+EOF
+
+test_expect_success 'R: ignore non-git options' '
+    git fast-import <input
+'
+
 test_done
-- 
1.6.5.3.164.g07b0c

^ permalink raw reply related

* [PATCH v8 6/7] fast-import: allow for multiple --import-marks= arguments
From: Sverre Rabbelier @ 2009-12-04 17:06 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	vcs-fast-import-devs
  Cc: Sverre Rabbelier
In-Reply-To: <1259946420-8845-6-git-send-email-srabbelier@gmail.com>

The --import-marks= option may be specified multiple times on the
commandline and should result in all marks being read in. Only one
import-marks feature may be specified in the stream, which is
overriden by any --import-marks= commandline options.

If one wishes to specify import-marks files in addition to the one
specified in the stream, it is easy to repeat the stream option as a
--import-marks= commandline option.

Also verify this behavior with tests.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

	New in this series. Without this it is no longer possible to specify
	multiple --import-marks= options on the commandline.

 Documentation/git-fast-import.txt |    8 +++++++-
 fast-import.c                     |   21 ++++++++++++++++-----
 t/t9300-fast-import.sh            |   22 ++++++++++++++++++++++
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 2d5f533..752f85c 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -867,7 +867,8 @@ it does not.
 The <feature> part of the command may be any string matching
 ^[a-zA-Z][a-zA-Z-]*$ and should be understood by fast-import.
 
-Feature work identical as their option counterparts.
+Feature work identical as their option counterparts with the
+exception of the import-marks feature, see below.
 
 The following features are currently supported:
 
@@ -876,6 +877,11 @@ The following features are currently supported:
 * export-marks
 * force
 
+The import-marks behaves differently from when it is specified as
+commandline option in that only one "feature import-marks" is allowed
+per stream. Also, any --import-marks= specified on the commandline
+will override those from the stream (if any).
+
 `option`
 ~~~~~~~~
 Processes the specified option so that git fast-import behaves in a
diff --git a/fast-import.c b/fast-import.c
index ab099b6..4c3406e 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -322,6 +322,7 @@ static struct object_entry *object_table[1 << 16];
 static struct mark_set *marks;
 static const char *export_marks_file;
 static const char *import_marks_file;
+static int import_marks_file_from_stream;
 
 /* Our last blob */
 static struct last_object last_blob = { STRBUF_INIT, 0, 0, 0 };
@@ -2469,9 +2470,19 @@ static void parse_progress(void)
 	skip_optional_lf();
 }
 
-static void option_import_marks(const char *marks)
+static void option_import_marks(const char *marks, int from_stream)
 {
+	if (import_marks_file) {
+		if (from_stream)
+			die("Only one import-marks command allowed per stream");
+
+		/* read previous mark file */
+		if(!import_marks_file_from_stream)
+			read_marks();
+	}
+
 	import_marks_file = xstrdup(marks);
+	import_marks_file_from_stream = from_stream;
 }
 
 static void option_date_format(const char *fmt)
@@ -2538,12 +2549,12 @@ static int parse_one_option(const char *option)
 	return 1;
 }
 
-static int parse_one_feature(const char *feature)
+static int parse_one_feature(const char *feature, int from_stream)
 {
 	if (!prefixcmp(feature, "date-format=")) {
 		option_date_format(feature + 12);
 	} else if (!prefixcmp(feature, "import-marks=")) {
-		option_import_marks(feature + 13);
+		option_import_marks(feature + 13, from_stream);
 	} else if (!prefixcmp(feature, "export-marks=")) {
 		option_export_marks(feature + 13);
 	} else if (!prefixcmp(feature, "force")) {
@@ -2562,7 +2573,7 @@ static void parse_feature(void)
 	if (seen_data_command)
 		die("Got feature command '%s' after data command", feature);
 
-	if (parse_one_feature(feature))
+	if (parse_one_feature(feature, 1))
 		return;
 
 	die("This version of fast-import does not support feature %s.", feature);
@@ -2618,7 +2629,7 @@ static void parse_argv(void)
 		if (parse_one_option(a + 2))
 			continue;
 
-		if (parse_one_feature(a + 2))
+		if (parse_one_feature(a + 2, 0))
 			continue;
 
 		die("unknown option %s", a);
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 74dff11..ba92775 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1286,6 +1286,15 @@ test_expect_success 'R: abort on receiving feature after data command' '
 '
 
 cat >input << EOF
+feature import-marks=git.marks
+feature import-marks=git2.marks
+EOF
+
+test_expect_success 'R: only one import-marks feature allowed per stream' '
+	test_must_fail git fast-import <input
+'
+
+cat >input << EOF
 feature export-marks=git.marks
 blob
 mark :1
@@ -1324,6 +1333,19 @@ test_expect_success \
     'cat input | git fast-import --import-marks=marks.out &&
     test_cmp marks.out marks.new'
 
+
+cat >input <<EOF
+feature import-marks=nonexistant.marks
+feature export-marks=combined.marks
+EOF
+
+test_expect_success 'R: multiple --import-marks= should be honoured' '
+    head -n2 marks.out > one.marks &&
+    tail -n +3 marks.out > two.marks &&
+    git fast-import --import-marks=one.marks --import-marks=two.marks <input &&
+    test_cmp marks.out combined.marks
+'
+
 cat >input << EOF
 option git quiet
 blob
-- 
1.6.5.3.164.g07b0c

^ permalink raw reply related

* [PATCH v8 7/7] fast-import: add (non-)relative-marks feature
From: Sverre Rabbelier @ 2009-12-04 17:07 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	vcs-fast-import-devs
  Cc: Sverre Rabbelier, Daniel Barkalow
In-Reply-To: <1259946420-8845-7-git-send-email-srabbelier@gmail.com>

After specifying 'feature relative-marks' the paths specified with
'feature import-marks' and 'feature export-marks' are relative to an
internal directory in the current repository.

In git-fast-import this means that the paths are relative to the
'.git/info/fast-import' directory. However, other importers may use a
different location.

Add 'feature non-relative-marks' to disable this behavior, this way
it is possible to, for example, specify the import-marks location as
relative, and the export-marks location as non-relative.

Also add tests to verify this behavior.

Cc: Daniel Barkalow <barkalow@iabervon.org>
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

	As requested by Daniel, it is now possible to have the marks be
	relative to a constant directory. We might want to consider making
	this the default at some point.

	This patch opens the way for remote-helpers to use the marks feature
	without poluting the work tree, which I think is very important.

 Documentation/git-fast-import.txt |   16 ++++++++++++++++
 fast-import.c                     |   19 +++++++++++++++++--
 t/t9300-fast-import.sh            |   25 +++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 752f85c..1a63835 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -75,6 +75,20 @@ OPTIONS
 	set of marks.  If a mark is defined to different values,
 	the last file wins.
 
+--relative-marks::
+	After specifying --relative-marks= the paths specified
+	with --import-marks= and --export-marks= are relative
+	to an internal directory in the current repository.
+	In git-fast-import this means that the paths are relative
+	to the .git/info/fast-import directory. However, other
+	importers may use a different location.
+
+--no-relative-marks::
+	Negates a previous --relative-marks. Allows for combining
+	relative and non-relative marks by interweaving
+	--(no-)-relative-marks= with the --(import|export)-marks=
+	options.
+
 --export-pack-edges=<file>::
 	After creating a packfile, print a line of data to
 	<file> listing the filename of the packfile and the last
@@ -875,6 +889,8 @@ The following features are currently supported:
 * date-format
 * import-marks
 * export-marks
+* relative-marks
+* no-relative-marks
 * force
 
 The import-marks behaves differently from when it is specified as
diff --git a/fast-import.c b/fast-import.c
index 4c3406e..8d50a1e 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -323,6 +323,7 @@ static struct mark_set *marks;
 static const char *export_marks_file;
 static const char *import_marks_file;
 static int import_marks_file_from_stream;
+static int relative_marks_paths;
 
 /* Our last blob */
 static struct last_object last_blob = { STRBUF_INIT, 0, 0, 0 };
@@ -2470,6 +2471,16 @@ static void parse_progress(void)
 	skip_optional_lf();
 }
 
+static char* make_fast_import_path(const char *path)
+{
+	if (!relative_marks_paths || is_absolute_path(path))
+		return xstrdup(path);
+
+	struct strbuf abs_path = STRBUF_INIT;
+	strbuf_addf(&abs_path, "%s/info/fast-import/%s", get_git_dir(), path);
+	return strbuf_detach(&abs_path, NULL);
+}
+
 static void option_import_marks(const char *marks, int from_stream)
 {
 	if (import_marks_file) {
@@ -2481,7 +2492,7 @@ static void option_import_marks(const char *marks, int from_stream)
 			read_marks();
 	}
 
-	import_marks_file = xstrdup(marks);
+	import_marks_file = make_fast_import_path(marks);
 	import_marks_file_from_stream = from_stream;
 }
 
@@ -2516,7 +2527,7 @@ static void option_active_branches(const char *branches)
 
 static void option_export_marks(const char *marks)
 {
-	export_marks_file = xstrdup(marks);
+	export_marks_file = make_fast_import_path(marks);
 }
 
 static void option_export_pack_edges(const char *edges)
@@ -2557,6 +2568,10 @@ static int parse_one_feature(const char *feature, int from_stream)
 		option_import_marks(feature + 13, from_stream);
 	} else if (!prefixcmp(feature, "export-marks=")) {
 		option_export_marks(feature + 13);
+	} else if (!prefixcmp(feature, "relative-marks")) {
+		relative_marks_paths = 1;
+	} else if (!prefixcmp(feature, "no-relative-marks")) {
+		relative_marks_paths = 0;
 	} else if (!prefixcmp(feature, "force")) {
 		force_update = 1;
 	} else {
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index ba92775..a1b8c2b 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1346,6 +1346,31 @@ test_expect_success 'R: multiple --import-marks= should be honoured' '
     test_cmp marks.out combined.marks
 '
 
+cat >input <<EOF
+feature relative-marks
+feature import-marks=relative.in
+feature export-marks=relative.out
+EOF
+
+test_expect_success 'R: feature relative-marks should be honoured' '
+    mkdir -p .git/info/fast-import/ &&
+    cp marks.new .git/info/fast-import/relative.in &&
+    git fast-import <input &&
+    test_cmp marks.new .git/info/fast-import/relative.out
+'
+
+cat >input <<EOF
+feature relative-marks
+feature import-marks=relative.in
+feature no-relative-marks
+feature export-marks=non-relative.out
+EOF
+
+test_expect_success 'R: feature no-relative-marks should be honoured' '
+    git fast-import <input &&
+    test_cmp marks.new non-relative.out
+'
+
 cat >input << EOF
 option git quiet
 blob
-- 
1.6.5.3.164.g07b0c

^ permalink raw reply related

* Re: [ANNOUNCE] Git 1.6.5.4
From: Junio C Hamano @ 2009-12-04 17:10 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Todd Zullinger, Michael J Gruber, git
In-Reply-To: <m2r5rb9hes.fsf@igel.home>

Andreas Schwab <schwab@linux-m68k.org> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> I think it depends on the likelihood that a distro has xmlto so old that
>> it does not understand --stringparam yet it uses stylesheet so new that
>> setting the parameter makes a positive difference (either it gives the
>> full URL or at least squelches the "You should define the parameter"
>> noise) in the output.
>
> openSUSE 11.2, for example.  Its xmlto has a non-standard --xsltopts
> option that passes its argument down to xsltproc.

Ok, as I said that I've been wrong before in this area ;-)

I don't think I will have much time for git today, and it would be
appreciated if somebody can work on this and send a tested patch that
applies cleanly on top of 'maint' to implement the @@MAN_BASE_URL@@
replacement from manpage-base.xsl.in to manpage-base.xsl as Todd suggested
earlier.

^ permalink raw reply

* Re: [BUG] crash on make test
From: Junio C Hamano @ 2009-12-04 17:12 UTC (permalink / raw)
  To: Jeff King; +Cc: Marinescu Paul dan, Junio C Hamano, git@vger.kernel.org
In-Reply-To: <20091204103557.GC27495@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I think we should apply the patch below to maint, as this is something
> that can come up due to permissions problems. But I fear it won't
> actually fix the test failure you are seeing; you will just see it die()
> instead of segfaulting. However, the value of errno should give us a
> clue about what is happening, so please try running the test again with
> this patch.

Thanks, and I agree with the whole of the above paragraph, not just the
first sentence.

^ permalink raw reply

* Re: Running commands in wrong environment
From: Junio C Hamano @ 2009-12-04 17:16 UTC (permalink / raw)
  To: Jeff King; +Cc: Marinescu Paul dan, Junio C Hamano, git@vger.kernel.org
In-Reply-To: <20091204104441.GD27495@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> But the consequences, as you mention, could include data loss, which
> argues for being on the safe side. In that case, we would probably want
> an "xsetenv" to die() if we fail to avoid cluttering the code
> everywhere.
>
> I dunno. If we're going to do it, it is probably maint-worthy. Junio?

My gut feeling is xsetenv/xputenv would be sufficient.  I do not think we
make any setenv/putenv that we do not care about failing, perhaps other
than the "if LESS is not there set it to this default value, as it would
give users nicer experience if they use 'less'."

^ permalink raw reply

* Re: git reset --hard in .git causes a checkout in that directory
From: Junio C Hamano @ 2009-12-04 17:25 UTC (permalink / raw)
  To: Jeff King; +Cc: Maarten Lankhorst, Junio C Hamano, git
In-Reply-To: <20091204111158.GE27495@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Junio, I think the following should go to maint (I didn't bother
> splitting the --merge and --hard code; --merge is in v1.6.2. I assumed
> we don't care about maint releases that far back).

Thanks.  The test already checks that the change won't break soft reset,
which is good, but it does not seem to check/specify what should happen in
the mixed reset in this case (I think it should be allowed).  Could you
add one while at it?

> diff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh
> index 42bf518..3ddf0ac 100755
> --- a/t/t7103-reset-bare.sh
> +++ b/t/t7103-reset-bare.sh
> @@ -11,6 +11,16 @@ test_expect_success 'setup non-bare' '
>  	git commit -a -m two
>  '
>  
> +test_expect_success 'hard reset requires a worktree' '
> +	(cd .git &&
> +	 test_must_fail git reset --hard)
> +'
> +
> +test_expect_success 'merge reset requires a worktree' '
> +	(cd .git &&
> +	 test_must_fail git reset --merge)
> +'
> +
>  test_expect_success 'setup bare' '
>  	git clone --bare . bare.git &&
>  	cd bare.git
> @@ -20,6 +30,10 @@ test_expect_success 'hard reset is not allowed' '
>  	test_must_fail  git reset --hard HEAD^
>  '
>  
> +test_expect_success 'merge reset is not allowed' '
> +	test_must_fail git reset --merge HEAD^
> +'
> +
>  test_expect_success 'soft reset is allowed' '
>  	git reset --soft HEAD^ &&
>  	test "`git show --pretty=format:%s | head -n 1`" = "one"
> -- 
> 1.6.6.rc1.18.ga777f.dirty

^ permalink raw reply

* Re: rev-parse --show-cdup in .git
From: Junio C Hamano @ 2009-12-04 17:32 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List
In-Reply-To: <4B192EAE.8000806@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> I'm sure this was discussed somewhere, but I can't find it:
>
> When called from within .git, git rev-parse --show-cdup returns nothing
> rather than "..", not even an error code. Is this intended?

I do not think --show-cdup should be linked to --is-inside-work-tree in
any way, if that is what you are getting at by mentioning "error code".

> This is all the more disturbing since the cwd of hooks seems to be
> GIT_DIR. Is that something one can rely upon? In that case one can
> simply use ".." for cdup.

I think you will see something that is different from and is more sensible
than ".." when you use GIT_WORK_TREE environment variable (or its
corresponding configuration variable).

^ permalink raw reply

* Re: [PATCH 3/3] Add a command "fix" to rebase --interactive.
From: Junio C Hamano @ 2009-12-04 17:40 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git, Johannes.Schindelin
In-Reply-To: <6d779d2c244bf5d5b7924cdc5daf66a8186e4bc7.1259934977.git.mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0bd3bf7..539413d 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -302,7 +302,7 @@ nth_string () {
>  
>  make_squash_message () {
>  	if test -f "$SQUASH_MSG"; then
> -		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
> +		COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)\(th\|st\|nd\|rd\) commit message.*:/\1/p" \
>  			< "$SQUASH_MSG" | sed -ne '$p')+1))

This sed replacement worries me.  I don't have a time to check myself
today but do we use \(this\|or\|that\) alternates with our sed script
already elsewhere in the codebase (test scripts do not count)?

Otherwise this may suddenly be breaking a platform that has an
implementation of sed that may be substandard but so far has been
sufficient to work with git.

^ permalink raw reply

* Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"
From: Matthieu Moy @ 2009-12-04 17:40 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Michael Haggerty, git, gitster, Johannes.Schindelin
In-Reply-To: <4B192701.4000308@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

>> If the idea of a "fix" command is acceptable, then I would like to
>> implement a further convenience: if a group of commits to be folded
>> together includes *only* "fix" commits, then the first log message
>> should be used without even opening an editor.  But I would like to
>> get a reaction to the "fix" command in general before doing so.
>
> I'd say that would make a useful command ("fix") even more useful, being
> just the right counterpart to "reword" for trivial commit message fixes.

+1 for fix, and +1 for the "don't even launch the editor" too.

> OTOH, it would not be possible any more to squash in a few fixes and
> then edit the message. Maybe having to quit the editor is not that much
> work after all.

Well, it's still possible, by using "squash" and deleting the extra
part. In both cases, there's one scenario where one has a little more
to do than strictly required (either delete a bit of texte or close
the editor), but I think the senario "just want to fix the content
with another patch, but the commit message was good" is so common that
it deserves being optimized.

In particular, that would make a rebase -i with only reordering and
fixes non-interactive. With 3 fixes, you just wait for Git around a
second and it's done, smoothly, while it requires launching/closing
the editor 3 times ...

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: git-blame.el: what is format-spec?
From: Andreas Schwab @ 2009-12-04 17:42 UTC (permalink / raw)
  To: Sergei Organov; +Cc: git
In-Reply-To: <87ljhi3cao.fsf@osv.gnss.ru>

Sergei Organov <osv@javad.com> writes:

> However, isn't it a bad idea to require Gnus(!) for git-blame to run? Gnus
> is not installed on our server where I've encountered the problem. 

Gnus has been part of Emacs since more than 10 years.

> Was format-spec actually moved to core emacs recently?

Yes, in 23.1.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"
From: Junio C Hamano @ 2009-12-04 17:44 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Michael J Gruber, Michael Haggerty, git, gitster,
	Johannes.Schindelin
In-Reply-To: <vpqfx7qocwl.fsf@bauges.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>>> If the idea of a "fix" command is acceptable, then I would like to
>>> implement a further convenience: if a group of commits to be folded
>>> together includes *only* "fix" commits, then the first log message
>>> should be used without even opening an editor.  But I would like to
>>> get a reaction to the "fix" command in general before doing so.
>>
>> I'd say that would make a useful command ("fix") even more useful, being
>> just the right counterpart to "reword" for trivial commit message fixes.
>
> +1 for fix, and +1 for the "don't even launch the editor" too.

I like it, too.  Also I vaguely recall that there was a series that died
that would have allowed you to give hints to help this behaviour at the
time you make "fix-up" commits; we may want to resurrect it on top of this
feature.

^ permalink raw reply

* Re: [PATCH 3/3] Add a command "fix" to rebase --interactive.
From: Matthieu Moy @ 2009-12-04 17:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael Haggerty, git, Johannes.Schindelin
In-Reply-To: <7v638mskmx.fsf@alter.siamese.dyndns.org>

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

> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index 0bd3bf7..539413d 100755
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -302,7 +302,7 @@ nth_string () {
>>  
>>  make_squash_message () {
>>  	if test -f "$SQUASH_MSG"; then
>> -		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
>> +		COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)\(th\|st\|nd\|rd\) commit message.*:/\1/p" \
>>  			< "$SQUASH_MSG" | sed -ne '$p')+1))
>
> This sed replacement worries me.  I don't have a time to check myself
> today but do we use \(this\|or\|that\) alternates with our sed script
> already elsewhere in the codebase (test scripts do not count)?

It seems we don't:

git$ git grep '\\|' *.sh
git-rebase--interactive.sh:             COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)\(th\|st\|nd\|rd\) commit message.*:/\
git$ 

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [RFC PATCH v2 1/8] Pass unknown protocols to external protocol handlers
From: Shawn O. Pearce @ 2009-12-04 17:55 UTC (permalink / raw)
  To: Ilari Liusvaara; +Cc: git
In-Reply-To: <1259942168-24869-4-git-send-email-ilari.liusvaara@elisanet.fi>

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> Change URL handling to allow external protocol handlers to implement
> new protocols without the '::' syntax if helper name does not conflict
> with any built-in protocol.
> 
> foo:// now invokes git-remote-foo with foo:// URL.
...

Uh, great, but...

> @@ -30,6 +50,7 @@ static struct child_process *get_helper(struct transport *transport)
>  	const char **refspecs = NULL;
>  	int refspec_nr = 0;
>  	int refspec_alloc = 0;
> +	int duped;
>  
>  	if (data->helper)
>  		return data->helper;
...
> +	/* Open the output as FILE* so strbuf_getline() can be used.
> +	   Do this with duped fd because fclose() will close the fd,
> +	   and stuff like disowning will require the fd to remain.
> +
> +	   Set the stream to unbuffered because some reads are critical
> +	   in sense that any overreading will cause deadlocks.
> +	*/
> +	if((duped = dup(helper->out)) < 0)
> +		die_errno("Can't dup helper output fd");
> +	data->out = xfdopen(duped, "r");
> +	setvbuf(data->out, NULL, _IONBF, 0);
> +

This is an entirely unrelated change.  Please split it into its
own commit so its easier to review, test, blah blah blah.

-- 
Shawn.

^ permalink raw reply

* Endianness bug in git-svn or called component?
From: Nathaniel W Filardo @ 2009-12-04 17:44 UTC (permalink / raw)
  To: git

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

On this machine,

mirrors hydra:/tank0/mirrors/misc% uname -a
FreeBSD hydra.priv.oc.ietfng.org 9.0-CURRENT FreeBSD 9.0-CURRENT #13: Sat Nov 14 19:40:25 EST 2009 root@hydra.priv.oc.ietfng.org:/systank/obj/systank/src/sys/NWFKERN  sparc64

attempting to fetch from an svn source yields the following error:

rs hydra:/tank0/mirrors/misc% git svn init -s https://joshua.svn.sourceforge.net/svnroot/joshua test-joshua
Initialized empty Git repository in /tank0/mirrors/misc/test-joshua/.git/                                       
mirrors hydra:/tank0/mirrors/misc% cd test-joshua                                                               
mirrors hydra:/tank0/mirrors/misc/test-joshua% git svn fetch
        A       scripts/distributedLM/config.template       
[...]
        A       build.xml
r1 = fe84a7d821ec6d92da75133ac7ad1deb476b6484 (refs/remotes/trunk)
error: index uses  extension, which we do not understand
fatal: index file corrupt
write-tree: command returned error: 128

Even on previously initialized and fetched git-svn repos, "git svn fetch"
produces the same response.

This is the same symptom as an older bug, but I have not attempted to track
down what's going wrong this time in hopes that somebody more familiar can
fix it faster; see
http://kerneltrap.org/mailarchive/git/2006/5/28/205750/thread#mid-205750 .

If there's anything more that would be helpful to know, please just ask.

Thanks.
--nwf;

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

^ permalink raw reply

* Re: [PATCH v8 7/7] fast-import: add (non-)relative-marks feature
From: Daniel Barkalow @ 2009-12-04 18:09 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	vcs-fast-import-devs
In-Reply-To: <1259946420-8845-8-git-send-email-srabbelier@gmail.com>

On Fri, 4 Dec 2009, Sverre Rabbelier wrote:

> After specifying 'feature relative-marks' the paths specified with
> 'feature import-marks' and 'feature export-marks' are relative to an
> internal directory in the current repository.
> 
> In git-fast-import this means that the paths are relative to the
> '.git/info/fast-import' directory. However, other importers may use a
> different location.
> 
> Add 'feature non-relative-marks' to disable this behavior, this way
> it is possible to, for example, specify the import-marks location as
> relative, and the export-marks location as non-relative.
> 
> Also add tests to verify this behavior.
> 
> Cc: Daniel Barkalow <barkalow@iabervon.org>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
> 
> 	As requested by Daniel, it is now possible to have the marks be
> 	relative to a constant directory. We might want to consider making
> 	this the default at some point.
> 
> 	This patch opens the way for remote-helpers to use the marks feature
> 	without poluting the work tree, which I think is very important.

I think it would be better to make relative paths be the only available 
method, in part because we don't want to polute the work tree, but more 
because otherwise scripts aren't transferrable. That is, if you have an 
absolute path, you can't send the same script to two different importers 
(because they'd have to write their marks to the same location). And 
there's no way for the program constructing a script to determine a good 
absolute location if the script may be run on a different host than it's 
generated on (think of getting a Linux user getting a script attached to a 
bug report from a native Windows user; there are no absolute paths that 
are valid on both of these).

(However, it probably does make sense to permit absolute paths in the 
command line, since whoever's writing the command line is presumably aware 
of any local conventions)

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: git-blame.el: what is format-spec?
From: David Kågedal @ 2009-12-04 17:36 UTC (permalink / raw)
  To: Sergei Organov; +Cc: git, Andreas Schwab
In-Reply-To: <87ljhi3cao.fsf@osv.gnss.ru>

Sergei Organov <osv@javad.com> writes:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>> Sergei Organov <osv@javad.com> writes:
>>
>>> What is format-spec function in current git-blame.el? Neither my GNU
>>> Emacs 22.2.1 nor Google knows anything about it.
>>
>> It's part of Emacs since more than 9 years, imported from Gnus.
>>
>
> Thanks, I now see it in Gnus on my own computer, in
> lisp/gnus/format-spec.el.gz.
>
> GNU Emacs 22.2.1 (i486-pc-linux-gnu, GTK+ Version 2.12.11) of 2008-11-10
> on raven, modified by Debian 
>
> However, isn't it a bad idea to require Gnus(!) for git-blame to run? Gnus
> is not installed on our server where I've encountered the problem. Was
> format-spec actually moved to core emacs recently?

That was not my intention when I posted the patch. I seem to recall that
I asked for testing, in particular from users with older Emacsen than
23. But I got no response, and only recently discovered that the patch
hade been accepted.

format-spec is included in Emacs 23, and is a useful function. But we
can rewrite git-blame.el to do the formatting manuall instead.

-- 
David Kågedal

^ 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