Git development
 help / color / mirror / Atom feed
* Re: Python extension commands in git - request for policy change
From: Eric S. Raymond @ 2012-11-25 22:47 UTC (permalink / raw)
  To: Krzysztof Mazur; +Cc: git
In-Reply-To: <20121125214139.GA29465@shrek.podlesie.net>

Krzysztof Mazur <krzysiek@podlesie.net>:
> What about embedded systems? git is also useful there. C and shell is
> everywhere, python is not.

Supposing this is true (and I question it with regard to shell) if you
tell me how you live without gitk and the Perl pieces I'll play that
right back at you as your answer.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* [PATCH 00/11] alternative unify appending of sob
From: Brandon Casey @ 2012-11-26  1:45 UTC (permalink / raw)
  To: git; +Cc: pclouds, gitster, Brandon Casey

From: Brandon Casey <bcasey@nvidia.com>

I hate to have the battle of the patches, but I kinda prefer the
append_signoff code in sequencer.c over the code in log-tree.c as a
foundation to build on.

So, this series is similar to Duy's "unification" series, but it goes in the
opposite direction and builds on top of sequencer.c and additionally adds the
elements from my original series to treat the "(cherry picked from..." line
added by 'cherry-pick -x' in the same way that other footer elements are
treated (after addressing Junio's comments about rfc2822 continuation lines
and duplicate s-o-b's).

I've integrated Duy's series with a few minor tweaks.  I added a couple
of additional tests to t4014 and corrected one of the tests which had
incorrect behavior.  I think his sign-off's should still be valid, so I
kept them in.  Sorry that I've been slow, and now the two of us are stepping
on each other's toes, but Duy please take a look and let me know if there's
anything you dislike.

The main difference between this series and Duy's, aside from the change in
behavior with respect to a "(cherry picked from...)" line, is that the
detection of a s-o-b footer is done in a somewhat more generic way.  The
log-tree.c code expects to find something that looks like "[-A-Za-z]+:" on
the left-hand side, and something that looks like an email address on the
right-hand side.  The sequencer.c code relaxes this definition so that an
email address is not required on the right hand side.  This allows lines like
"Change-id: IXXXX" or "Bug: XXX" to be considered valid footer elements.

So, this series should result in s-o-b and "(cherry picked from...)" being
appended to commit messages in a more consistent and deterministic way.  For
example, the decision about whether to insert a blank line before appending
a s-o-b.  As discussed, cherry-pick and commit will only refrain from
appending a s-o-b if the committer's s-o-b appears as the last one in a
conforming footer, while format-patch will refrain from appending it if it
appears anywhere within the footer.

-Brandon

Brandon Casey (8):
  sequencer.c: remove broken support for rfc2822 continuation in footer
  t/test-lib-functions.sh: allow to specify the tag name to test_commit
  t/t3511: add some tests of 'cherry-pick -s' functionality
  sequencer.c: recognize "(cherry picked from ..." as part of s-o-b
    footer
  sequencer.c: always separate "(cherry picked from" from commit body
  sequencer.c: teach append_signoff how to detect duplicate s-o-b
  sequencer.c: teach append_signoff to avoid adding a duplicate newline
  Unify appending signoff in format-patch, commit and sequencer

Nguyễn Thái Ngọc Duy (3):
  t4014: more tests about appending s-o-b lines
  format-patch: stricter S-o-b detection
  format-patch: update append_signoff prototype

 builtin/commit.c         |   2 +-
 builtin/log.c            |  13 +--
 log-tree.c               |  92 ++---------------
 revision.h               |   2 +-
 sequencer.c              | 133 +++++++++++++++---------
 sequencer.h              |   2 +-
 t/t3511-cherry-pick-x.sh | 219 +++++++++++++++++++++++++++++++++++++++
 t/t4014-format-patch.sh  | 262 +++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib-functions.sh  |   9 +-
 9 files changed, 580 insertions(+), 154 deletions(-)
 create mode 100755 t/t3511-cherry-pick-x.sh

-- 
1.8.0.284.g959048a

^ permalink raw reply

* [PATCH 01/11] sequencer.c: remove broken support for rfc2822 continuation in footer
From: Brandon Casey @ 2012-11-26  1:45 UTC (permalink / raw)
  To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>

Commit c1e01b0c generalized the detection of the last paragraph
signed-off-by footer and used rfc2822 as a guideline.  Support for rfc2822
style continuation lines was also implemented, but not correctly, so it has
never detected a line beginning with space or tab as a continuation of the
previous line.

Since a commit message is not governed by the line length limits imposed
by rfc2822 for email messages, and it does not seem like this functionality
would produce "better" commit messages anyway, let's remove this broken
functionality.

Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 sequencer.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 2260490..656df6b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1023,7 +1023,6 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
 	int hit = 0;
 	int i, j, k;
 	int len = sb->len - ignore_footer;
-	int first = 1;
 	const char *buf = sb->buf;
 
 	for (i = len - 1; i > 0; i--) {
@@ -1040,11 +1039,6 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
 			; /* do nothing */
 		k++;
 
-		if ((buf[k] == ' ' || buf[k] == '\t') && !first)
-			continue;
-
-		first = 0;
-
 		for (j = 0; i + j < len; j++) {
 			ch = buf[i + j];
 			if (ch == ':')
-- 
1.8.0.284.g959048a

^ permalink raw reply related

* [PATCH 02/11] t/test-lib-functions.sh: allow to specify the tag name to test_commit
From: Brandon Casey @ 2012-11-26  1:45 UTC (permalink / raw)
  To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>

The <message> part of test_commit() may not be appropriate for a tag name.
So let's allow test_commit to accept a fourth argument to specify the tag
name.

Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 t/test-lib-functions.sh | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8889ba5..9e2b8b8 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -135,12 +135,13 @@ test_pause () {
 	fi
 }
 
-# Call test_commit with the arguments "<message> [<file> [<contents>]]"
+# Call test_commit with the arguments "<message> [<file> [<contents> [<tag>]]]"
 #
 # This will commit a file with the given contents and the given commit
-# message.  It will also add a tag with <message> as name.
+# message.  It will also add a tag with <message> as name unless <tag> is
+# given.
 #
-# Both <file> and <contents> default to <message>.
+# <file>, <contents>, and <tag> all default to <message>.
 
 test_commit () {
 	notick= &&
@@ -168,7 +169,7 @@ test_commit () {
 		test_tick
 	fi &&
 	git commit $signoff -m "$1" &&
-	git tag "$1"
+	git tag "${4:-$1}"
 }
 
 # Call test_merge with the arguments "<message> <commit>", where <commit>
-- 
1.8.0.284.g959048a

^ permalink raw reply related

* [PATCH 03/11] t/t3511: add some tests of 'cherry-pick -s' functionality
From: Brandon Casey @ 2012-11-26  1:45 UTC (permalink / raw)
  To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>

Add some tests to ensure that 'cherry-pick -s' operates in the following
manner:

   * Inserts a blank line before appending a s-o-b to a commit message that
     does not contain a s-o-b footer

   * Does not mistake first line "subject: description" as a s-o-b footer

   * Does not mistake single word message body as conforming to rfc2822

   * Appends a s-o-b when last s-o-b in footer does not match committer
     s-o-b, even when committer's s-o-b exists elsewhere in footer.

   * Does not append a s-o-b when last s-o-b matches committer s-o-b

   * Correctly detects a non-conforming footer containing a mix of s-o-b
     like elements and s-o-b elements. (marked "expect failure")

Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 t/t3511-cherry-pick-x.sh | 111 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)
 create mode 100755 t/t3511-cherry-pick-x.sh

diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
new file mode 100755
index 0000000..a6c4168
--- /dev/null
+++ b/t/t3511-cherry-pick-x.sh
@@ -0,0 +1,111 @@
+#!/bin/sh
+
+test_description='Test cherry-pick -x and -s'
+
+. ./test-lib.sh
+
+pristine_detach () {
+	git cherry-pick --quit &&
+	git checkout -f "$1^0" &&
+	git read-tree -u --reset HEAD &&
+	git clean -d -f -f -q -x
+}
+
+mesg_one_line='base: commit message'
+
+mesg_no_footer="$mesg_one_line
+
+OneWordBodyThatsNotA-S-o-B"
+
+mesg_with_footer="$mesg_no_footer
+
+Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+Signed-off-by: A.U. Thor <author@example.com>
+Signed-off-by: B.U. Thor <buthor@example.com>"
+
+mesg_broken_footer="$mesg_no_footer
+
+Signed-off-by: B.U. Thor <buthor@example.com>
+Not a valid footer element
+Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+
+mesg_with_footer_sob="$mesg_with_footer
+Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+
+
+test_expect_success setup '
+	git config advice.detachedhead false &&
+	echo unrelated >unrelated &&
+	git add unrelated &&
+	test_commit initial foo a &&
+	test_commit "$mesg_one_line" foo b mesg-one-line &&
+	git reset --hard initial &&
+	test_commit "$mesg_no_footer" foo b mesg-no-footer &&
+	git reset --hard initial &&
+	test_commit "$mesg_broken_footer" foo b mesg-broken-footer &&
+	git reset --hard initial &&
+	test_commit "$mesg_with_footer" foo b mesg-with-footer &&
+	git reset --hard initial &&
+	test_commit "$mesg_with_footer_sob" foo b mesg-with-footer-sob &&
+	pristine_detach initial &&
+	test_commit conflicting unrelated
+'
+
+test_expect_success 'cherry-pick -s inserts blank line after one line subject' '
+	pristine_detach initial &&
+	git cherry-pick -s mesg-one-line &&
+	cat <<-EOF >expect &&
+		$mesg_one_line
+
+		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
+test_expect_failure 'cherry-pick -s inserts blank line after non-conforming footer' '
+	pristine_detach initial &&
+	git cherry-pick -s mesg-broken-footer &&
+	cat <<-EOF >expect &&
+		$mesg_broken_footer
+
+		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s inserts blank line when conforming footer not found' '
+	pristine_detach initial &&
+	git cherry-pick -s mesg-no-footer &&
+	cat <<-EOF >expect &&
+		$mesg_no_footer
+
+		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s adds sob when last sob doesnt match committer' '
+	pristine_detach initial &&
+	git cherry-pick -s mesg-with-footer &&
+	cat <<-EOF >expect &&
+		$mesg_with_footer
+		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s refrains from adding duplicate trailing sob' '
+	pristine_detach initial &&
+	git cherry-pick -s mesg-with-footer-sob &&
+	cat <<-EOF >expect &&
+		$mesg_with_footer_sob
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
+test_done
-- 
1.8.0.284.g959048a

^ permalink raw reply related

* [PATCH 04/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
From: Brandon Casey @ 2012-11-26  1:45 UTC (permalink / raw)
  To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>

When 'cherry-pick -s' is used to append a signed-off-by line to a cherry
picked commit, it does not currently detect the "(cherry picked from..."
that may have been appended by a previous 'cherry-pick -x' as part of the
s-o-b footer and it will insert a blank line before appending a new s-o-b.

Let's detect "(cherry picked from...)" as part of the footer so that we
will produce this:

   Signed-off-by: A U Thor <author@example.com>
   (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
   Signed-off-by: C O Mitter <committer@example.com>

instead of this:

   Signed-off-by: A U Thor <author@example.com>
   (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)

   Signed-off-by: C O Mitter <committer@example.com>

Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 sequencer.c              | 42 ++++++++++++++++++++++++------------
 t/t3511-cherry-pick-x.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+), 13 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 656df6b..19eaf11 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -18,6 +18,7 @@
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
 const char sign_off_header[] = "Signed-off-by: ";
+static const char cherry_picked_prefix[] = "(cherry picked from commit ";
 
 static void remove_sequencer_state(void)
 {
@@ -492,7 +493,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 		}
 
 		if (opts->record_origin) {
-			strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+			strbuf_addstr(&msgbuf, cherry_picked_prefix);
 			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
 			strbuf_addstr(&msgbuf, ")\n");
 		}
@@ -1017,11 +1018,32 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 	return pick_commits(todo_list, opts);
 }
 
-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
+static int is_rfc2822_line(const char *buf, int len)
+{
+	int i;
+
+	for (i = 0; i < len; i++) {
+		int ch = buf[i];
+		if (ch == ':')
+			break;
+		if (isalnum(ch) || (ch == '-'))
+			continue;
+		return 0;
+	}
+
+	return 1;
+}
+
+static int is_cherry_pick_from_line(const char *buf, int len)
+{
+	return (strlen(cherry_picked_prefix) + 41) <= len &&
+		!prefixcmp(buf, cherry_picked_prefix);
+}
+
+static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
 {
-	int ch;
 	int hit = 0;
-	int i, j, k;
+	int i, k;
 	int len = sb->len - ignore_footer;
 	const char *buf = sb->buf;
 
@@ -1039,15 +1061,9 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
 			; /* do nothing */
 		k++;
 
-		for (j = 0; i + j < len; j++) {
-			ch = buf[i + j];
-			if (ch == ':')
-				break;
-			if (isalnum(ch) ||
-			    (ch == '-'))
-				continue;
+		if (!(is_rfc2822_line(buf+i, k-i) ||
+			is_cherry_pick_from_line(buf+i, k-i)))
 			return 0;
-		}
 	}
 	return 1;
 }
@@ -1064,7 +1080,7 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer)
 	for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--)
 		; /* do nothing */
 	if (prefixcmp(msgbuf->buf + i, sob.buf)) {
-		if (!i || !ends_rfc2822_footer(msgbuf, ignore_footer))
+		if (!i || !has_conforming_footer(msgbuf, ignore_footer))
 			strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
 		strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len);
 	}
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index a6c4168..32c0bb1 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -32,6 +32,10 @@ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
 mesg_with_footer_sob="$mesg_with_footer
 Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
 
+mesg_with_cherry_footer="$mesg_with_footer_sob
+(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)
+Tested-by: C.U. Thor <cuthor@example.com>"
+
 
 test_expect_success setup '
 	git config advice.detachedhead false &&
@@ -47,6 +51,8 @@ test_expect_success setup '
 	test_commit "$mesg_with_footer" foo b mesg-with-footer &&
 	git reset --hard initial &&
 	test_commit "$mesg_with_footer_sob" foo b mesg-with-footer-sob &&
+	git reset --hard initial &&
+	test_commit "$mesg_with_cherry_footer" foo b mesg-with-cherry-footer &&
 	pristine_detach initial &&
 	test_commit conflicting unrelated
 '
@@ -98,6 +104,19 @@ test_expect_success 'cherry-pick -s adds sob when last sob doesnt match committe
 	test_cmp expect actual
 '
 
+test_expect_success 'cherry-pick -x -s adds sob when last sob doesnt match committer' '
+	pristine_detach initial &&
+	sha1=`git rev-parse mesg-with-footer^0` &&
+	git cherry-pick -x -s mesg-with-footer &&
+	cat <<-EOF >expect &&
+		$mesg_with_footer
+		(cherry picked from commit $sha1)
+		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'cherry-pick -s refrains from adding duplicate trailing sob' '
 	pristine_detach initial &&
 	git cherry-pick -s mesg-with-footer-sob &&
@@ -108,4 +127,40 @@ test_expect_success 'cherry-pick -s refrains from adding duplicate trailing sob'
 	test_cmp expect actual
 '
 
+test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists for committer' '
+	pristine_detach initial &&
+	sha1=`git rev-parse mesg-with-footer-sob^0` &&
+	git cherry-pick -x -s mesg-with-footer-sob &&
+	cat <<-EOF >expect &&
+		$mesg_with_footer_sob
+		(cherry picked from commit $sha1)
+		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -x treats "(cherry picked from..." line as part of footer' '
+	pristine_detach initial &&
+	sha1=`git rev-parse mesg-with-cherry-footer^0` &&
+	git cherry-pick -x mesg-with-cherry-footer &&
+	cat <<-EOF >expect &&
+		$mesg_with_cherry_footer
+		(cherry picked from commit $sha1)
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s treats "(cherry picked from..." line as part of footer' '
+	pristine_detach initial &&
+	git cherry-pick -s mesg-with-cherry-footer &&
+	cat <<-EOF >expect &&
+		$mesg_with_cherry_footer
+		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.8.0.284.g959048a

^ permalink raw reply related

* [PATCH 05/11] sequencer.c: always separate "(cherry picked from" from commit body
From: Brandon Casey @ 2012-11-26  1:45 UTC (permalink / raw)
  To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>

Start treating the "(cherry picked from" line added by cherry-pick -x
the same way that the s-o-b lines are treated.  Namely, separate them
from the main commit message body with an empty line.

Introduce tests to test this functionality.

Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 sequencer.c              | 106 +++++++++++++++++++++++++----------------------
 t/t3511-cherry-pick-x.sh |  53 ++++++++++++++++++++++++
 2 files changed, 109 insertions(+), 50 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 19eaf11..7c0852a 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -20,6 +20,60 @@
 const char sign_off_header[] = "Signed-off-by: ";
 static const char cherry_picked_prefix[] = "(cherry picked from commit ";
 
+static int is_rfc2822_line(const char *buf, int len)
+{
+	int i;
+
+	for (i = 0; i < len; i++) {
+		int ch = buf[i];
+		if (ch == ':')
+			break;
+		if (isalnum(ch) || (ch == '-'))
+			continue;
+		return 0;
+	}
+
+	return 1;
+}
+
+static int is_cherry_pick_from_line(const char *buf, int len)
+{
+	return (strlen(cherry_picked_prefix) + 41) <= len &&
+		!prefixcmp(buf, cherry_picked_prefix);
+}
+
+static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
+{
+	int hit = 0;
+	int i, k;
+	int len = sb->len - ignore_footer;
+	const char *buf = sb->buf;
+
+	for (i = len - 1; i > 0; i--) {
+		if (hit && buf[i] == '\n')
+			break;
+		hit = (buf[i] == '\n');
+	}
+
+	/* require at least one blank line */
+	if (!hit || buf[i] != '\n')
+		return 0;
+
+	while (i < len - 1 && buf[i] == '\n')
+		i++;
+
+	for (; i < len; i = k) {
+		for (k = i; k < len && buf[k] != '\n'; k++)
+			; /* do nothing */
+		k++;
+
+		if (!(is_rfc2822_line(buf+i, k-i) ||
+			is_cherry_pick_from_line(buf+i, k-i)))
+			return 0;
+	}
+	return 1;
+}
+
 static void remove_sequencer_state(void)
 {
 	struct strbuf seq_dir = STRBUF_INIT;
@@ -493,6 +547,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 		}
 
 		if (opts->record_origin) {
+			if (!has_conforming_footer(&msgbuf, 0))
+				strbuf_addch(&msgbuf, '\n');
 			strbuf_addstr(&msgbuf, cherry_picked_prefix);
 			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
 			strbuf_addstr(&msgbuf, ")\n");
@@ -1018,56 +1074,6 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 	return pick_commits(todo_list, opts);
 }
 
-static int is_rfc2822_line(const char *buf, int len)
-{
-	int i;
-
-	for (i = 0; i < len; i++) {
-		int ch = buf[i];
-		if (ch == ':')
-			break;
-		if (isalnum(ch) || (ch == '-'))
-			continue;
-		return 0;
-	}
-
-	return 1;
-}
-
-static int is_cherry_pick_from_line(const char *buf, int len)
-{
-	return (strlen(cherry_picked_prefix) + 41) <= len &&
-		!prefixcmp(buf, cherry_picked_prefix);
-}
-
-static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
-{
-	int hit = 0;
-	int i, k;
-	int len = sb->len - ignore_footer;
-	const char *buf = sb->buf;
-
-	for (i = len - 1; i > 0; i--) {
-		if (hit && buf[i] == '\n')
-			break;
-		hit = (buf[i] == '\n');
-	}
-
-	while (i < len - 1 && buf[i] == '\n')
-		i++;
-
-	for (; i < len; i = k) {
-		for (k = i; k < len && buf[k] != '\n'; k++)
-			; /* do nothing */
-		k++;
-
-		if (!(is_rfc2822_line(buf+i, k-i) ||
-			is_cherry_pick_from_line(buf+i, k-i)))
-			return 0;
-	}
-	return 1;
-}
-
 void append_signoff(struct strbuf *msgbuf, int ignore_footer)
 {
 	struct strbuf sob = STRBUF_INIT;
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 32c0bb1..9dd6d5d 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -57,6 +57,19 @@ test_expect_success setup '
 	test_commit conflicting unrelated
 '
 
+test_expect_success 'cherry-pick -x inserts blank line after one line subject' '
+	pristine_detach initial &&
+	sha1=`git rev-parse mesg-one-line^0` &&
+	git cherry-pick -x mesg-one-line &&
+	cat <<-EOF >expect &&
+		$mesg_one_line
+
+		(cherry picked from commit $sha1)
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'cherry-pick -s inserts blank line after one line subject' '
 	pristine_detach initial &&
 	git cherry-pick -s mesg-one-line &&
@@ -81,6 +94,19 @@ test_expect_failure 'cherry-pick -s inserts blank line after non-conforming foot
 	test_cmp expect actual
 '
 
+test_expect_success 'cherry-pick -x inserts blank line when conforming footer not found' '
+	pristine_detach initial &&
+	sha1=`git rev-parse mesg-no-footer^0` &&
+	git cherry-pick -x mesg-no-footer &&
+	cat <<-EOF >expect &&
+		$mesg_no_footer
+
+		(cherry picked from commit $sha1)
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'cherry-pick -s inserts blank line when conforming footer not found' '
 	pristine_detach initial &&
 	git cherry-pick -s mesg-no-footer &&
@@ -93,6 +119,20 @@ test_expect_success 'cherry-pick -s inserts blank line when conforming footer no
 	test_cmp expect actual
 '
 
+test_expect_success 'cherry-pick -x -s inserts blank line when conforming footer not found' '
+	pristine_detach initial &&
+	sha1=`git rev-parse mesg-no-footer^0` &&
+	git cherry-pick -x -s mesg-no-footer &&
+	cat <<-EOF >expect &&
+		$mesg_no_footer
+
+		(cherry picked from commit $sha1)
+		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'cherry-pick -s adds sob when last sob doesnt match committer' '
 	pristine_detach initial &&
 	git cherry-pick -s mesg-with-footer &&
@@ -163,4 +203,17 @@ test_expect_success 'cherry-pick -s treats "(cherry picked from..." line as part
 	test_cmp expect actual
 '
 
+test_expect_success 'cherry-pick -x -s treats "(cherry picked from..." line as part of footer' '
+	pristine_detach initial &&
+	sha1=`git rev-parse mesg-with-cherry-footer^0` &&
+	git cherry-pick -x -s mesg-with-cherry-footer &&
+	cat <<-EOF >expect &&
+		$mesg_with_cherry_footer
+		(cherry picked from commit $sha1)
+		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+	EOF
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.8.0.284.g959048a

^ permalink raw reply related

* [PATCH 06/11] sequencer.c: teach append_signoff how to detect duplicate s-o-b
From: Brandon Casey @ 2012-11-26  1:45 UTC (permalink / raw)
  To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>

Teach append_signoff how to detect a duplicate s-o-b in the commit footer.
This is in preparation to unify the append_signoff implementations in
log-tree.c and sequencer.c.

Fixes test in t3511.

Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 builtin/commit.c         |  2 +-
 sequencer.c              | 43 +++++++++++++++++++++++++++++++------------
 sequencer.h              |  2 +-
 t/t3511-cherry-pick-x.sh |  2 +-
 4 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 1dd2ec5..7b9e2ac 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -700,7 +700,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			previous = eol;
 		}
 
-		append_signoff(&sb, ignore_footer);
+		append_signoff(&sb, ignore_footer, 0);
 	}
 
 	if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
diff --git a/sequencer.c b/sequencer.c
index 7c0852a..3062ad4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -42,12 +42,19 @@ static int is_cherry_pick_from_line(const char *buf, int len)
 		!prefixcmp(buf, cherry_picked_prefix);
 }
 
-static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
+/* Returns 0 for non-conforming footer
+ * Returns 1 for conforming footer
+ * Returns 2 when sob exists within conforming footer
+ * Returns 3 when sob exists within conforming footer as last entry
+ */
+static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
+	int ignore_footer)
 {
 	int hit = 0;
-	int i, k;
+	int i, k = 0;
 	int len = sb->len - ignore_footer;
 	const char *buf = sb->buf;
+	int found_sob = 0;
 
 	for (i = len - 1; i > 0; i--) {
 		if (hit && buf[i] == '\n')
@@ -63,14 +70,24 @@ static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
 		i++;
 
 	for (; i < len; i = k) {
+		int found_rfc2822;
+
 		for (k = i; k < len && buf[k] != '\n'; k++)
 			; /* do nothing */
 		k++;
 
-		if (!(is_rfc2822_line(buf+i, k-i) ||
-			is_cherry_pick_from_line(buf+i, k-i)))
+		found_rfc2822 = is_rfc2822_line(buf+i, k-i);
+		if (found_rfc2822 && sob &&
+			!strncasecmp(buf+i, sob->buf, sob->len))
+			found_sob = k;
+
+		if (!(found_rfc2822 || is_cherry_pick_from_line(buf+i, k-i)))
 			return 0;
 	}
+	if (found_sob == i)
+		return 3;
+	if (found_sob)
+		return 2;
 	return 1;
 }
 
@@ -291,7 +308,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
 	rollback_lock_file(&index_lock);
 
 	if (opts->signoff)
-		append_signoff(msgbuf, 0);
+		append_signoff(msgbuf, 0, 0);
 
 	if (!clean) {
 		int i;
@@ -547,7 +564,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 		}
 
 		if (opts->record_origin) {
-			if (!has_conforming_footer(&msgbuf, 0))
+			if (!has_conforming_footer(&msgbuf, NULL, 0))
 				strbuf_addch(&msgbuf, '\n');
 			strbuf_addstr(&msgbuf, cherry_picked_prefix);
 			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
@@ -1074,9 +1091,10 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 	return pick_commits(todo_list, opts);
 }
 
-void append_signoff(struct strbuf *msgbuf, int ignore_footer)
+void append_signoff(struct strbuf *msgbuf, int ignore_footer, int no_dup_sob)
 {
 	struct strbuf sob = STRBUF_INIT;
+	int has_footer = 0;
 	int i;
 
 	strbuf_addstr(&sob, sign_off_header);
@@ -1085,10 +1103,11 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer)
 	strbuf_addch(&sob, '\n');
 	for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--)
 		; /* do nothing */
-	if (prefixcmp(msgbuf->buf + i, sob.buf)) {
-		if (!i || !has_conforming_footer(msgbuf, ignore_footer))
-			strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
-		strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len);
-	}
+	if (!i || !(has_footer =
+		has_conforming_footer(msgbuf, &sob, ignore_footer)))
+		strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
+	if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
+		strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
+				sob.buf, sob.len);
 	strbuf_release(&sob);
 }
diff --git a/sequencer.h b/sequencer.h
index 9d57d57..c4c7132 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -48,6 +48,6 @@ int sequencer_pick_revisions(struct replay_opts *opts);
 
 extern const char sign_off_header[];
 
-void append_signoff(struct strbuf *msgbuf, int ignore_footer);
+void append_signoff(struct strbuf *msgbuf, int ignore_footer, int no_dup_sob);
 
 #endif
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 9dd6d5d..4b67343 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -82,7 +82,7 @@ test_expect_success 'cherry-pick -s inserts blank line after one line subject' '
 	test_cmp expect actual
 '
 
-test_expect_failure 'cherry-pick -s inserts blank line after non-conforming footer' '
+test_expect_success 'cherry-pick -s inserts blank line after non-conforming footer' '
 	pristine_detach initial &&
 	git cherry-pick -s mesg-broken-footer &&
 	cat <<-EOF >expect &&
-- 
1.8.0.284.g959048a

^ permalink raw reply related

* [PATCH 07/11] sequencer.c: teach append_signoff to avoid adding a duplicate newline
From: Brandon Casey @ 2012-11-26  1:45 UTC (permalink / raw)
  To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>

Teach append_signoff to detect whether a blank line exists at the position
that the signed-off-by line will be added, and avoid adding an additional
one if one already exists.  This is necessary to allow format-patch to add a
s-o-b to a patch with no commit message without adding an extra newline.  A
following patch will make format-patch use this function rather than the
append_signoff implementation inside log-tree.c.

Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 sequencer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 3062ad4..eb93dd6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1103,8 +1103,8 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, int no_dup_sob)
 	strbuf_addch(&sob, '\n');
 	for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--)
 		; /* do nothing */
-	if (!i || !(has_footer =
-		has_conforming_footer(msgbuf, &sob, ignore_footer)))
+	if (msgbuf->buf[i] != '\n' && (!i || !(has_footer =
+		has_conforming_footer(msgbuf, &sob, ignore_footer))))
 		strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
 	if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
 		strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
-- 
1.8.0.284.g959048a

^ permalink raw reply related

* [PATCH 08/11] t4014: more tests about appending s-o-b lines
From: Brandon Casey @ 2012-11-26  1:45 UTC (permalink / raw)
  To: git; +Cc: pclouds, gitster, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

[bc: fix test 90 "signoff: some random signoff-alike" and mark as failing.
     Correct behavior should insert a blank line after message body and
     signed-off-by ]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 t/t4014-format-patch.sh | 146 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 146 insertions(+)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 16a4ca1..dfe9209 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -963,4 +963,150 @@ test_expect_success 'format patch ignores color.ui' '
 	test_cmp expect actual
 '
 
+append_signoff()
+{
+	C=`git commit-tree HEAD^^{tree} -p HEAD` &&
+	git format-patch --stdout --signoff ${C}^..${C} |
+		tee append_signoff.patch |
+		sed -n "1,/^---$/p" |
+		grep -n -E "^Subject|Sign|^$"
+}
+
+test_expect_success 'signoff: commit with no body' '
+	append_signoff </dev/null >actual &&
+	cat <<\EOF | sed "s/EOL$//" >expected &&
+4:Subject: [PATCH] EOL
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: commit with only subject' '
+	echo subject | append_signoff >actual &&
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: commit with only subject that does not end with NL' '
+	echo -n subject | append_signoff >actual &&
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: no existing signoffs' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: no existing signoffs and no trailing NL' '
+	printf "subject\n\nbody" | append_signoff >actual &&
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: some random signoff' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: my@house
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: my@house
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_failure 'signoff: some random signoff-alike' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+Fooled-by-me: my@house
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+11:
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff at the end' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff at the end, no trailing NL' '
+	printf "subject\n\nSigned-off-by: C O Mitter <committer@example.com>" |
+		append_signoff >actual &&
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff NOT at the end' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: C O Mitter <committer@example.com>
+Signed-off-by: my@house
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+12:Signed-off-by: my@house
+EOF
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.0.284.g959048a

^ permalink raw reply related

* [PATCH 09/11] format-patch: stricter S-o-b detection
From: Brandon Casey @ 2012-11-26  1:45 UTC (permalink / raw)
  To: git; +Cc: pclouds, gitster, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

S-o-b in the middle of a sentence, at the beginning of the sentence
but it is just because of text wrapping, or a full paragraph of valid
S-o-b in the middle of the message. All these are not S-o-b, but
detected as is. Fix them.

[bc: add two additional tests to demonstrate shortcomings of the current
   code:

   1. failure to detect non-conforming elements in the footer when last
      line matches committer's s-o-b.
   2. failure to handle various s-o-b -like elements in the footer as
      conforming. e.g. "Change-id: IXXXX or Bug: 1234"

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 log-tree.c              | 37 +++++++++++++++++--
 t/t4014-format-patch.sh | 95 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 130 insertions(+), 2 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 4f86def..14ebf69 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -260,14 +260,47 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
 	int has_signoff = 0;
 	char *cp;
 
-	cp = sb->buf;
+	/*
+	 * Only search in the last paragrah, don't be fooled by a
+	 * paragraph full of valid S-o-b in the middle.
+	 */
+	cp = sb->buf + sb->len - 1;
+	while (cp > sb->buf) {
+		if (cp[0] == '\n' && cp[1] == '\n') {
+			cp += 2;
+			break;
+		}
+		cp--;
+	}
 
 	/* First see if we already have the sign-off by the signer */
 	while ((cp = strstr(cp, signed_off_by))) {
+		const char *s = cp;
+		cp += strlen(signed_off_by);
+
+		if (!has_signoff && s > sb->buf) {
+			/*
+			 * S-o-b in the middle of a sentence is not
+			 * really S-o-b
+			 */
+			if (s[-1] != '\n')
+				continue;
+
+			if (s - 1 > sb->buf && s[-2] == '\n')
+				; /* the first S-o-b */
+			else if (!detect_any_signoff(sb->buf, s - sb->buf))
+				/*
+				 * The previous line looks like an
+				 * S-o-b. There's still no guarantee
+				 * that it's an actual S-o-b. For that
+				 * we need to look back until we find
+				 * a blank line, which is too costly.
+				 */
+				continue;
+		}
 
 		has_signoff = 1;
 
-		cp += strlen(signed_off_by);
 		if (cp + signoff_len >= sb->buf + sb->len)
 			break;
 		if (strncmp(cp, signoff, signoff_len))
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index dfe9209..30e37a7 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1062,6 +1062,60 @@ EOF
 	test_cmp expected actual
 '
 
+test_expect_success 'signoff: not really a signoff' '
+	append_signoff <<\EOF >actual &&
+subject
+
+I want to mention about Signed-off-by: here.
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:I want to mention about Signed-off-by: here.
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: not really a signoff (2)' '
+	append_signoff <<\EOF >actual &&
+subject
+
+My unfortunate
+Signed-off-by: example happens to be wrapped here.
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:Signed-off-by: example happens to be wrapped here.
+11:
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: valid S-o-b paragraph in the middle' '
+	append_signoff <<\EOF >actual &&
+subject
+
+Signed-off-by: my@house
+Signed-off-by: your@house
+
+A lot of houses.
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: my@house
+10:Signed-off-by: your@house
+11:
+13:
+14:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'signoff: the same signoff at the end' '
 	append_signoff <<\EOF >actual &&
 subject
@@ -1109,4 +1163,45 @@ EOF
 	test_cmp expected actual
 '
 
+test_expect_failure 'signoff: detect garbage in non-conforming footer' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Tested-by: my@house
+Some Trash
+Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+13:Signed-off-by: C O Mitter <committer@example.com>
+14:
+15:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_failure 'signoff: footer begins with non-signoff without @ sign' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Tested-by: my@house
+Change-id: Ideadbeef
+Signed-off-by: C O Mitter <committer@example.com>
+Bug: 1234
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+13:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.0.284.g959048a

^ permalink raw reply related

* [PATCH 10/11] format-patch: update append_signoff prototype
From: Brandon Casey @ 2012-11-26  1:45 UTC (permalink / raw)
  To: git; +Cc: pclouds, gitster, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

This is a preparation step for merging with append_signoff from
sequencer.c

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 builtin/log.c | 13 +------------
 log-tree.c    | 21 +++++++++++++--------
 revision.h    |  2 +-
 3 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index e7b7db1..bb48344 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1058,7 +1058,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct commit *origin = NULL, *head = NULL;
 	const char *in_reply_to = NULL;
 	struct patch_ids ids;
-	char *add_signoff = NULL;
 	struct strbuf buf = STRBUF_INIT;
 	int use_patch_format = 0;
 	int quiet = 0;
@@ -1154,16 +1153,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
 			     PARSE_OPT_KEEP_DASHDASH);
 
-	if (do_signoff) {
-		const char *committer;
-		const char *endpos;
-		committer = git_committer_info(IDENT_STRICT);
-		endpos = strchr(committer, '>');
-		if (!endpos)
-			die(_("bogus committer info %s"), committer);
-		add_signoff = xmemdupz(committer, endpos - committer + 1);
-	}
-
 	for (i = 0; i < extra_hdr.nr; i++) {
 		strbuf_addstr(&buf, extra_hdr.items[i].string);
 		strbuf_addch(&buf, '\n');
@@ -1354,7 +1343,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		total++;
 		start_number--;
 	}
-	rev.add_signoff = add_signoff;
+	rev.add_signoff = do_signoff;
 	while (0 <= --nr) {
 		int shown;
 		commit = list[nr];
diff --git a/log-tree.c b/log-tree.c
index 14ebf69..be8e7ab 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -253,9 +253,11 @@ static int detect_any_signoff(char *letter, int size)
 	return seen_head && seen_name;
 }
 
-static void append_signoff(struct strbuf *sb, const char *signoff)
+static void append_signoff(struct strbuf *sb, int flags)
 {
-	static const char signed_off_by[] = "Signed-off-by: ";
+	char* signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
+					 getenv("GIT_COMMITTER_EMAIL")));
+	static const char sign_off_header[] = "Signed-off-by: ";
 	size_t signoff_len = strlen(signoff);
 	int has_signoff = 0;
 	char *cp;
@@ -274,9 +276,9 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
 	}
 
 	/* First see if we already have the sign-off by the signer */
-	while ((cp = strstr(cp, signed_off_by))) {
+	while ((cp = strstr(cp, sign_off_header))) {
 		const char *s = cp;
-		cp += strlen(signed_off_by);
+		cp += strlen(sign_off_header);
 
 		if (!has_signoff && s > sb->buf) {
 			/*
@@ -317,9 +319,10 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
 	if (!has_signoff)
 		strbuf_addch(sb, '\n');
 
-	strbuf_addstr(sb, signed_off_by);
+	strbuf_addstr(sb, sign_off_header);
 	strbuf_add(sb, signoff, signoff_len);
 	strbuf_addch(sb, '\n');
+	free(signoff);
 }
 
 static unsigned int digits_in_number(unsigned int number)
@@ -695,8 +698,10 @@ void show_log(struct rev_info *opt)
 	/*
 	 * And then the pretty-printed message itself
 	 */
-	if (ctx.need_8bit_cte >= 0)
-		ctx.need_8bit_cte = has_non_ascii(opt->add_signoff);
+	if (ctx.need_8bit_cte >= 0 && opt->add_signoff)
+		ctx.need_8bit_cte =
+			has_non_ascii(fmt_name(getenv("GIT_COMMITTER_NAME"),
+					       getenv("GIT_COMMITTER_EMAIL")));
 	ctx.date_mode = opt->date_mode;
 	ctx.date_mode_explicit = opt->date_mode_explicit;
 	ctx.abbrev = opt->diffopt.abbrev;
@@ -707,7 +712,7 @@ void show_log(struct rev_info *opt)
 	pretty_print_commit(&ctx, commit, &msgbuf);
 
 	if (opt->add_signoff)
-		append_signoff(&msgbuf, opt->add_signoff);
+		append_signoff(&msgbuf, 0);
 
 	if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
 	    ctx.notes_message && *ctx.notes_message) {
diff --git a/revision.h b/revision.h
index 059bfff..435a60b 100644
--- a/revision.h
+++ b/revision.h
@@ -137,7 +137,7 @@ struct rev_info {
 	int		numbered_files;
 	char		*message_id;
 	struct string_list *ref_message_ids;
-	const char	*add_signoff;
+	int              add_signoff;
 	const char	*extra_headers;
 	const char	*log_reencode;
 	const char	*subject_prefix;
-- 
1.8.0.284.g959048a

^ permalink raw reply related

* [PATCH 11/11] Unify appending signoff in format-patch, commit and sequencer
From: Brandon Casey @ 2012-11-26  1:45 UTC (permalink / raw)
  To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>

There are two implementations of append_signoff in log-tree.c and
sequencer.c, which do more or less the same thing.  Unify on top of the
sequencer.c implementation.

Add a test in t4014 to demonstrate support for non-s-o-b elements in the
commit footer provided by sequence.c:append_sob.  Mark tests fixed as
appropriate.

[Commit message mostly stolen from Nguyễn Thái Ngọc Duy's original
 unification patch]

Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 log-tree.c              | 122 +-----------------------------------------------
 t/t4014-format-patch.sh |  27 +++++++++--
 2 files changed, 26 insertions(+), 123 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index be8e7ab..1fb0a16 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -9,6 +9,7 @@
 #include "string-list.h"
 #include "color.h"
 #include "gpg-interface.h"
+#include "sequencer.h"
 
 struct decoration name_decoration = { "object names" };
 
@@ -206,125 +207,6 @@ void show_decorations(struct rev_info *opt, struct commit *commit)
 	putchar(')');
 }
 
-/*
- * Search for "^[-A-Za-z]+: [^@]+@" pattern. It usually matches
- * Signed-off-by: and Acked-by: lines.
- */
-static int detect_any_signoff(char *letter, int size)
-{
-	char *cp;
-	int seen_colon = 0;
-	int seen_at = 0;
-	int seen_name = 0;
-	int seen_head = 0;
-
-	cp = letter + size;
-	while (letter <= --cp && *cp == '\n')
-		continue;
-
-	while (letter <= cp) {
-		char ch = *cp--;
-		if (ch == '\n')
-			break;
-
-		if (!seen_at) {
-			if (ch == '@')
-				seen_at = 1;
-			continue;
-		}
-		if (!seen_colon) {
-			if (ch == '@')
-				return 0;
-			else if (ch == ':')
-				seen_colon = 1;
-			else
-				seen_name = 1;
-			continue;
-		}
-		if (('A' <= ch && ch <= 'Z') ||
-		    ('a' <= ch && ch <= 'z') ||
-		    ch == '-') {
-			seen_head = 1;
-			continue;
-		}
-		/* no empty last line doesn't match */
-		return 0;
-	}
-	return seen_head && seen_name;
-}
-
-static void append_signoff(struct strbuf *sb, int flags)
-{
-	char* signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
-					 getenv("GIT_COMMITTER_EMAIL")));
-	static const char sign_off_header[] = "Signed-off-by: ";
-	size_t signoff_len = strlen(signoff);
-	int has_signoff = 0;
-	char *cp;
-
-	/*
-	 * Only search in the last paragrah, don't be fooled by a
-	 * paragraph full of valid S-o-b in the middle.
-	 */
-	cp = sb->buf + sb->len - 1;
-	while (cp > sb->buf) {
-		if (cp[0] == '\n' && cp[1] == '\n') {
-			cp += 2;
-			break;
-		}
-		cp--;
-	}
-
-	/* First see if we already have the sign-off by the signer */
-	while ((cp = strstr(cp, sign_off_header))) {
-		const char *s = cp;
-		cp += strlen(sign_off_header);
-
-		if (!has_signoff && s > sb->buf) {
-			/*
-			 * S-o-b in the middle of a sentence is not
-			 * really S-o-b
-			 */
-			if (s[-1] != '\n')
-				continue;
-
-			if (s - 1 > sb->buf && s[-2] == '\n')
-				; /* the first S-o-b */
-			else if (!detect_any_signoff(sb->buf, s - sb->buf))
-				/*
-				 * The previous line looks like an
-				 * S-o-b. There's still no guarantee
-				 * that it's an actual S-o-b. For that
-				 * we need to look back until we find
-				 * a blank line, which is too costly.
-				 */
-				continue;
-		}
-
-		has_signoff = 1;
-
-		if (cp + signoff_len >= sb->buf + sb->len)
-			break;
-		if (strncmp(cp, signoff, signoff_len))
-			continue;
-		if (!isspace(cp[signoff_len]))
-			continue;
-		/* we already have him */
-		return;
-	}
-
-	if (!has_signoff)
-		has_signoff = detect_any_signoff(sb->buf, sb->len);
-
-	if (!has_signoff)
-		strbuf_addch(sb, '\n');
-
-	strbuf_addstr(sb, sign_off_header);
-	strbuf_add(sb, signoff, signoff_len);
-	strbuf_addch(sb, '\n');
-	free(signoff);
-}
-
 static unsigned int digits_in_number(unsigned int number)
 {
 	unsigned int i = 10, result = 1;
@@ -712,7 +594,7 @@ void show_log(struct rev_info *opt)
 	pretty_print_commit(&ctx, commit, &msgbuf);
 
 	if (opt->add_signoff)
-		append_signoff(&msgbuf, 0);
+		append_signoff(&msgbuf, 0, 1);
 
 	if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
 	    ctx.notes_message && *ctx.notes_message) {
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 30e37a7..1dea58b 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1046,7 +1046,28 @@ EOF
 	test_cmp expected actual
 '
 
-test_expect_failure 'signoff: some random signoff-alike' '
+test_expect_success 'signoff: misc conforming footer elements' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: my@house
+(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)
+Tested-by: Some One <someone@example.com>
+Bug: 1234
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: my@house
+15:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: some random signoff-alike' '
 	append_signoff <<\EOF >actual &&
 subject
 
@@ -1163,7 +1184,7 @@ EOF
 	test_cmp expected actual
 '
 
-test_expect_failure 'signoff: detect garbage in non-conforming footer' '
+test_expect_success 'signoff: detect garbage in non-conforming footer' '
 	append_signoff <<\EOF >actual &&
 subject
 
@@ -1184,7 +1205,7 @@ EOF
 	test_cmp expected actual
 '
 
-test_expect_failure 'signoff: footer begins with non-signoff without @ sign' '
+test_expect_success 'signoff: footer begins with non-signoff without @ sign' '
 	append_signoff <<\EOF >actual &&
 subject
 
-- 
1.8.0.284.g959048a

^ permalink raw reply related

* Re: Multiple threads of compression
From: Brandon Casey @ 2012-11-26  2:37 UTC (permalink / raw)
  To: Thorsten Glaser; +Cc: git@vger.kernel.org
In-Reply-To: <loom.20121125T171702-64@post.gmane.org>

On Sun, Nov 25, 2012 at 8:27 AM, Thorsten Glaser <tg@debian.org> wrote:

> tl;dr: I would like to have a *global* option for git to restrict
> the number of threads of execution it uses. Several subcommands,
> like pack-objects, are already equipped with an optioin for this,
> but unfortunately, these are seldom invoked by hand¹, so this can’t
> work in my situations.

See the git-config man page.

The number of threads that pack uses can be configured in the global
or system gitconfig file by setting pack.threads.

e.g.

   $ git config --system pack.threads 1

Also, modern git accepts a '-c' option which allows you to set
configuration options on the command line, e.g. 'git -c pack.threads=1
gc'.

The other setting you should probably look at is pack.windowMemory
which should help you control the amount of memory git uses while
packing.  Also look at core.packedGitWindowSize and
core.packedGitLimit if your repository is really large.

>
> ① automatic garbage collection, “git gc --aggressive --prune=now”,
>   and cloning are the use cases I have at hand right now.
>
> À propos, while here: is gc --aggressive safe to run on a live,
> online-shared repository, or does it break other users accessing
> the repository concurrently? (If it’s safe I’d very much like to do
> that in a, say weekly, cronjob on FusionForge, our hosting system.)

Running 'git gc' with --aggressive should be as safe as running it
without --aggressive.

But, you should think about whether you really need to run it more
than once, or at all.  When you use --aggressive, git will perform the
entire delta search again for _every_ object in the repository.  The
general usecase for --aggressive is when you suspect that the original
delta search produced sub-optimal deltas or if you modify the size of
the delta search window or depth and want to regenerate your packed
objects to improve compression or access speed.  Even then, you will
not likely get much benefit from running with --aggressive a second
time.

hth,
-Brandon

^ permalink raw reply

* Re: Requirements for integrating a new git subcommand
From: Junio C Hamano @ 2012-11-26  2:57 UTC (permalink / raw)
  To: esr; +Cc: git
In-Reply-To: <20121122053012.GA17265@thyrsus.com>

"Eric S. Raymond" <esr@thyrsus.com> writes:

> While the weave operation can build a commit graph with any structure
> desired, an important restriction of the inverse (unraveling)
> operation is that it operates on *master branches only*. The unravel
> operation discards non-master-branch content, emitting a warning 
> to standard error when it has to do so.

Imagine that I have a simple four-commit diamond:

    I---A
     \   \
      B---M

where Amy and Bob forked the initial commit made by Ian and created
one commit each, and their branches were merged into one 'master'
branch by a merge commit made by Mac.  How many state snapshots will
I see when I ask you to unravel this?  Three, or four?

If you are going to give me all four states, then I do not
understand why this needs to be limited to the master branch only.
Even if you start from a single commit at the tip of 'master', once
you hit a merge, you will need to follow all of two (or more) paths
to dig down to the root(s), so supporting to start digging from more
than one commit is not all that different.

If you are going to give me only three states, following the first
parent ancestry chain, then the description needs to state it more
clearly.  I am not saying first-parent-only history is useless, but
the user needs to know that merges are flattened in the unraveled
result and the resulting history becomes linear, following the first
parent ancestry chain of the original history (if that is what the
tool does) before deciding if this tool matches what she needs.

As to the procedural stuff, I think others have sufficiently
answered already.  If I may add something, a new stuff typically
start its life in contrib/ before it proves to be useful.

^ permalink raw reply

* Re: [PATCH] diff: Fixes shortstat number of files
From: Junio C Hamano @ 2012-11-26  3:28 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: git
In-Reply-To: <1353533210-29684-1-git-send-email-apelisse@gmail.com>

Antoine Pelisse <apelisse@gmail.com> writes:

> Subject: Re: [PATCH] diff: Fixes shortstat number of files

Please replace "Fixes" with "Fix at least (because our log messages
are written as if a patch is giving an order to the codebase, iow,
in imperative mood), but we would prefer to see a concrete
description on what is fixed, when we can.  And in this case, I
think we can, perhaps:

    diff: do not count unmerged paths twice in --shortstat/--numstat

or something.

> There is a discrepancy between the last line of `git diff --stat`
> and `git diff --shortstat` in case of a merge.
> The unmerged files are actually counted twice, thus doubling the
> value of "file changed".

I think the current 'master' and upward is broken with respect to
this; I am consistently getting two entries for unmerged paths
across --stat, --shortstat and --numstat options (iow, not just
shortstat and numstat but the '--stat' seems to be broken as well).

Thanks.

^ permalink raw reply

* Re: [PATCH] fast-export: Allow pruned-references in mark file
From: Junio C Hamano @ 2012-11-26  4:03 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: git
In-Reply-To: <1353750432-17373-1-git-send-email-apelisse@gmail.com>

Antoine Pelisse <apelisse@gmail.com> writes:

> fast-export can fail because of some pruned-reference when importing a
> mark file.
>
> The problem happens in the following scenario:
>
>     $ git fast-export --export-marks=MARKS master
>     (rewrite master)
>     $ git prune
>     $ git fast-export --import-marks=MARKS master
>
> This might fail if some references have been removed by prune
> because some marks will refer to non-existing commits.
>
> Let's warn when we have a mark for a commit we don't know.
> Also, increment the last_idnum before, so we don't override
> the mark.

Is this a safe and sane thing to do, and if so why?  Could you
describe that in the log message here?

> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---
>  builtin/fast-export.c |   11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 12220ad..141b245 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -607,16 +607,19 @@ static void import_marks(char *input_file)
>  			|| *mark_end != ' ' || get_sha1(mark_end + 1, sha1))
>  			die("corrupt mark line: %s", line);
>  
> +		if (last_idnum < mark)
> +			last_idnum = mark;
> +
>  		object = parse_object(sha1);
> -		if (!object)
> -			die ("Could not read blob %s", sha1_to_hex(sha1));
> +		if (!object) {
> +			warning("Could not read blob %s", sha1_to_hex(sha1));
> +			continue;
> +		}
>  
>  		if (object->flags & SHOWN)
>  			error("Object %s already has a mark", sha1_to_hex(sha1));
>  
>  		mark_object(object, mark);
> -		if (last_idnum < mark)
> -			last_idnum = mark;
>  
>  		object->flags |= SHOWN;
>  	}

^ permalink raw reply

* Re: [PATCH v3 0/7] New remote-bzr remote helper
From: Junio C Hamano @ 2012-11-26  4:09 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <CAMP44s2F9C4cr6v===M3AWHieaiUk1adigcU8txpZPL0wN81Ow@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Sun, Nov 11, 2012 at 3:19 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> This is a re-roll of the previous series to add support to fetch and push
>> special modes, and refactor some related code.
>
> It seems this one got forgotten, I only see v2 in pu.

Oops; I think that was fell through the cracks during the maintainer
hand-off.  As the previous one has already been cooking in 'next'
for a week or so, I would appreciate if you send incremental updates
to fix or enhance what is in there.

Thanks.

^ permalink raw reply

* Re: [PATCH] Add documentation on how to integrate commands.
From: Junio C Hamano @ 2012-11-26  4:47 UTC (permalink / raw)
  To: Eric S. Raymond; +Cc: git
In-Reply-To: <20121124122333.BAD7B4065F@snark.thyrsus.com>

esr@thyrsus.com (Eric S. Raymond) writes:

> ---

Sign off?

>  Documentation/CommandIntegration |   69 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/CommandIntegration
>
> diff --git a/Documentation/CommandIntegration b/Documentation/CommandIntegration
> new file mode 100644
> index 0000000..be248f7
> --- /dev/null
> +++ b/Documentation/CommandIntegration
> @@ -0,0 +1,69 @@
> += Integrating new subcommands =
> +
> +This is how-to documentation for people who want to add extension
> +commands to git.
> +
> +== Runtime environment ==
> +
> +git subcommands are standalone executables that live in the git
> +execution directory, normally /usr/lib/git-core.  The git executable itself
> +is a thin wrapper that sets GIT_DIR and passes command-line arguments
> +to the subcommand.
> +
> +(If "git foo" is not found in the git execution directory, the wrapper
> +will look in the rest of your $PATH for it.  Thus, it's possible

As the first sentence in this paragraph does not make it clear
enough that you are defining a new term "git execution directory",
"execution directory" here may be misleading and can easily be
mistaken as if we look something in the directory where the user
runs "git" in.  We usually call it "exec path".

> +== Implementation languages ==
> +
> +Most subcommands are written in C or shell.  A few are written in
> +Perl.  A tiny minority are written in Python.
> +
> +While we strongly encourage coding in portable C for portability, these
> +specific scripting languages are also acceptable. We won't accept more
> +without a very strong technical case, as we don't want to broaden the
> +git suite's required dependencies.

Actually, we tend to avoid Python dependency for anything important
and allow it only on fringes; people who lack Python environment are
not missing much, and we would want to keep it that way until the
situation on the Windows front changes.

> +C commands are normally written as single modules, named after the
> +command, that link a core library called libgit.  Thus, your command

I would prefer to see this sentence not call libgit.a a "library".
We primarily use libgit.a to let linker pick necessary object files
without us having to list object files for non-builtin command
implementations and it is not designed to be used by other people.

> +== Integrating a command ==
> +
> +Here are the things you need to do when you want to merge a new 
> +subcommand into the git tree.
> +
> +1. Append your command name to one of the variables BUILTIN_OBJS,
> +EXTRA_PROGRAMS, SCRIPT_SH, SCRIPT_PERL or SCRIPT_PYTHON.
> +
> +2. Drop its test in the t directory.
> +
> +That's all there is to it.

And when sending a patch in, do not forget to sign off your patches
;-)

^ permalink raw reply

* Re: Requirements for integrating a new git subcommand
From: Eric S. Raymond @ 2012-11-26  4:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmwy5xe9n.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com>:
> "Eric S. Raymond" <esr@thyrsus.com> writes:
> 
> > While the weave operation can build a commit graph with any structure
> > desired, an important restriction of the inverse (unraveling)
> > operation is that it operates on *master branches only*. The unravel
> > operation discards non-master-branch content, emitting a warning 
> > to standard error when it has to do so.
> 
> Imagine that I have a simple four-commit diamond:
> 
>     I---A
>      \   \
>       B---M
> 
> where Amy and Bob forked the initial commit made by Ian and created
> one commit each, and their branches were merged into one 'master'
> branch by a merge commit made by Mac.  How many state snapshots will
> I see when I ask you to unravel this?  Three, or four?

You will see four tree states.  I have managed to remove the
master-branch-only restriction.

> As to the procedural stuff, I think others have sufficiently
> answered already.  If I may add something, a new stuff typically
> start its life in contrib/ before it proves to be useful.

Thank you, I have submitted a documentation patch which folds
in the on-list discussion.

As a separate point...are you requesting that I submit my integration
patch to drop git-weave in contrib?  If so, I will of course comply.

But I will point out that git-weave is not a half-thought out
experiment; it is fully documented and has a functional test.  The
case for its usefulness is bolstered by one previous contrib script,
which the author has agreed to retire in favor of git-weave.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* Re: [PATCH 0/2] second try
From: Junio C Hamano @ 2012-11-26  4:50 UTC (permalink / raw)
  To: Sven Strickroth; +Cc: git, Jeff King, Jakub Narebski, Eric Wong
In-Reply-To: <50B11AF5.2090701@tu-clausthal.de>

Sven Strickroth <sven.strickroth@tu-clausthal.de> writes:

> Am 11.11.2012 17:40 schrieb Sven Strickroth:
>> Am 06.10.2012 20:28 schrieb Junio C Hamano:
>>> It is either that it was simply forgotten, or after I wrote the part
>>> you quoted early in January there were discussions later that showed
>>> the patch was not desirable for some reason. I do not recall which.
>> 
>> I noticed no threads about possible problems, so I try again.
>
> On November 11th I submitted the updated patches again, however, without
> any reaction or comments.

I think between Peff and me it fell in the cracks during the
hand-off; I do not know about the others, probably people did not
find it interesting perhaps?

I'll add Eric Wong (git-svn submaintainer) to Cc.

^ permalink raw reply

* [ANNOUNCE] Git v1.8.0.1
From: Junio C Hamano @ 2012-11-26  4:54 UTC (permalink / raw)
  To: git; +Cc: Linux Kernel

The latest maintenance release Git v1.8.0.1 is now available at
the usual places.

The release tarballs are found at:

    http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

4e7492f7558f3ba2a450c43efa7de3b0b1adc6c1  git-1.8.0.1.tar.gz
6c0e64d53a8543447f595e3bac4966ba9257e783  git-htmldocs-1.8.0.1.tar.gz
cb26eea4ebe53b41cbc5c5f430499f7d76605414  git-manpages-1.8.0.1.tar.gz

Also the following public repositories all have a copy of the v1.8.0.1
tag and the maint branch that the tag points at:

  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

Git v1.8.0.1 Release Notes
==========================

Fixes since v1.8.0
------------------

 * The configuration parser had an unnecessary hardcoded limit on
   variable names that was not checked consistently.

 * The "say" function in the test scaffolding incorrectly allowed
   "echo" to interpret "\a" as if it were a C-string asking for a
   BEL output.

 * "git mergetool" feeds /dev/null as a common ancestor when dealing
   with an add/add conflict, but p4merge backend cannot handle
   it. Work it around by passing a temporary empty file.

 * "git log -F -E --grep='<ere>'" failed to use the given <ere>
   pattern as extended regular expression, and instead looked for the
   string literally.

 * "git grep -e pattern <tree>" asked the attribute system to read
   "<tree>:.gitattributes" file in the working tree, which was
   nonsense.

 * A symbolic ref refs/heads/SYM was not correctly removed with "git
   branch -d SYM"; the command removed the ref pointed by SYM
   instead.

 * Earlier we fixed documentation to hyphenate "remote-tracking branch"
   to clarify that these are not a remote entity, but unhyphenated
   spelling snuck in to a few places since then.

 * "git pull --rebase" run while the HEAD is detached tried to find
   the upstream branch of the detached HEAD (which by definition
   does not exist) and emitted unnecessary error messages.

 * The refs/replace hierarchy was not mentioned in the
   repository-layout docs.

 * Sometimes curl_multi_timeout() function suggested a wrong timeout
   value when there is no file descriptors to wait on and the http
   transport ended up sleeping for minutes in select(2) system call.
   A workaround has been added for this.

 * Various rfc2047 quoting issues around a non-ASCII name on the
   From: line in the output from format-patch have been corrected.

 * "git diff -G<pattern>" did not honor textconv filter when looking
   for changes.

 * Bash completion script (in contrib/) did not correctly complete a
   lazy "git checkout $name_of_remote_tracking_branch_that_is_unique"
   command line.

 * RSS feed from "gitweb" had a xss hole in its title output.

 * "git config --path $key" segfaulted on "[section] key" (a boolean
   "true" spelled without "=", not "[section] key = true").

 * "git checkout -b foo" while on an unborn branch did not say
   "Switched to a new branch 'foo'" like other cases.

Also contains other minor fixes and documentation updates.

----------------------------------------------------------------

Changes since v1.8.0 are as follows:

Andreas Schwab (1):
      commit: fixup misplacement of --no-post-rewrite description

Ben Walton (1):
      Remove the hard coded length limit on variable names in config files

Carlos Martín Nieto (1):
      config: don't segfault when given --path with a missing value

David Aguilar (1):
      mergetools/p4merge: Handle "/dev/null"

Jan H. Schönherr (7):
      utf8: fix off-by-one wrapping of text
      format-patch: do not wrap non-rfc2047 headers too early
      format-patch: do not wrap rfc2047 encoded headers too late
      format-patch: introduce helper function last_line_length()
      format-patch: make rfc2047 encoding more strict
      format-patch: fix rfc2047 address encoding with respect to rfc822 specials
      format-patch tests: check quoting/encoding in To: and Cc: headers

Jeff King (3):
      diff_grep: use textconv buffers for add/deleted files
      gitweb: escape html in rss title
      checkout: print a message when switching unborn branches

Junio C Hamano (9):
      builtin/grep.c: make configuration callback more reusable
      grep: move the configuration parsing logic to grep.[ch]
      grep: move pattern-type bits support to top-level grep.[ch]
      revisions: initialize revs->grep_filter using grep_init()
      log --grep: use the same helper to set -E/-F options as "git grep"
      test-lib: Fix say_color () not to interpret \a\b\c in the message
      Start preparing for 1.8.0.1
      Further preparation for 1.8.0.1
      Git 1.8.0.1

Marc Khouzam (1):
      Completion must sort before using uniq

Matthieu Moy (2):
      Documentation: remote tracking branch -> remote-tracking branch
      Document 'git commit --no-edit' explicitly

Michael J Gruber (1):
      push/pull: adjust missing upstream help text to changed interface

Nguyễn Thái Ngọc Duy (1):
      grep: stop looking at random places for .gitattributes

Phil Hord (1):
      git-pull: Avoid merge-base on detached head

Philip Oakley (1):
      Doc repository-layout: Show refs/replace

René Scharfe (6):
      refs: lock symref that is to be deleted, not its target
      branch: factor out check_branch_commit()
      branch: factor out delete_branch_config()
      branch: delete symref branch, not its target
      branch: skip commit checks when deleting symref branches
      branch: show targets of deleted symrefs, not sha1s

Romain Francoise (1):
      mailmap: avoid out-of-bounds memory access

Stefan Zager (1):
      Fix potential hang in https handshake

^ permalink raw reply

* Can I zip a git repo without losing anything?
From: Carl Smith @ 2012-11-26  4:55 UTC (permalink / raw)
  To: git

Hi all

This is my first post to this list, so thank you for all your work.

After suggesting using zip files to move our projects around, I was
told that you can not zip a git repo without loosing all the history.
This didn't make sense to me, but two people told me the same thing,
so I wasn't sure. I think they may have been confusing the zipped file
you can download from GitHub with a zipped git repo.

If someone could put me straight on this, I'd really appreciate it.

Thanks

Carl

^ permalink raw reply

* Re: Can I zip a git repo without losing anything?
From: Andrew Ardill @ 2012-11-26  5:06 UTC (permalink / raw)
  To: Carl Smith; +Cc: git@vger.kernel.org
In-Reply-To: <CAP-uhDcQg0BuEdHDTa6qVqLCeB-LE1GZtEqHgY_j1--XodsDKw@mail.gmail.com>

On 26 November 2012 15:55, Carl Smith <carl.input@gmail.com> wrote:
> Hi all
>
> This is my first post to this list, so thank you for all your work.
>
> After suggesting using zip files to move our projects around, I was
> told that you can not zip a git repo without loosing all the history.
> This didn't make sense to me, but two people told me the same thing,
> so I wasn't sure. I think they may have been confusing the zipped file
> you can download from GitHub with a zipped git repo.

Hi Carl,

The basics of it are as follows, a little simplified to hopefully make it clear

Git uses a working directory that contains the files which you are
working on. These get converted into a snapshot when you commit, and
these commits form the history of your project. These snapshots along
with everything else that git needs to work are stored in the git
directory, often called ".git".

When you zip the files you are working on you are creating a manual
snapshot of your project. If you zip the git directory you are
compressing the entire git repository and this has enough information
to recreate your entire history. If you zip both of them you get your
history as well as any changes that have not been committed yet. When
a server holds a copy of your repository it will not include a working
directory at all, but instead just the git directory. In this
situation the git directory will often be called the same name as the
project.

The zip from GitHub does indeed only contain the working directory,
and so doesn't include the history.

Hope that helps!

Regards,

Andrew Ardill
Regards,

Andrew Ardill

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Sitaram Chamarty @ 2012-11-26  5:10 UTC (permalink / raw)
  To: esr; +Cc: Krzysztof Mazur, git
In-Reply-To: <20121125224728.GD6937@thyrsus.com>

On Mon, Nov 26, 2012 at 4:17 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Krzysztof Mazur <krzysiek@podlesie.net>:
>> What about embedded systems? git is also useful there. C and shell is
>> everywhere, python is not.
>
> Supposing this is true (and I question it with regard to shell) if you
> tell me how you live without gitk and the Perl pieces I'll play that
> right back at you as your answer.

gitk is unlikely to be used on an embedded system, the perl pieces more so.

I have never understood why people complain about readability in perl.
 Just because it uses the ascii charset a bit more?  You expect a
mathematician or indeed any scientist to use special symbols to mean
special things, why not programmers?

Perhaps people should be forced to use COBOL for a few years (like I
did, a long while ago) to appreciate brevity.

^ 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