* [PATCH v4 04/12] t/t3511: add some tests of 'cherry-pick -s' functionality
From: Brandon Casey @ 2013-02-12 10:17 UTC (permalink / raw)
To: git; +Cc: gitster, pclouds, jrnieder, Brandon Casey, Brandon Casey
In-Reply-To: <1360664260-11803-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>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.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..2a040b7
--- /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
+
+The signed-off-by string should begin with the words Signed-off-by followed
+by a colon and space, and then the signers name and email address. e.g.
+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.1.3.579.gd9af3b6
^ permalink raw reply related
* [PATCH v4 05/12] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
From: Brandon Casey @ 2013-02-12 10:17 UTC (permalink / raw)
To: git; +Cc: gitster, pclouds, jrnieder, Brandon Casey, Brandon Casey
In-Reply-To: <1360664260-11803-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 Mmitter <committer@example.com>
instead of this:
Signed-off-by: A U Thor <author@example.com>
(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
Signed-off-by: C O Mmitter <committer@example.com>
[with improvements from Jonathan Nieder]
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
---
sequencer.c | 51 ++++++++++++++++++++++++++++++++------------
t/t3511-cherry-pick-x.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 92 insertions(+), 14 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index aa2cb8e..93495b0 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)
{
@@ -496,7 +497,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");
}
@@ -1021,16 +1022,44 @@ 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)
{
- char ch, prev;
- int i, j, k;
+ int i;
+
+ for (i = 0; i < len; i++) {
+ int ch = buf[i];
+ if (ch == ':')
+ return 1;
+ if (!isalnum(ch) && ch != '-')
+ break;
+ }
+
+ return 0;
+}
+
+static int is_cherry_picked_from_line(const char *buf, int len)
+{
+ /*
+ * We only care that it looks roughly like (cherry picked from ...)
+ */
+ return len > strlen(cherry_picked_prefix) + 1 &&
+ !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')';
+}
+
+static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
+{
+ char prev;
+ int i, k;
int len = sb->len - ignore_footer;
const char *buf = sb->buf;
+ /* footer must end with newline */
+ if (!len || buf[len - 1] != '\n')
+ return 0;
+
prev = '\0';
for (i = len - 1; i > 0; i--) {
- ch = buf[i];
+ char ch = buf[i];
if (prev == '\n' && ch == '\n') /* paragraph break */
break;
prev = ch;
@@ -1045,15 +1074,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 - 1) &&
+ !is_cherry_picked_from_line(buf + i, k - i - 1))
return 0;
- }
}
return 1;
}
@@ -1070,7 +1093,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 2a040b7..73da182 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.1.3.579.gd9af3b6
^ permalink raw reply related
* [PATCH v4 06/12] sequencer.c: require a conforming footer to be preceded by a blank line
From: Brandon Casey @ 2013-02-12 10:17 UTC (permalink / raw)
To: git; +Cc: gitster, pclouds, jrnieder, Brandon Casey
In-Reply-To: <1360664260-11803-1-git-send-email-drafnel@gmail.com>
Currently, append_signoff() performs a search for the last line of the
commit buffer by searching back from the end until it hits a newline. If
it reaches the beginning of the buffer without finding a newline, that
means either the commit message was empty, or there was only one line in it.
In this case, append_signoff will skip the call to has_conforming_footer
since it already knows that it is necessary to append a newline before
appending the sob.
Let's perform this function inside of has_conforming_footer where it
appropriately belongs and generalize it so that we require that the
footer paragraph be an actual distinct paragraph separated by a blank
line.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
sequencer.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 93495b0..178e84b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1065,6 +1065,10 @@ static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
prev = ch;
}
+ /* require at least one blank line */
+ if (prev != '\n' || buf[i] != '\n')
+ return 0;
+
/* advance to start of last paragraph */
while (i < len - 1 && buf[i] == '\n')
i++;
@@ -1093,7 +1097,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 || !has_conforming_footer(msgbuf, ignore_footer))
+ if (!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);
}
--
1.8.1.3.579.gd9af3b6
^ permalink raw reply related
* [PATCH v4 07/12] sequencer.c: always separate "(cherry picked from" from commit body
From: Brandon Casey @ 2013-02-12 10:17 UTC (permalink / raw)
To: git; +Cc: gitster, pclouds, jrnieder, Brandon Casey, Brandon Casey
In-Reply-To: <1360664260-11803-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>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
---
sequencer.c | 128 ++++++++++++++++++++++++-----------------------
t/t3511-cherry-pick-x.sh | 53 ++++++++++++++++++++
2 files changed, 118 insertions(+), 63 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 178e84b..249c4a0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -20,6 +20,69 @@
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 == ':')
+ return 1;
+ if (!isalnum(ch) && ch != '-')
+ break;
+ }
+
+ return 0;
+}
+
+static int is_cherry_picked_from_line(const char *buf, int len)
+{
+ /*
+ * We only care that it looks roughly like (cherry picked from ...)
+ */
+ return len > strlen(cherry_picked_prefix) + 1 &&
+ !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')';
+}
+
+static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
+{
+ char prev;
+ int i, k;
+ int len = sb->len - ignore_footer;
+ const char *buf = sb->buf;
+
+ /* footer must end with newline */
+ if (!len || buf[len - 1] != '\n')
+ return 0;
+
+ prev = '\0';
+ for (i = len - 1; i > 0; i--) {
+ char ch = buf[i];
+ if (prev == '\n' && ch == '\n') /* paragraph break */
+ break;
+ prev = ch;
+ }
+
+ /* require at least one blank line */
+ if (prev != '\n' || buf[i] != '\n')
+ return 0;
+
+ /* advance to start of last paragraph */
+ 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 - 1) &&
+ !is_cherry_picked_from_line(buf + i, k - i - 1))
+ return 0;
+ }
+ return 1;
+}
+
static void remove_sequencer_state(void)
{
struct strbuf seq_dir = STRBUF_INIT;
@@ -497,6 +560,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");
@@ -1022,69 +1087,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 == ':')
- return 1;
- if (!isalnum(ch) && ch != '-')
- break;
- }
-
- return 0;
-}
-
-static int is_cherry_picked_from_line(const char *buf, int len)
-{
- /*
- * We only care that it looks roughly like (cherry picked from ...)
- */
- return len > strlen(cherry_picked_prefix) + 1 &&
- !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')';
-}
-
-static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
-{
- char prev;
- int i, k;
- int len = sb->len - ignore_footer;
- const char *buf = sb->buf;
-
- /* footer must end with newline */
- if (!len || buf[len - 1] != '\n')
- return 0;
-
- prev = '\0';
- for (i = len - 1; i > 0; i--) {
- char ch = buf[i];
- if (prev == '\n' && ch == '\n') /* paragraph break */
- break;
- prev = ch;
- }
-
- /* require at least one blank line */
- if (prev != '\n' || buf[i] != '\n')
- return 0;
-
- /* advance to start of last paragraph */
- 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 - 1) &&
- !is_cherry_picked_from_line(buf + i, k - i - 1))
- 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 73da182..a845e45 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.1.3.579.gd9af3b6
^ permalink raw reply related
* [PATCH v4 08/12] sequencer.c: teach append_signoff how to detect duplicate s-o-b
From: Brandon Casey @ 2013-02-12 10:17 UTC (permalink / raw)
To: git; +Cc: gitster, pclouds, jrnieder, Brandon Casey, Brandon Casey
In-Reply-To: <1360664260-11803-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 | 59 ++++++++++++++++++++++++++++++++++++------------
sequencer.h | 4 +++-
t/t3511-cherry-pick-x.sh | 2 +-
4 files changed, 50 insertions(+), 17 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 1a0e5f1..94c96b7 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 249c4a0..3364faa 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -44,12 +44,20 @@ static int is_cherry_picked_from_line(const char *buf, int len)
!prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')';
}
-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)
{
char prev;
int i, k;
int len = sb->len - ignore_footer;
const char *buf = sb->buf;
+ int found_sob = 0;
/* footer must end with newline */
if (!len || buf[len - 1] != '\n')
@@ -72,14 +80,25 @@ 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 - 1) &&
- !is_cherry_picked_from_line(buf + i, k - i - 1))
+ found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1);
+ if (found_rfc2822 && sob &&
+ !strncmp(buf + i, sob->buf, sob->len))
+ found_sob = k;
+
+ if (!(found_rfc2822 ||
+ is_cherry_picked_from_line(buf + i, k - i - 1)))
return 0;
}
+ if (found_sob == i)
+ return 3;
+ if (found_sob)
+ return 2;
return 1;
}
@@ -301,7 +320,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;
@@ -560,7 +579,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));
@@ -1087,21 +1106,33 @@ 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, unsigned flag)
{
+ unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
struct strbuf sob = STRBUF_INIT;
- int i;
+ int has_footer;
strbuf_addstr(&sob, sign_off_header);
strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"),
getenv("GIT_COMMITTER_EMAIL")));
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 (!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 the whole message buffer is equal to the sob, pretend that we
+ * found a conforming footer with a matching sob
+ */
+ if (msgbuf->len - ignore_footer == sob.len &&
+ !strncmp(msgbuf->buf, sob.buf, sob.len))
+ has_footer = 3;
+ else
+ has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
+
+ if (!has_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..1fc22dc 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -6,6 +6,8 @@
#define SEQ_TODO_FILE "sequencer/todo"
#define SEQ_OPTS_FILE "sequencer/opts"
+#define APPEND_SIGNOFF_DEDUP (1u << 0)
+
enum replay_action {
REPLAY_REVERT,
REPLAY_PICK
@@ -48,6 +50,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, unsigned flag);
#endif
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index a845e45..f977279 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.1.3.579.gd9af3b6
^ permalink raw reply related
* [PATCH v4 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline
From: Brandon Casey @ 2013-02-12 10:17 UTC (permalink / raw)
To: git; +Cc: gitster, pclouds, jrnieder, Brandon Casey, Brandon Casey
In-Reply-To: <1360664260-11803-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 refrain from adding an
additional one if one already exists. Or, add an additional line if one
is needed to make sure the new footer is separated from the message body
by a blank line.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
sequencer.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 3364faa..3c63e3a 100644
I dropped the mention of format-patch in the commit message. This
implementation is less about making format-patch work and more about
cleaning up a strangely formatted commit message.
-Brandon
--- a/sequencer.c
+++ b/sequencer.c
@@ -1110,6 +1110,7 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
{
unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
struct strbuf sob = STRBUF_INIT;
+ const char *append_newlines = NULL;
int has_footer;
strbuf_addstr(&sob, sign_off_header);
@@ -1127,8 +1128,17 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
else
has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
- if (!has_footer)
- strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
+ if (!has_footer) {
+ size_t len = msgbuf->len - ignore_footer;
+ if (len && msgbuf->buf[len - 1] != '\n')
+ append_newlines = "\n\n";
+ else if (len > 1 && msgbuf->buf[len - 2] != '\n')
+ append_newlines = "\n";
+ }
+
+ if (append_newlines)
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
+ append_newlines, strlen(append_newlines));
if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
--
1.8.1.3.579.gd9af3b6
^ permalink raw reply
* [PATCH v4 11/12] format-patch: update append_signoff prototype
From: Brandon Casey @ 2013-02-12 10:17 UTC (permalink / raw)
To: git; +Cc: gitster, pclouds, jrnieder, Brandon Casey
In-Reply-To: <1360664260-11803-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>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/log.c | 13 +------------
log-tree.c | 17 +++++++++++++----
revision.h | 2 +-
3 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..59de484 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1086,7 +1086,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;
@@ -1193,16 +1192,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
rev.subject_prefix = strbuf_detach(&sprefix, NULL);
}
- 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');
@@ -1393,7 +1382,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 5dc45c4..ac1cd68 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -10,6 +10,8 @@
#include "color.h"
#include "gpg-interface.h"
+#define APPEND_SIGNOFF_DEDUP (1u <<0)
+
struct decoration name_decoration = { "object names" };
enum decoration_type {
@@ -253,9 +255,12 @@ 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 ignore_footer, unsigned flag)
{
+ unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
static const char signed_off_by[] = "Signed-off-by: ";
+ char *signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
+ getenv("GIT_COMMITTER_EMAIL")));
size_t signoff_len = strlen(signoff);
int has_signoff = 0;
char *cp;
@@ -275,6 +280,7 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
if (!isspace(cp[signoff_len]))
continue;
/* we already have him */
+ free(signoff);
return;
}
@@ -287,6 +293,7 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
strbuf_addstr(sb, signed_off_by);
strbuf_add(sb, signoff, signoff_len);
strbuf_addch(sb, '\n');
+ free(signoff);
}
static unsigned int digits_in_number(unsigned int number)
@@ -672,8 +679,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;
@@ -686,7 +695,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, APPEND_SIGNOFF_DEDUP);
if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
ctx.notes_message && *ctx.notes_message) {
diff --git a/revision.h b/revision.h
index 5da09ee..01bd2b7 100644
--- a/revision.h
+++ b/revision.h
@@ -138,7 +138,7 @@ struct rev_info {
int reroll_count;
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.1.3.579.gd9af3b6
^ permalink raw reply related
* [PATCH v4 10/12] t4014: more tests about appending s-o-b lines
From: Brandon Casey @ 2013-02-12 10:17 UTC (permalink / raw)
To: git; +Cc: gitster, pclouds, jrnieder, Brandon Casey
In-Reply-To: <1360664260-11803-1-git-send-email-drafnel@gmail.com>
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
[bc: Squash the tests from Duy's original unify-appending-sob series.
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.
Add two additional tests:
1. failure to detect non-conforming elements in the footer when last
line matches committer's s-o-b.
2. ensure various s-o-b -like elements in the footer are handled 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>
---
t/t4014-format-patch.sh | 241 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 241 insertions(+)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 7fa3647..a415b89 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1021,4 +1021,245 @@ test_expect_success 'cover letter using branch description (6)' '
grep hello actual >/dev/null
'
+append_signoff()
+{
+ C=$(git commit-tree HEAD^^{tree} -p HEAD) &&
+ git format-patch --stdout --signoff $C^..$C >append_signoff.patch &&
+ sed -n -e "1,/^---$/p" append_signoff.patch |
+ egrep -n "^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' '
+ printf 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_failure '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_failure '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_failure '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
+
+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_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_success 'signoff: footer begins with non-signoff without @ sign' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Reviewed-id: Noone
+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:
+14:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
test_done
--
1.8.1.3.579.gd9af3b6
^ permalink raw reply related
* [PATCH v4 12/12] Unify appending signoff in format-patch, commit and sequencer
From: Brandon Casey @ 2013-02-12 10:17 UTC (permalink / raw)
To: git; +Cc: gitster, pclouds, jrnieder, Brandon Casey, Brandon Casey
In-Reply-To: <1360664260-11803-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 | 91 +------------------------------------------------
t/t4014-format-patch.sh | 31 ++++++++++++++---
2 files changed, 27 insertions(+), 95 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index ac1cd68..c9d9a37 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -9,8 +9,7 @@
#include "string-list.h"
#include "color.h"
#include "gpg-interface.h"
-
-#define APPEND_SIGNOFF_DEDUP (1u <<0)
+#include "sequencer.h"
struct decoration name_decoration = { "object names" };
@@ -208,94 +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 ignore_footer, unsigned flag)
-{
- unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
- static const char signed_off_by[] = "Signed-off-by: ";
- char *signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
- getenv("GIT_COMMITTER_EMAIL")));
- size_t signoff_len = strlen(signoff);
- int has_signoff = 0;
- char *cp;
-
- cp = sb->buf;
-
- /* First see if we already have the sign-off by the signer */
- while ((cp = strstr(cp, signed_off_by))) {
-
- has_signoff = 1;
-
- cp += strlen(signed_off_by);
- 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 */
- free(signoff);
- return;
- }
-
- if (!has_signoff)
- has_signoff = detect_any_signoff(sb->buf, sb->len);
-
- if (!has_signoff)
- strbuf_addch(sb, '\n');
-
- strbuf_addstr(sb, signed_off_by);
- 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;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index a415b89..97fde9e 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1103,7 +1103,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
@@ -1119,7 +1140,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'signoff: not really a signoff' '
+test_expect_success 'signoff: not really a signoff' '
append_signoff <<\EOF >actual &&
subject
@@ -1135,7 +1156,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'signoff: not really a signoff (2)' '
+test_expect_success 'signoff: not really a signoff (2)' '
append_signoff <<\EOF >actual &&
subject
@@ -1152,7 +1173,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'signoff: valid S-o-b paragraph in the middle' '
+test_expect_success 'signoff: valid S-o-b paragraph in the middle' '
append_signoff <<\EOF >actual &&
subject
@@ -1220,7 +1241,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
--
1.8.1.3.579.gd9af3b6
^ permalink raw reply related
* [PATCH/FYI v4 13/12] fixup! t/t3511: add some tests of 'cherry-pick -s' functionality
From: Brandon Casey @ 2013-02-12 10:17 UTC (permalink / raw)
To: git; +Cc: gitster, pclouds, jrnieder, Brandon Casey
In-Reply-To: <1360664260-11803-1-git-send-email-drafnel@gmail.com>
---
This test tests the behavior of 'cherry-pick -s' of a commit with an empty
commit message.
I created the test when I noticed during my series that cherry-pick was
adding a sob twice when a commit with an empty commit message was
cherry-picked.
I'm not sure we should apply this though. I'm leaning towards saying that
the 'cherry-pick -s' behavior with respect to a commit with an empty message
body should be undefined. If we want it to be undefined then we probably
shouldn't introduce a test which would have the effect of defining it.
Junio, if you think we should apply it, it's prepared as a fixup commit and
should autosquash easily.
-Brandon
t/t3505-cherry-pick-empty.sh | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/t/t3505-cherry-pick-empty.sh b/t/t3505-cherry-pick-empty.sh
index a0c6e30..da4c60e 100755
--- a/t/t3505-cherry-pick-empty.sh
+++ b/t/t3505-cherry-pick-empty.sh
@@ -58,6 +58,16 @@ test_expect_success 'cherry-pick a commit with an empty message with --allow-emp
git cherry-pick --allow-empty-message empty-branch
'
+test_expect_success 'cherry-pick a commit with an empty message with --allow-empty-message and -s' '
+ git reset --hard HEAD^ &&
+ git cherry-pick --allow-empty-message -s empty-branch &&
+ { git show --pretty=format:%B -s empty-branch &&
+ printf "Signed-off-by: %s <%s>\n" "$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL"
+ } >expect &&
+ git show --pretty=format:%B -s HEAD >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'cherry pick an empty non-ff commit without --allow-empty' '
git checkout master &&
echo fourth >>file2 &&
--
1.8.1.1.252.gdb33759
^ permalink raw reply related
* Re: A good Git technique for referring back to original files
From: MikeW @ 2013-02-12 10:19 UTC (permalink / raw)
To: git
In-Reply-To: <vpq1ucl9agt.fsf@grenoble-inp.fr>
Matthieu Moy <Matthieu.Moy <at> grenoble-inp.fr> writes:
>
> MikeW <mw_phil <at> yahoo.co.uk> writes:
>
> > Since git is so good at tracking file content, I wondered whether
there was any
> > technique using git that would simplify the back-referencing task.
>
> I'm not sure I understand the question, but if you want to add meta-data
> to Git commits (e.g. "this Git commit is revision 42 in CVS repository
> foo"), then have a look at git-notes. It won't give you directly
> "reference to other VCS", but at least can be used as a storage
> mechanism to store these references.
>
Thanks for the reply.
In my work environment both the SDK and the original files are available
(in an enclosing directory).
--SDK_content
|
SDK_subproj1-- ...
| |
| content
|
SDK_subproj2- ...
| |
| content
|
SDK_subprojN- ...
| |
| content
|
Working_SDK ... (under git, baseline generated from subproj1..N)
|
content derived from subproj1..N
What I had in mind was something I could run over, say, SDK_content
(alternatively, from within Working_SDK, referring back to SDK_content)
which would note the changed files in Working_SDK and locate the
original files in SDK_subproj1..N letting me merge the changes back.
^ permalink raw reply
* [PATCH v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline
From: Brandon Casey @ 2013-02-12 10:33 UTC (permalink / raw)
To: git; +Cc: gitster, pclouds, jrnieder, Brandon Casey, Brandon Casey
In-Reply-To: <1360664260-11803-10-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 refrain from adding an
additional one if one already exists. Or, add an additional line if one
is needed to make sure the new footer is separated from the message body
by a blank line.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
A slight tweak. And I promise, no more are coming.
-Brandon
sequencer.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 3364faa..084573b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1127,8 +1127,19 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
else
has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
- if (!has_footer)
- strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
+ if (!has_footer) {
+ const char *append_newlines = NULL;
+ size_t len = msgbuf->len - ignore_footer;
+
+ if (len && msgbuf->buf[len - 1] != '\n')
+ append_newlines = "\n\n";
+ else if (len > 1 && msgbuf->buf[len - 2] != '\n')
+ append_newlines = "\n";
+
+ if (append_newlines)
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
+ append_newlines, strlen(append_newlines));
+ }
if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
--
1.8.1.1.252.gdb33759
^ permalink raw reply related
* Re: Improve 'git help' with basic user guide linkss
From: Philip Oakley @ 2013-02-12 11:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <27C7EA38DDD345F994F01E5B8FB6D278@PhilipOakley>
From: "Philip Oakley" <philipoakley@iee.org>
Sent: Friday, February 08, 2013 11:16 PM
> From: "Junio C Hamano" <gitster@pobox.com>
> Sent: Friday, February 08, 2013 10:54 PM
>> "Philip Oakley" <philipoakley@iee.org> writes:
>>
>>> My initial https://github.com/PhilipOakley/git/commit/e6217d simply
>>> updates
>>> - N_("See 'git help <command>' for more information on a specific
>>> command.");
>>> + N_("See 'git help <command>' for more information on a specific
>>> command.\n"
>>> + "Or 'git help <guide>', such as 'tutorial' for an introduction
>>> to Git.");
>>> as a starter for the new users.
>>
>> Yeah, that would be a good change to make to "git help<RETURN>"
>> output.
>
> I'll sort some patches early next week (the weekend's committed
> elsewhere)
>
>>
>>> My view is that help --all (-a) is essentially incomplete as it
>>> currently doesn't provide all the help.
>>
>> It has always been about "tell me all subcommands", not about "give
>> me all the help you could give me". You are not adding a "help"
>> subcommand to a system you wrote last week. Changing the semantics
>> this late feels, eh, too late.
>
> OK, I'll limit the the follow-ons to just an extra --guides option
> (probably just a list of the common guides initially), and
> leave --all(-a) for just the commands.
>
The Git man page includes the different command types listed by category
(Main porcelain, Ancillary {manipulators, interrogators}, Interacting
with others, etc.).
Obviously (?) this is generated from the command-list.txt file, though I
don't see a shell script that would generate the
'cmds-mainporcelain.txt' (etc.) files
(//github.com/gitster/git-htmldocs). They are also part of the msysgit
install.
Where should I be looking to see how they are generated?
Philip
^ permalink raw reply
* Re: Pushing a git repository to a new server
From: Michael J Gruber @ 2013-02-12 11:28 UTC (permalink / raw)
To: Jeff King; +Cc: Ethan Reesor, Konstantin Khomoutov, git
In-Reply-To: <20130211162714.GB16402@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 11.02.2013 17:27:
> On Mon, Feb 11, 2013 at 02:57:51AM -0500, Ethan Reesor wrote:
>
>> On Mon, Feb 11, 2013 at 2:50 AM, Konstantin Khomoutov
>> <kostix+git@007spb.ru> wrote:
>>> What's wrong with
>>> $ ssh myuser@remotehost 'mkdir /path/to/MyRepo.git; cd $_; git init --bare'
>>> $ git push --all git@remotehost:MyOtherRepo.git
>>> ?
>>
>> Nothing, I just wanted to make myself a command to do that for me.
>
> We talked about this a long time ago. One problem is that it's
> inherently unportable, as the procedure to make a repo is potentially
> different on every server (and certainly that is the case between a
> regular user running stock git and something like GitHub or Google Code;
> I imagine even gitolite has some special procedures for creating repos,
> too).
>
> One proposal made in the previous discussion was to define a microformat
> for repository administration commands. So that you could connect and
> say "git admin-create-repo /path/to/MyRepo.git", and the server-provided
> admin-create-repo command would take care of the details. Then stock git
> could forward it to "git init --bare", GitHub could do the same and
> create the necessary database records, etc.
>
> And once that standardized method was in place, it would be easy to add
> a "--create" option to "git push" to request an "admin-create-repo"
> before pushing.
>
> I still think that's a reasonable way forward, but nobody was interested
> enough to start writing code for it.
>
> -Peff
>
I'm not sure providers like GitHub would fancy an interface which allows
the programmatic creation of repos (giving a new meaning to "fork
bomb"). But I bet you know better ;-)
An alternative would be to teach git (the client) about repo types and
how to create them. After all, a repo URL "ssh://host/path" gives a
clear indication that "ssh host git init path" will create a repo. I'm
wondering whether it's more likely to convince providers (the server
side) or more is gained by covering the simpler cases client-side (our
side).
Michael
^ permalink raw reply
* Re: Improve 'git help' with basic user guide linkss
From: John Keeping @ 2013-02-12 11:37 UTC (permalink / raw)
To: Philip Oakley; +Cc: Junio C Hamano, Git List
In-Reply-To: <2DFA2C4F62AE484681753EBC3774D222@PhilipOakley>
On Tue, Feb 12, 2013 at 11:11:17AM -0000, Philip Oakley wrote:
> Obviously (?) this is generated from the command-list.txt file, though I
> don't see a shell script that would generate the
> 'cmds-mainporcelain.txt' (etc.) files
> (//github.com/gitster/git-htmldocs). They are also part of the msysgit
> install.
>
> Where should I be looking to see how they are generated?
Documentation/cmd-list.perl
If you look in Documentation/Makefile, cmds-mainporcelain.txt is a
member of cmds_txt, which depends on cmd-list.made, which is a stamp
file created after invoking cmd-list.perl.
John
^ permalink raw reply
* Re: Improve 'git help' with basic user guide linkss
From: Philip Oakley @ 2013-02-12 12:00 UTC (permalink / raw)
To: John Keeping; +Cc: Junio C Hamano, Git List
In-Reply-To: <20130212113731.GA13501@farnsworth.metanate.com>
From: "John Keeping" <john@keeping.me.uk>
Sent: Tuesday, February 12, 2013 11:37 AM
> On Tue, Feb 12, 2013 at 11:11:17AM -0000, Philip Oakley wrote:
>> Obviously (?) this is generated from the command-list.txt file,
>> though I
>> don't see a shell script that would generate the
>> 'cmds-mainporcelain.txt' (etc.) files
>> (//github.com/gitster/git-htmldocs). They are also part of the
>> msysgit
>> install.
>>
>> Where should I be looking to see how they are generated?
>
> Documentation/cmd-list.perl
>
> If you look in Documentation/Makefile, cmds-mainporcelain.txt is a
> member of cmds_txt, which depends on cmd-list.made, which is a stamp
> file created after invoking cmd-list.perl.
>
>
> John
Many thanks - some easy reading while unwell ;-)
^ permalink raw reply
* [PATCH] completion: support 'git config --local'
From: Matthieu Moy @ 2013-02-12 12:20 UTC (permalink / raw)
To: git, gitster; +Cc: Dasa Paddock, Matthieu Moy
In-Reply-To: <85E0E68E8961D64E9200C534AC5E1B240A443EDC@RED-INF-EXMB-P1.esri.com>
This needs to be done in two places: __git_config_get_set_variables to
allow clever completion of "git config --local --get foo<tab>", and
_git_config to allow "git config --loc<tab>" to complete to --local.
While we're there, change the order of options in the code to match
git-config.txt.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
> I think this line should include --local:
>
> https://github.com/git/git/blob/next/contrib/completion/git-completion.bash#L1782
> "--global|--system|--file=*)"
>
> This would help for:
> git config -l --local
Yes, but not only ;-)
contrib/completion/git-completion.bash | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5770b6f..2e1ad67 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1570,7 +1570,7 @@ __git_config_get_set_variables ()
while [ $c -gt 1 ]; do
word="${words[c]}"
case "$word" in
- --global|--system|--file=*)
+ --system|--global|--local|--file=*)
config_file="$word"
break
;;
@@ -1676,7 +1676,7 @@ _git_config ()
case "$cur" in
--*)
__gitcomp "
- --global --system --file=
+ --system --global --local --file=
--list --replace-all
--get --get-all --get-regexp
--add --unset --unset-all
--
1.8.1.2.548.g956380a.dirty
^ permalink raw reply related
* [PATCHv4 4/6] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe
From: Michal Nazarewicz @ 2013-02-12 14:02 UTC (permalink / raw)
To: peff, gitster; +Cc: git
In-Reply-To: <cover.1360677646.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
The command_close_bidi_pipe() function will insist on closing both
input and output pipes returned by command_bidi_pipe(). With this
change it is possible to close one of the pipes in advance and
pass undef as an argument.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 6bc9a3c..d6e6c9e 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -426,12 +426,25 @@ Note that you should not rely on whatever actually is in C<CTX>;
currently it is simply the command name but in future the context might
have more complicated structure.
+C<PIPE_IN> and C<PIPE_OUT> may be C<undef> if they have been closed prior to
+calling this function. This may be useful in a query-response type of
+commands where caller first writes a query and later reads response, eg:
+
+ my ($pid, $in, $out, $ctx) = $r->command_bidi_pipe('cat-file --batch-check');
+ print $out "000000000\n";
+ close $out;
+ while (<$in>) { ... }
+ $r->command_close_bidi_pipe($pid, $in, undef, $ctx);
+
+This idiom may prevent potential dead locks caused by data sent to the output
+pipe not being flushed and thus not reaching the executed command.
+
=cut
sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
- _cmd_close($ctx, $in, $out);
+ _cmd_close($ctx, grep defined, $in, $out);
waitpid $pid, 0;
if ($? >> 8) {
throw Git::Error::Command($ctx, $? >>8);
--
1.8.1.3.572.g32bae1f
^ permalink raw reply related
* [PATCHv4 6/6] git-send-email: use git credential to obtain password
From: Michal Nazarewicz @ 2013-02-12 14:02 UTC (permalink / raw)
To: peff, gitster; +Cc: git
In-Reply-To: <cover.1360677646.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
If smtp_user is provided but smtp_pass is not, instead of
prompting for password, make git-send-email use git
credential command instead.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
Documentation/git-send-email.txt | 4 +--
git-send-email.perl | 59 +++++++++++++++++++++++-----------------
2 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 44a1f7c..0cffef8 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -164,8 +164,8 @@ Sending
Furthermore, passwords need not be specified in configuration files
or on the command line. If a username has been specified (with
'--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass' or 'sendemail.smtppass'), then
+a password is obtained using 'git-credential'.
--smtp-server=<host>::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..76bbfc3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,39 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
}
+# Returns 1 if authentication succeeded or was not necessary
+# (smtp_user was not specified), and 0 otherwise.
+
+sub smtp_auth_maybe {
+ if (!defined $smtp_authuser || $auth) {
+ return 1;
+ }
+
+ # Workaround AUTH PLAIN/LOGIN interaction defect
+ # with Authen::SASL::Cyrus
+ eval {
+ require Authen::SASL;
+ Authen::SASL->import(qw(Perl));
+ };
+
+ # TODO: Authentication may fail not because credentials were
+ # invalid but due to other reasons, in which we should not
+ # reject credentials.
+ $auth = Git::credential({
+ 'protocol' => 'smtp',
+ 'host' => join(':', $smtp_server, $smtp_server_port),
+ 'username' => $smtp_authuser,
+ # if there's no password, "git credential fill" will
+ # give us one, otherwise it'll just pass this one.
+ 'password' => $smtp_authpass
+ }, sub {
+ my $cred = shift;
+ return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
+ });
+
+ return $auth;
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
@@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
defined $smtp_server_port ? " port=$smtp_server_port" : "";
}
- if (defined $smtp_authuser) {
- # Workaround AUTH PLAIN/LOGIN interaction defect
- # with Authen::SASL::Cyrus
- eval {
- require Authen::SASL;
- Authen::SASL->import(qw(Perl));
- };
-
- if (!defined $smtp_authpass) {
-
- system "stty -echo";
-
- do {
- print "Password: ";
- $_ = <STDIN>;
- print "\n";
- } while (!defined $_);
-
- chomp($smtp_authpass = $_);
-
- system "stty echo";
- }
-
- $auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
- }
+ smtp_auth_maybe or die $smtp->message;
$smtp->mail( $raw_from ) or die $smtp->message;
$smtp->to( @recipients ) or die $smtp->message;
--
1.8.1.3.572.g32bae1f
^ permalink raw reply related
* [PATCHv4 0/6] git-credential support in git-send-email
From: Michal Nazarewicz @ 2013-02-12 14:02 UTC (permalink / raw)
To: peff, gitster; +Cc: git
From: Michal Nazarewicz <mina86@mina86.com>
Besids git-credential support in git-send-email, there are some other
minor improvements to Git.pm in this patchset. Patch 3/6 is new
compared to the previous patchset.
Michal Nazarewicz (6):
Git.pm: allow command_close_bidi_pipe to be called as method
Git.pm: fix example in command_close_bidi_pipe documentation
Git.pm: refactor command_close_bidi_pipe to use _cmd_close
Git.pm: allow pipes to be closed prior to calling
command_close_bidi_pipe
Git.pm: add interface for git credential command
git-send-email: use git credential to obtain password
Documentation/git-send-email.txt | 4 +-
git-send-email.perl | 59 +++++++-----
perl/Git.pm | 198 ++++++++++++++++++++++++++++++++++-----
3 files changed, 213 insertions(+), 48 deletions(-)
--
1.8.1.3.572.g32bae1f
^ permalink raw reply
* [PATCHv4 1/6] Git.pm: allow command_close_bidi_pipe to be called as method
From: Michal Nazarewicz @ 2013-02-12 14:02 UTC (permalink / raw)
To: peff, gitster; +Cc: git
In-Reply-To: <cover.1360677646.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
The documentation of command_close_bidi_pipe() claims that it can
be called as a method, but it does not check whether the first
argument is $self or not assuming the latter. Using _maybe_self()
fixes this.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..bbb753a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -430,7 +430,7 @@ have more complicated structure.
sub command_close_bidi_pipe {
local $?;
- my ($pid, $in, $out, $ctx) = @_;
+ my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
unless (close $fh) {
if ($!) {
--
1.8.1.3.572.g32bae1f
^ permalink raw reply related
* [PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close
From: Michal Nazarewicz @ 2013-02-12 14:02 UTC (permalink / raw)
To: peff, gitster; +Cc: git
In-Reply-To: <cover.1360677646.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
The body of the loop in command_close_bidi_pipe function is identical to
what _cmd_close function does so instead of duplicating, refactor change
_cmd_close so that it accepts list of file handlers to be closed, which
makes it usable with command_close_bidi_pipe.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 11f310a..6bc9a3c 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -267,13 +267,13 @@ sub command {
if (not defined wantarray) {
# Nothing to pepper the possible exception with.
- _cmd_close($fh, $ctx);
+ _cmd_close($ctx, $fh);
} elsif (not wantarray) {
local $/;
my $text = <$fh>;
try {
- _cmd_close($fh, $ctx);
+ _cmd_close($ctx, $fh);
} catch Git::Error::Command with {
# Pepper with the output:
my $E = shift;
@@ -286,7 +286,7 @@ sub command {
my @lines = <$fh>;
defined and chomp for @lines;
try {
- _cmd_close($fh, $ctx);
+ _cmd_close($ctx, $fh);
} catch Git::Error::Command with {
my $E = shift;
$E->{'-outputref'} = \@lines;
@@ -313,7 +313,7 @@ sub command_oneline {
my $line = <$fh>;
defined $line and chomp $line;
try {
- _cmd_close($fh, $ctx);
+ _cmd_close($ctx, $fh);
} catch Git::Error::Command with {
# Pepper with the output:
my $E = shift;
@@ -381,7 +381,7 @@ have more complicated structure.
sub command_close_pipe {
my ($self, $fh, $ctx) = _maybe_self(@_);
$ctx ||= '<unknown>';
- _cmd_close($fh, $ctx);
+ _cmd_close($ctx, $fh);
}
=item command_bidi_pipe ( COMMAND [, ARGUMENTS... ] )
@@ -431,18 +431,8 @@ have more complicated structure.
sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
- foreach my $fh ($in, $out) {
- unless (close $fh) {
- if ($!) {
- carp "error closing pipe: $!";
- } elsif ($? >> 8) {
- throw Git::Error::Command($ctx, $? >>8);
- }
- }
- }
-
+ _cmd_close($ctx, $in, $out);
waitpid $pid, 0;
-
if ($? >> 8) {
throw Git::Error::Command($ctx, $? >>8);
}
@@ -1355,9 +1345,11 @@ sub _execv_git_cmd { exec('git', @_); }
# Close pipe to a subprocess.
sub _cmd_close {
- my ($fh, $ctx) = @_;
- if (not close $fh) {
- if ($!) {
+ my $ctx = shift @_;
+ foreach my $fh (@_) {
+ if (close $fh) {
+ # nop;
+ } elsif ($!) {
# It's just close, no point in fatalities
carp "error closing pipe: $!";
} elsif ($? >> 8) {
--
1.8.1.3.572.g32bae1f
^ permalink raw reply related
* [PATCHv4 5/6] Git.pm: add interface for git credential command
From: Michal Nazarewicz @ 2013-02-12 14:02 UTC (permalink / raw)
To: peff, gitster; +Cc: git
In-Reply-To: <cover.1360677646.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
Add a credential() function which is an interface to the git
credential command. The code is heavily based on credential_*
functions in <contrib/mw-to-git/git-remote-mediawiki>.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 151 insertions(+)
diff --git a/perl/Git.pm b/perl/Git.pm
index d6e6c9e..a24458c 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -59,6 +59,7 @@ require Exporter;
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
remote_refs prompt
+ credential credential_read credential_write
temp_acquire temp_release temp_reset temp_path);
@@ -1003,6 +1004,156 @@ sub _close_cat_blob {
}
+=item credential_read( FILEHANDLE )
+
+Reads credential key-value pairs from C<FILEHANDLE>. Reading stops at EOF or
+when an empty line is encountered. Each line must be of the form C<key=value>
+with a non-empty key. Function returns hash with all read values. Any white
+space (other then new-line character) is preserved.
+
+=cut
+
+sub credential_read {
+ my ($self, $reader) = _maybe_self(@_);
+ my %credential;
+ while (<$reader>) {
+ chomp;
+ if ($_ eq '') {
+ last;
+ } elsif (!/^([^=]+)=(.*)$/) {
+ throw Error::Simple("unable to parse git credential data:\n$_");
+ }
+ $credential{$1} = $2;
+ }
+ return %credential;
+}
+
+=item credential_write( FILEHANDLE, CREDENTIAL_HASHREF )
+
+Writes credential key-value pairs from hash referenced by
+C<CREDENTIAL_HASHREF> to C<FILEHANDLE>. Keys and values cannot contain
+new-lines or NUL bytes characters, and key cannot contain equal signs nor be
+empty (if they do Error::Simple is thrown). Any white space is preserved. If
+value for a key is C<undef>, it will be skipped.
+
+If C<'url'> key exists it will be written first. (All the other key-value
+pairs are written in sorted order but you should not depend on that). Once
+all lines are written, an empty line is printed.
+
+=cut
+
+sub credential_write {
+ my ($self, $writer, $credential) = _maybe_self(@_);
+ my ($key, $value);
+
+ # Check if $credential is valid prior to writing anything
+ while (($key, $value) = each %$credential) {
+ if (!defined $key || !length $key) {
+ throw Error::Simple("credential key empty or undefined");
+ } elsif ($key =~ /[=\n\0]/) {
+ throw Error::Simple("credential key contains invalid characters: $key");
+ } elsif (defined $value && $value =~ /[\n\0]/) {
+ throw Error::Simple("credential value for key=$key contains invalid characters: $value");
+ }
+ }
+
+ for $key (sort {
+ # url overwrites other fields, so it must come first
+ return -1 if $a eq 'url';
+ return 1 if $b eq 'url';
+ return $a cmp $b;
+ } keys %$credential) {
+ if (defined $credential->{$key}) {
+ print $writer $key, '=', $credential->{$key}, "\n";
+ }
+ }
+ print $writer "\n";
+}
+
+sub _credential_run {
+ my ($self, $credential, $op) = _maybe_self(@_);
+ my ($pid, $reader, $writer, $ctx) = command_bidi_pipe('credential', $op);
+
+ credential_write $writer, $credential;
+ close $writer;
+
+ if ($op eq "fill") {
+ %$credential = credential_read $reader;
+ }
+ if (<$reader>) {
+ throw Error::Simple("unexpected output from git credential $op response:\n$_\n");
+ }
+
+ command_close_bidi_pipe($pid, $reader, undef, $ctx);
+}
+
+=item credential( CREDENTIAL_HASHREF [, OPERATION ] )
+
+=item credential( CREDENTIAL_HASHREF, CODE )
+
+Executes C<git credential> for a given set of credentials and specified
+operation. In both forms C<CREDENTIAL_HASHREF> needs to be a reference to
+a hash which stores credentials. Under certain conditions the hash can
+change.
+
+In the first form, C<OPERATION> can be C<'fill'>, C<'approve'> or C<'reject'>,
+and function will execute corresponding C<git credential> sub-command. If
+it's omitted C<'fill'> is assumed. In case of C<'fill'> the values stored in
+C<CREDENTIAL_HASHREF> will be changed to the ones returned by the C<git
+credential fill> command. The usual usage would look something like:
+
+ my %cred = (
+ 'protocol' => 'https',
+ 'host' => 'example.com',
+ 'username' => 'bob'
+ );
+ Git::credential \%cred;
+ if (try_to_authenticate($cred{'username'}, $cred{'password'})) {
+ Git::credential \%cred, 'approve';
+ ... do more stuff ...
+ } else {
+ Git::credential \%cred, 'reject';
+ }
+
+In the second form, C<CODE> needs to be a reference to a subroutine. The
+function will execute C<git credential fill> to fill the provided credential
+hash, then call C<CODE> with C<CREDENTIAL_HASHREF> as the sole argument. If
+C<CODE>'s return value is defined, the function will execute C<git credential
+approve> (if return value yields true) or C<git credential reject> (if return
+value is false). If the return value is undef, nothing at all is executed;
+this is useful, for example, if the credential could neither be verified nor
+rejected due to an unrelated network error. The return value is the same as
+what C<CODE> returns. With this form, the usage might look as follows:
+
+ if (Git::credential {
+ 'protocol' => 'https',
+ 'host' => 'example.com',
+ 'username' => 'bob'
+ }, sub {
+ my $cred = shift;
+ return !!try_to_authenticate($cred->{'username'},
+ $cred->{'password'});
+ }) {
+ ... do more stuff ...
+ }
+
+=cut
+
+sub credential {
+ my ($self, $credential, $op_or_code) = (_maybe_self(@_), 'fill');
+
+ if ('CODE' eq ref $op_or_code) {
+ _credential_run $credential, 'fill';
+ my $ret = $op_or_code->($credential);
+ if (defined $ret) {
+ _credential_run $credential, $ret ? 'approve' : 'reject';
+ }
+ return $ret;
+ } else {
+ _credential_run $credential, $op_or_code;
+ }
+}
+
{ # %TEMP_* Lexical Context
my (%TEMP_FILEMAP, %TEMP_FILES);
--
1.8.1.3.572.g32bae1f
^ permalink raw reply related
* [PATCHv4 2/6] Git.pm: fix example in command_close_bidi_pipe documentation
From: Michal Nazarewicz @ 2013-02-12 14:02 UTC (permalink / raw)
To: peff, gitster; +Cc: git
In-Reply-To: <cover.1360677646.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index bbb753a..11f310a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -418,7 +418,7 @@ and it is the fourth value returned by C<command_bidi_pipe()>. The call idiom
is:
my ($pid, $in, $out, $ctx) = $r->command_bidi_pipe('cat-file --batch-check');
- print "000000000\n" $out;
+ print $out "000000000\n";
while (<$in>) { ... }
$r->command_close_bidi_pipe($pid, $in, $out, $ctx);
--
1.8.1.3.572.g32bae1f
^ permalink raw reply related
* Re: Fwd: Re: [git-multimail] License unknown (#1)
From: Andy Parkins @ 2013-02-12 15:11 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git
In-Reply-To: <511A04DA.7010809@alum.mit.edu>
On Tuesday 12 February 2013 09:01:14 Michael Haggerty wrote:
> I assume you are the Andy Parkins who originally submitted
> post-commit-email to the Git project...
I am indeed. Hello.
> I have derived another script from yours:
>
> https://github.com/mhagger/git-multimail
>
> I affixed the GPLv2 boilerplate to the code under the assumption that it
> inherited this license from the Git project. But afterwards I realized
> that I am not entirely confident that the GPLv2 applies to
> post-commit-email.
I'm sorry, that's entirely my fault. I had read somewhere in the git
submission policy that everything submitted was naturally under the same
license as git itself.
> Obviously the situation is even more complicated because other people
> have contributed patches to the script since your original submission.
> But it would nevertheless be very helpful if you would clarify your
> original intention, *especially* if your intention was to put the script
> under a license different than GPLv2.
My intention was to put my code under exactly the license the git itself is
under. I am delighted for you or anyone else to use or derive from it.
I'm also sorry I didn't respond to your first email, it accidentally got
sorted into spam.
If anyone wants some sort of legal declaration, I'm happy to give it.
> -------- Original Message --------
> Subject: Re: [git-multimail] License unknown (#1)
> Date: Sun, 27 Jan 2013 19:52:58 +0100
> From: Michael Haggerty <mhagger@alum.mit.edu>
> To: git discussion list <git@vger.kernel.org>, Andy Parkins
> <andyparkins@gmail.com>
> CC: mhagger/git-multimail
> <reply+i-10159725-60cb2c338c594bd09d77fe2f8d628aa55114a3f6-119718@reply.git
> hub.com>, Michiel Holtkamp <notifications@github.com>
>
> I have a question about the license of contrib/hooks/post-commit-email.
> I had assumed that since it is in the git project, which is GPLv2, and
> since it contains no contrary information, it would by implication also
> fall under GPLv2. But the file itself contains no explicit license
> information, and it is not clear to me that the "signed-off-by" line
> implies a particular license, either. (The signed-off-by *does* seem to
> imply that the source code is under some kind of open source license,
> but not which one.)
>
> If somebody can explain what license the code is under and how they come
> to that conclusion, I would be very grateful.
>
> And if Andy Parkins (the original author) is listening, please indicate
> whether you had any intent *other* than GPLv2.
>
> For anybody who is interested, the file was first committed in
> 4557e0de5b and has been modified by several authors since then.
>
> Given the pretty clear open-sourciness of the script, I don't think this
> has to be made into a big issue. But it would be nice to state the
> license explicitly for future users' information.
>
> Thanks,
> Michael
>
> On 01/27/2013 02:38 PM, Michiel Holtkamp wrote:
> > Actually, I'm not sure that it is GPLv2 for the original script. The
> > COPYING file in the main project declares the project as GPLv2, but it
> > also says that people contributing should make their preferences (for
> > licensing) known. Maybe we can assume it's GPLv2, (as the original
> > writer might have assumed it was GPLv2), but it's not explicitly stated
> > so I'm not sure (IANAL).
--
Dr Andy Parkins
Director
FussyLogic Ltd
tel: 0845 557 7645
web: www.fussylogic.co.uk
Company Registration No. 8198285 Registered in England and Wales
This email and any attachments to it may be confidential and are
intended solely for the use of the individual to whom it is addressed.
Any views or opinions expressed are solely those of the author and do
not necessarily represent those of FussyLogic Ltd.
If you are not the intended recipient of this email, you must neither
take any action based upon its contents, nor copy or show it to
anyone.
Please contact the sender if you believe you have received this email
in error.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox