* [PATCH v2 01/10] sequencer.c: remove broken support for rfc2822 continuation in footer
From: Brandon Casey @ 2013-01-21 8:40 UTC (permalink / raw)
To: gitster; +Cc: pclouds, git, Brandon Casey, Brandon Casey
In-Reply-To: <1358757627-16682-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 aef5e8a..fe25ef4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1027,7 +1027,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--) {
@@ -1044,11 +1043,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.1.1.252.gdb33759
^ permalink raw reply related
* Re: git interactive rebase 'consume' command
From: Stephen Kelly @ 2013-01-21 8:40 UTC (permalink / raw)
To: git
In-Reply-To: <7v8v7nli2a.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Stephen Kelly <steveire@gmail.com> writes:
>> One scenario is something like this:
>>
>> Start with a clean HEAD (always a good idea :) )
>> hack hack hack
>> make multiple commits
>> realize that a hunk you committed in an early patch belongs in a later
>> one. use git rebase -i to fix it.
>>
>> Is that more clear?
>
> Not really.
I think there are other scenarios, but I guess this won't happen anyway.
Thanks,
Steve.
^ permalink raw reply
* [PATCH v2 02/10] t/test-lib-functions.sh: allow to specify the tag name to test_commit
From: Brandon Casey @ 2013-01-21 8:40 UTC (permalink / raw)
To: gitster; +Cc: pclouds, git, Brandon Casey, Brandon Casey
In-Reply-To: <1358757627-16682-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 fa62d01..c601918 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.1.1.252.gdb33759
^ permalink raw reply related
* [PATCH v2 05/10] sequencer.c: always separate "(cherry picked from" from commit body
From: Brandon Casey @ 2013-01-21 8:40 UTC (permalink / raw)
To: gitster; +Cc: pclouds, git, Brandon Casey, Brandon Casey
In-Reply-To: <1358757627-16682-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 | 114 +++++++++++++++++++++++++----------------------
t/t3511-cherry-pick-x.sh | 53 ++++++++++++++++++++++
2 files changed, 113 insertions(+), 54 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index fe76a1d..163dc12 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -20,6 +20,64 @@
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_picked_from_line(const char *buf, int len)
+{
+ /*
+ * We only care that it looks roughly like (cherry picked from ...)
+ */
+ return !prefixcmp(buf, cherry_picked_prefix) &&
+ (buf[len - 1] == ')' ||
+ (buf[len - 1] == '\n' && buf[len - 2] == ')'));
+}
+
+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_picked_from_line(buf + i, k - i)))
+ return 0;
+ }
+ return 1;
+}
+
static void remove_sequencer_state(void)
{
struct strbuf seq_dir = STRBUF_INIT;
@@ -497,6 +555,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,60 +1082,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_picked_from_line(const char *buf, int len)
-{
- /*
- * We only care that it looks roughly like (cherry picked from ...)
- */
- return !prefixcmp(buf, cherry_picked_prefix) &&
- (buf[len - 1] == ')' ||
- (buf[len - 1] == '\n' && buf[len - 2] == ')'));
-}
-
-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_picked_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.1.1.252.gdb33759
^ permalink raw reply related
* [PATCH v2 04/10] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
From: Brandon Casey @ 2013-01-21 8:40 UTC (permalink / raw)
To: gitster; +Cc: pclouds, git, Brandon Casey, Brandon Casey
In-Reply-To: <1358757627-16682-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>
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
sequencer.c | 46 ++++++++++++++++++++++++++++------------
t/t3511-cherry-pick-x.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 88 insertions(+), 13 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index fe25ef4..fe76a1d 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,11 +1022,36 @@ 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_picked_from_line(const char *buf, int len)
+{
+ /*
+ * We only care that it looks roughly like (cherry picked from ...)
+ */
+ return !prefixcmp(buf, cherry_picked_prefix) &&
+ (buf[len - 1] == ')' ||
+ (buf[len - 1] == '\n' && buf[len - 2] == ')'));
+}
+
+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;
@@ -1043,15 +1069,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_picked_from_line(buf + i, k - i)))
return 0;
- }
}
return 1;
}
@@ -1068,7 +1088,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.1.1.252.gdb33759
^ permalink raw reply related
* [PATCH v2 06/10] sequencer.c: teach append_signoff how to detect duplicate s-o-b
From: Brandon Casey @ 2013-01-21 8:40 UTC (permalink / raw)
To: gitster; +Cc: pclouds, git, Brandon Casey, Brandon Casey
In-Reply-To: <1358757627-16682-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 | 46 ++++++++++++++++++++++++++++++++++++----------
sequencer.h | 2 +-
t/t3511-cherry-pick-x.sh | 2 +-
4 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 7c2a3d4..081ff66 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 163dc12..d4a2ece 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -46,12 +46,20 @@ static int is_cherry_picked_from_line(const char *buf, int len)
(buf[len - 1] == '\n' && buf[len - 2] == ')'));
}
-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 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')
@@ -67,14 +75,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) ||
+ found_rfc2822 = is_rfc2822_line(buf + i, k - i);
+ 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)))
return 0;
}
+ if (found_sob == i)
+ return 3;
+ if (found_sob)
+ return 2;
return 1;
}
@@ -296,7 +315,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;
@@ -555,7 +574,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));
@@ -1082,9 +1101,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);
@@ -1093,10 +1113,16 @@ 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);
+
+ 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..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.1.1.252.gdb33759
^ permalink raw reply related
* [PATCH v2 07/10] sequencer.c: teach append_signoff to avoid adding a duplicate newline
From: Brandon Casey @ 2013-01-21 8:40 UTC (permalink / raw)
To: gitster; +Cc: pclouds, git, Brandon Casey, Brandon Casey
In-Reply-To: <1358757627-16682-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 | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index d4a2ece..d51e6f2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1114,11 +1114,15 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, int no_dup_sob)
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 (!has_footer)
- strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
+ if (msgbuf->buf[i] != '\n') {
+ if (i)
+ 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,
--
1.8.1.1.252.gdb33759
^ permalink raw reply related
* [PATCH v2 08/10] t4014: more tests about appending s-o-b lines
From: Brandon Casey @ 2013-01-21 8:40 UTC (permalink / raw)
To: gitster; +Cc: pclouds, git, Brandon Casey
In-Reply-To: <1358757627-16682-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 | 242 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 242 insertions(+)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 7fa3647..3868cef 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1021,4 +1021,246 @@ 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} |
+ 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' '
+ 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.1.252.gdb33759
^ permalink raw reply related
* [PATCH v2 10/10] Unify appending signoff in format-patch, commit and sequencer
From: Brandon Casey @ 2013-01-21 8:40 UTC (permalink / raw)
To: gitster; +Cc: pclouds, git, Brandon Casey, Brandon Casey
In-Reply-To: <1358757627-16682-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 | 88 +------------------------------------------------
t/t4014-format-patch.sh | 31 ++++++++++++++---
2 files changed, 27 insertions(+), 92 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 83f33f4..299dad3 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,93 +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, int no_dup_sob)
-{
- 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 3868cef..d0ec097 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1104,7 +1104,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
@@ -1120,7 +1141,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
@@ -1136,7 +1157,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
@@ -1153,7 +1174,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
@@ -1221,7 +1242,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.1.252.gdb33759
^ permalink raw reply related
* [PATCH v2 09/10] format-patch: update append_signoff prototype
From: Brandon Casey @ 2013-01-21 8:40 UTC (permalink / raw)
To: gitster; +Cc: pclouds, git, Brandon Casey
In-Reply-To: <1358757627-16682-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 | 14 ++++++++++----
revision.h | 2 +-
3 files changed, 12 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..83f33f4 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 ignore_footer, int no_dup_sob)
{
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 +277,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 +290,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 +676,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 +692,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, 1);
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.1.252.gdb33759
^ permalink raw reply related
* [PATCH v2 03/10] t/t3511: add some tests of 'cherry-pick -s' functionality
From: Brandon Casey @ 2013-01-21 8:40 UTC (permalink / raw)
To: gitster; +Cc: pclouds, git, Brandon Casey, Brandon Casey
In-Reply-To: <1358757627-16682-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.1.1.252.gdb33759
^ permalink raw reply related
* Re: [RFC] git rm -u
From: Matthieu Moy @ 2013-01-21 8:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Eric James Michael Ritz, git, Tomas Carnecky
In-Reply-To: <7v622rn1bh.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> "git add -u" is one of the only exceptions (with "git grep"). I consider
>> this as a bug, and think this should be changed. This has been discussed
>> several times here, but no one took the time to actually do the change
>
> Did we ever agree that it is a good change to begin with? Pointers?
I don't think a consensus was reached, but it has been discussed at
least once in this thread:
http://thread.gmane.org/gmane.comp.version-control.git/166223/focus=168238
Essentially, the discussion boiled down to "it would be cool to change,
but the migration won't be easy".
The main argument for change is (for me) consistency. Having
"git add -p" tree-wide and "git add -u" limited to . is really strange.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [RFC] git rm -u
From: Junio C Hamano @ 2013-01-21 9:23 UTC (permalink / raw)
To: Piotr Krukowiecki
Cc: Matthieu Moy, Jonathan Nieder, Eric James Michael Ritz,
Git Mailing List, Tomas Carnecky
In-Reply-To: <CAA01Csrv26WrrJDAo-1cr+rW6rYFGQZpYgtafEh=Wgtzswdv_g@mail.gmail.com>
Piotr Krukowiecki <piotr.krukowiecki@gmail.com> writes:
> Do you mean "git add" will be disallowed without "." or ":/" argument?
> Or will this change in future and "git add" without argument will me
> "whole tree", same as ":/" ?
No. This is only about "git add -u<RETURN>", not any other forms of
"git add ...with or without other args...".
"git add -u<RETURN>" historically meant, and it still means, to
"update the index with every change in the working tree", even when
you are in a subdirectory.
Back when "git add -u" was invented, we didn't have the ":/", which
lets us tell commands that take pathspecs "I want everything from
the top of the working tree.". If "git add -u<RETURN>" limited its
operation to the current directory, after working everywhere in the
working tree, cd'ing around and ending up to be in a subdirectory
somwhere deep, you had to "cd ../../.. && git add -u", which was
cumbersome. If "git add -u" always meant the whole tree, limiting
it to the current directory with "git add -u .<RETURN>" was easy,
and that is why the default was chosen to the "whole tree".
Because we have ":/" these days, changing something that limits its
action to the current directory by default to instead work on the
whole tree no longer makes much sense. That is, if we _were_ to
change "git add -u<RETURN>", it would be in the opposite direction,
i.e. to update the index only with the paths below the current
directory.
Such a change has to be done carefully. Existing users do expect
the current behaviour, so we have to first _break_ their fingers and
habits and train them to say "add -u :/" when they mean the whole
tree operation. Silently accepting "add -u" and changing its
meaning to update the index only with the paths below the current
directory will cause them trouble by leaving changes they _thought_
they added out of the index, and is an unacceptable change.
The first step of migration is "git add -u<RETURN>" that loudly
warns, so that uses of that form in scripts are updated before the
second step to avoid a flag-day breakage and start traing fingers
and habits of the users.
The second step is to make "add -u<RETURN>" fail, again with a
message that tells users to be explicit and add ":/" or "." at the
end if they mean "the whole tree" or "the current directory".
After keeping Git in that secnd step for sufficiently long time to
train users to type ":/" or "." explicitly, we can then finally
switch the default of "git add -u<RETURN>" to limit it to the
current directory, instead of failing the command.
^ permalink raw reply
* Re: git-cvsimport-3 and incremental imports
From: John Keeping @ 2013-01-21 9:36 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20130120232008.GA25001@thyrsus.com>
On Sun, Jan 20, 2013 at 06:20:08PM -0500, Eric S. Raymond wrote:
> John Keeping <john@keeping.me.uk>:
>> I don't think there is any way to solve this without giving cvsps more
>> information, probably the last commit time for all git branches, but
>> perhaps I'm missing a fast-import feature that can help solve this
>> problem.
>
> Yes, you are. The magic incantation is
>
> from refs/heads/<branch>^0
>
> I've just pushed a cvsps-3.9 with an -i option that generates these at
> each branch root. Combine it with -d and you get incremental
> fast-export.
I don't think this is enough. I made a very similar change here for
testing (conditional on relative_date_start instead of a new flag) and I
needed this as well in order to prevent empty duplicate commits being
added:
-- >8 --
diff --git a/cvsps.c b/cvsps.c
index fb6a3ad..5771462 100644
--- a/cvsps.c
+++ b/cvsps.c
@@ -1560,7 +1560,7 @@ static bool visible(PatchSet * ps)
ok:
//fprintf(stderr, "Time check: %zd %zd %zd\n", restrict_date_start, restrict_date_end, ps->date);
if (restrict_date_start > 0 &&
- (ps->date < restrict_date_start ||
+ (ps->date <= restrict_date_start ||
(restrict_date_end > 0 && ps->date > restrict_date_end)))
return false;
-- 8< --
But this is nothing more than a sticking plaster that happens to do
enough in this particular case - if the Git repository happened to be on
a different branch, the start date would be wrong and too many or too
few commits could be output. Git doesn't detect that they commits are
identical to some that we already have because we're explicitly telling
it to make a new commit with the specified parent.
You can easily see the breakage by running the tests in the Git tree,
where the CVS revision map tests fail because they end up with duplicate
versions.
You'll need my cvsimport-3 branch to see these failures as it adds the
"git config" support that the tests rely on:
git://github.com/johnkeeping/git.git cvsimport-3
John
^ permalink raw reply related
* Re: git interactive rebase 'consume' command
From: Michael Haggerty @ 2013-01-21 11:05 UTC (permalink / raw)
To: Stephen Kelly; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <kdgtir$apt$1@ger.gmane.org>
On 01/20/2013 03:05 PM, Stephen Kelly wrote:
> I find the fixup command during an interactive rebase useful.
>
> Sometimes when cleaning up a branch, I end up in a situation like this:
>
> pick 07bc3c9 Good commit.
> pick 1313a5e Commit to fixup into c2f62a3.
> pick c2f62a3 Another commit.
>
>
> So, I have to reorder the commits, and change 1313a5e to 'f'. An alternative
> would be to squash 's' c2f62a3 into 1313a5e and clean up the commit message.
> The problem with that is it ends up with the wrong author time information.
I do "squash with successor then clean up commit message" all the time.
I had never worried (or even thought much) about the author time of the
resulting commit. I think I will continue not worrying about it :-)
I think it would be great to have a shorthand for this operation in "git
rebase --interactive" and I probably would have implemented it when I
added "fixup" if I had been able to think of a good name for it. Even
though I do this sort of thing less frequently than "fixup", it still
comes up often enough that a special command for it would be useful.
> So, I usually reorder and then fixup, but that can also be problematic if I
> get a conflict during the re-order (which is quite likely).
It is perverse to have to turn a well-defined and manifestly
conflict-free wish into one that has a good chance of conflicting, just
because of a limitation of the tool.
> I would prefer to be able to mark a commit as 'should be consumed', so that:
>
> pick 07bc3c9 Good commit.
> consume 1313a5e Commit to fixup into c2f62a3.
> pick c2f62a3 Another commit.
>
> will result in
>
> pick 07bc3c9 Good commit.
> pick 62a3c2f Another commit.
>
> directly.
Excellent. But the name is not self-explanatory. And there is
something different about your "consume" command:
Normally, "pick" means that the commit on that line is the start of a
new commit unrelated to its predecessors. And in general, the command
on one line only affects the lines that come before it, not the lines
that come after it. Under your proposal "consume" would change the
meaning of the following line, namely by changing what its "pick" means.
It might be more consistent to require the following line to be changed
to "squash":
pick 07bc3c9 Good commit.
consume 1313a5e Commit to fixup into c2f62a3.
squash c2f62a3 Another commit.
in which case the meaning of "consume" would be something like "pick
this commit but not its commit message. There would have to be a
prohibition against generating commits with *no* commit messages, to
prevent series like [consume, pick] or [consume, fix, pick] while
allowing series like [consume, consume, squash, fix, fix].
If this is the interpretation, the name "quiet/q" might make things clearer.
Yet another approach would be to allow options on the commands. For
example,
pick 07bc3c9 Good commit.
pick --quiet 1313a5e Commit to fixup into c2f62a3.
squash c2f62a3 Another commit.
In fact if options were implemented, then "fixup" would mean the same as
"squash --quiet", "reword" could be written "pick --edit", and I'm sure
the new flexibility would make it easier to add other features (e.g.,
"pick --reset-author").
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: git-cvsimport-3 and incremental imports
From: Eric S. Raymond @ 2013-01-21 11:28 UTC (permalink / raw)
To: John Keeping; +Cc: git
In-Reply-To: <20130121093658.GD7498@serenity.lan>
John Keeping <john@keeping.me.uk>:
> But this is nothing more than a sticking plaster that happens to do
> enough in this particular case
I'm beginning to think that's the best outcome we ever get in this
problem domain...
> - if the Git repository happened to be on
> a different branch, the start date would be wrong and too many or too
> few commits could be output. Git doesn't detect that they commits are
> identical to some that we already have because we're explicitly telling
> it to make a new commit with the specified parent.
Then I don't understand the actual failure case. Either that or you
don't understand the effect of -i. Have you actually experimented with
it? The reason I suspect you don't understand the feature is that it
shouldn't make any difference to the way -i works which repository branch is
active at the time of the second import.
Here is how I model what is going on:
1. We make commits to multiple branches of a CVS repo up to some given time T.
2. We import it, ending up with a collection of git branches all of which
have tip commits dated T or earlier. And *every* commit dated T or earlier
gets copied over.
3. We make more commits to the same set of branches in CVS.
4. We now run cvsps -d T on the repo. This generates an incremental
fast-import stream describing all CVS commits *newer* than T (see
the cvsps manual page).
5. That stream should consist of a set of disconnected branches, each
(because of -i) beginning with a root commit containing "from
refs/heads/foo^0" which says to parent the commit on the tip of
branch foo, whatever that happens to be. (I don't have to guess
about this, I tested the feature before shipping.)
6. Now, when git fast-import interprets that stream in the context of
the repository produced in step 2, for each branch in the
incremental dump the branch root commit is parented on the tip
commit of the same branch in the repo.
At step 6, it shouldn't matter at all which branch is active, because
where an incremental branch root gets attached has nothing to do with
which branch is active.
It is sufficient to avoid duplicate commits that cvsps -d 0 -d T and
cvsps -d T run on the same CVS repo operate on *disjoint sets* of CVS
file commits. I can see this technique possibly getting confused if T
falls in the middle of a changeset where the CVS timestamps for the
file commits are out of order. But that's the same case that will
fail if we're importing at file-commit granularity, so there's no new
bug here.
Can you explain at what step my logic is incorrect?
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* [PATCH v2] unpack-trees: do not abort when overwriting an existing file with the same content
From: Nguyễn Thái Ngọc Duy @ 2013-01-21 11:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358594648-26851-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Changes: do not lose worktree's executable permission.
t/t1011-read-tree-sparse-checkout.sh | 3 ++-
t/t2021-checkout-overwrite.sh | 18 ++++++++++++++++++
unpack-trees.c | 31 +++++++++++++++++++++++++++++++
3 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
index 5c0053a..38f9899 100755
--- a/t/t1011-read-tree-sparse-checkout.sh
+++ b/t/t1011-read-tree-sparse-checkout.sh
@@ -238,7 +238,8 @@ test_expect_success 'print errors when failed to update worktree' '
echo sub >.git/info/sparse-checkout &&
git checkout -f init &&
mkdir sub &&
- touch sub/added sub/addedtoo &&
+ echo modified >sub/added &&
+ echo modified >sub/addedtoo &&
test_must_fail git checkout top 2>actual &&
cat >expected <<\EOF &&
error: The following untracked working tree files would be overwritten by checkout:
diff --git a/t/t2021-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh
index 5da63e9..bb1696d 100755
--- a/t/t2021-checkout-overwrite.sh
+++ b/t/t2021-checkout-overwrite.sh
@@ -47,4 +47,22 @@ test_expect_success SYMLINKS 'checkout commit with dir must not remove untracked
test -h a/b
'
+test_expect_success 'do not abort on overwriting an existing file with the same content' '
+ echo abc >bar &&
+ git add bar &&
+ git commit -m "new file" &&
+ git reset HEAD^ &&
+ git checkout HEAD@{1}
+'
+
+test_expect_success POSIXPERM 'do abort on an existing file, same content but different permission' '
+ git checkout -f HEAD^ &&
+ echo abc >bar &&
+ git add bar &&
+ git commit -m "new file" &&
+ git reset HEAD^ &&
+ chmod a+x bar &&
+ test_must_fail git checkout HEAD@{1}
+'
+
test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 0e1a196..ea204ae 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -9,6 +9,8 @@
#include "refs.h"
#include "attr.h"
+#define SAME_CONTENT_SIZE_LIMIT (1024 * 1024)
+
/*
* Error messages expected by scripts out of plumbing commands such as
* read-tree. Non-scripted Porcelain is not required to use these messages
@@ -1363,6 +1365,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
struct unpack_trees_options *o)
{
struct cache_entry *result;
+ unsigned long ce_size;
/*
* It may be that the 'lstat()' succeeded even though
@@ -1405,6 +1408,34 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
return 0;
}
+ /*
+ * If it has the same content that we are going to overwrite,
+ * there's no point in complaining. We still overwrite it in
+ * the end though.
+ */
+ if (ce &&
+ S_ISREG(st->st_mode) && S_ISREG(ce->ce_mode) &&
+ (!trust_executable_bit ||
+ (0100 & (ce->ce_mode ^ st->st_mode)) == 0) &&
+ st->st_size < SAME_CONTENT_SIZE_LIMIT &&
+ sha1_object_info(ce->sha1, &ce_size) == OBJ_BLOB &&
+ ce_size == st->st_size) {
+ void *buffer = NULL;
+ unsigned long size;
+ enum object_type type;
+ struct strbuf sb = STRBUF_INIT;
+ int matched =
+ strbuf_read_file(&sb, ce->name, ce_size) == ce_size &&
+ (buffer = read_sha1_file(ce->sha1, &type, &size)) != NULL &&
+ type == OBJ_BLOB &&
+ size == ce_size &&
+ !memcmp(buffer, sb.buf, size);
+ free(buffer);
+ strbuf_release(&sb);
+ if (matched)
+ return 0;
+ }
+
return o->gently ? -1 :
add_rejected_path(o, error_type, name);
}
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* Re: git-cvsimport-3 and incremental imports
From: John Keeping @ 2013-01-21 12:00 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20130121112853.GA31693@thyrsus.com>
On Mon, Jan 21, 2013 at 06:28:53AM -0500, Eric S. Raymond wrote:
> John Keeping <john@keeping.me.uk>:
>> But this is nothing more than a sticking plaster that happens to do
>> enough in this particular case
>
> I'm beginning to think that's the best outcome we ever get in this
> problem domain...
I don't think we can ever get a perfect outcome, but it should be
possible to do a little bit better without too much effort.
>> - if the Git repository happened to be on
>> a different branch, the start date would be wrong and too many or too
>> few commits could be output. Git doesn't detect that they commits are
>> identical to some that we already have because we're explicitly telling
>> it to make a new commit with the specified parent.
>
> Then I don't understand the actual failure case. Either that or you
> don't understand the effect of -i. Have you actually experimented with
> it? The reason I suspect you don't understand the feature is that it
> shouldn't make any difference to the way -i works which repository branch is
> active at the time of the second import.
>
> Here is how I model what is going on:
>
> 1. We make commits to multiple branches of a CVS repo up to some given time T.
>
> 2. We import it, ending up with a collection of git branches all of which
> have tip commits dated T or earlier. And *every* commit dated T or earlier
> gets copied over.
>
> 3. We make more commits to the same set of branches in CVS.
>
> 4. We now run cvsps -d T on the repo. This generates an incremental
> fast-import stream describing all CVS commits *newer* than T (see
> the cvsps manual page).
This is the problem step. There are two scenarios that have problems:
1. If I create a new development branch in my Git repository and commit
something to it then git-cvsimport-3 will pass a time to cvsps that
is newer than the actual time of the last import, so T is wrong.
It may be possible to fix this case purely in git-cvsimport-3.
2. If the branch I have checked out is not the newest CVS branch, then
git-cvsimport-3 will pass a value of T that is before the time of the
last import. This case is more subtle but it results in unwanted
duplicate commits since git-fast-import will just do what it's told
and create the new commits.
So if we have the following commits:
commit1 at time 1
commit2 at time 2
commit3 at time 3
and I call "cvsps -d 2 -i" I end up with the series:
commit1 at time 1
commit2 at time 2
commit3 at time 3
commit2 at time 2 - effectively reverting the previous commit
commit3 at time 3 - a duplicate
... and potentially genuinely new commits
This is demonstrated by running the Git test t9650.
I also disagree that cvsps outputs commits *newer* than T since it will
also output commits *at* T, which is what I changed with the patch in my
previous message. This fixes the duplicate commit2 in the series above,
but not the duplicate commit3.
> 5. That stream should consist of a set of disconnected branches, each
> (because of -i) beginning with a root commit containing "from
> refs/heads/foo^0" which says to parent the commit on the tip of
> branch foo, whatever that happens to be. (I don't have to guess
> about this, I tested the feature before shipping.)
>
> 6. Now, when git fast-import interprets that stream in the context of
> the repository produced in step 2, for each branch in the
> incremental dump the branch root commit is parented on the tip
> commit of the same branch in the repo.
>
> At step 6, it shouldn't matter at all which branch is active, because
> where an incremental branch root gets attached has nothing to do with
> which branch is active.
>
> It is sufficient to avoid duplicate commits that cvsps -d 0 -d T and
> cvsps -d T run on the same CVS repo operate on *disjoint sets* of CVS
> file commits. I can see this technique possibly getting confused if T
> falls in the middle of a changeset where the CVS timestamps for the
> file commits are out of order. But that's the same case that will
> fail if we're importing at file-commit granularity, so there's no new
> bug here.
>
> Can you explain at what step my logic is incorrect?
Your logic is correct - for cvsps - the problem is where T comes from.
Perhaps it is simplest to just save a CVS_LAST_IMPORT_TIME file in
$GIT_DIR and not worry about it any more.
John
^ permalink raw reply
* [RFC/PATCH] add: warn when -u or -A is used without filepattern
From: Matthieu Moy @ 2013-01-21 12:00 UTC (permalink / raw)
To: git, gitster
Cc: Jonathan Nieder, Eric James Michael Ritz, Tomas Carnecky,
Matthieu Moy
In-Reply-To: <7v1udfn0tm.fsf@alter.siamese.dyndns.org>
Most git commands that can be used with our without a filepattern are
tree-wide by default, the filepattern being used to restrict their scope.
A few exceptions are: 'git grep', 'git clean', 'git add -u' and 'git add -A'.
The inconsistancy of 'git add -u' and 'git add -A' are particularly
problematic since other 'git add' subcommands (namely 'git add -p' and
'git add -e') are tree-wide by default.
Flipping the default now is unacceptable, so this patch starts training
users to type explicitely 'git add -u|-A :/' or 'git add -u|-A .', to prepare
for the next steps:
* forbid 'git add -u|-A' without filepattern (like 'git add' without
option)
* much later, maybe, re-allow 'git add -u|-A' without filepattern, with a
tree-wide scope.
A nice side effect of this patch is that it makes the :/ special
filepattern easier to discover for users.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Junio C Hamano <gitster@pobox.com> writes:
> The first step of "git add -u" migration plan would be to warn when
> no argument is given and update all the existing index entries, and
> give the same advise to use either "." or ":/". Keep this for three
> cycles: 3 * (8 to 10 weeks per cycle) = 27 weeks ~ 1/2 year.
The first step should look like this patch. The message remains vague
about the next steps ("change in a future Git version", no mention of
the exact change nor of the exact version in which it will happen),
but I'm fine with refining it (perhaps this could be a 2.0 change,
like the change to push.default?).
Documentation/git-add.txt | 7 ++++---
builtin/add.c | 30 +++++++++++++++++++++++++++++-
2 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index fd9e36b..5333559 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -107,9 +107,10 @@ apply to the index. See EDITING PATCHES below.
from the index if the corresponding files in the working tree
have been removed.
+
-If no <filepattern> is given, default to "."; in other words,
-update all tracked files in the current directory and its
-subdirectories.
+If no <filepattern> is given, the current version of Git defaults to
+"."; in other words, update all tracked files in the current directory
+and its subdirectories. This default will change in a future version
+of Git, hence the form without <filepattern> should not be used.
-A::
--all::
diff --git a/builtin/add.c b/builtin/add.c
index e664100..e6eb829 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -373,6 +373,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int add_new_files;
int require_pathspec;
char *seen = NULL;
+ const char *option_with_implicit_dot = NULL;
git_config(add_config, NULL);
@@ -392,7 +393,34 @@ int cmd_add(int argc, const char **argv, const char *prefix)
die(_("-A and -u are mutually incompatible"));
if (!show_only && ignore_missing)
die(_("Option --ignore-missing can only be used together with --dry-run"));
- if ((addremove || take_worktree_changes) && !argc) {
+ if (addremove)
+ option_with_implicit_dot = "--all";
+ if (take_worktree_changes)
+ option_with_implicit_dot = "--update";
+ if (option_with_implicit_dot && !argc) {
+ /*
+ * To be consistant with "git add -p" and most Git
+ * commands, we should default to being tree-wide, but
+ * this is not the original behavior and can't be
+ * changed until users trained themselves not to type
+ * "git add -u" or "git add -A". For now, we warn and
+ * keep the old behavior. Later, this warning can be
+ * turned into a die(...), and eventually we may
+ * reallow the command with a new behavior.
+ */
+ warning(_("The behavior of 'git add %s' with no path argument will change in a future\n"
+ "Git version and shouldn't be used anymore. To add content for the whole tree, run:\n"
+ "\n"
+ " git add %s :/\n"
+ "\n"
+ "To restrict the command to the current directory, run:\n"
+ "\n"
+ " git add %s .\n"
+ "\n"
+ "With the current Git version, the command is restricted to the current directory."),
+ option_with_implicit_dot,
+ option_with_implicit_dot,
+ option_with_implicit_dot);
static const char *here[2] = { ".", NULL };
argc = 1;
argv = here;
--
1.8.0.319.g8abfee4
^ permalink raw reply related
* Re: git-cvsimport-3 and incremental imports
From: Eric S. Raymond @ 2013-01-21 12:43 UTC (permalink / raw)
To: John Keeping; +Cc: git
In-Reply-To: <20130121120010.GE7498@serenity.lan>
John Keeping <john@keeping.me.uk>:
> I also disagree that cvsps outputs commits *newer* than T since it will
> also output commits *at* T, which is what I changed with the patch in my
> previous message.
Ah. OK, that is yet another bug inherited from 2.x - the code doesn't
match the documented (and correct) behavior. Please send me a patch
against the cvsps repo, I'll merge it.
> Perhaps it is simplest to just save a CVS_LAST_IMPORT_TIME file in
> $GIT_DIR and not worry about it any more.
Yes, I think you're right. Trying to carry that information in-band would
probably doom us to all sorts of bug-prone complications.
Thanks for the good analysis. I wish everybody I had to chase bugs with
could explain them with such clarity and concision.
Sigh. Now I have to figure out if cvsps's behavior can be rescued in Chris
Rorvick's recently-discovered failure case. I'm not optimistic.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* [PATCH 1/2] Update :/abc ambiguity check
From: Nguyễn Thái Ngọc Duy @ 2013-01-21 13:00 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
:/abc may mean two things:
- as a revision, it means the revision that has "abc" in commit
message.
- as a pathpec, it means "abc" from root.
Currently we see ":/abc" as a rev (most of the time), but never see it
as a pathspec even if "abc" exists and "git log :/abc" will gladly
take ":/abc" as rev even it's ambiguous. This patch makes it:
- ambiguous when "abc" exists on worktree
- a rev if abc does not exist on worktree
- a path if abc is not found in any commits (although better use
"--" to avoid ambiguation because searching through commit DAG is
expensive)
A plus from this patch is, because ":/" never matches anything as a
rev, it is never considered a valid rev and because root directory
always exists, ":/" is always unambiguously seen as a pathspec.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
This makes "git grep foo :/" work. For such an often used command,
the less I type the better.
The ".. is expensive" thing is not cool. But it hopefully does not
happen often. I expect people rely on shell's tab completion more
than :/typing-path-to-somewhere-manually. Copy/paste happens more
often. I usually type ":" then copy a path from diff --git line.
We should probably kill ":/abc as a rev" and replace it with @{/abc}
or something less ambiguous.
setup.c | 9 ++++++++-
t/t4208-log-magic-pathspec.sh | 17 +++++++++++++++--
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/setup.c b/setup.c
index f108c4b..47acc11 100644
--- a/setup.c
+++ b/setup.c
@@ -66,7 +66,14 @@ int check_filename(const char *prefix, const char *arg)
const char *name;
struct stat st;
- name = prefix ? prefix_filename(prefix, strlen(prefix), arg) : arg;
+ if (!prefixcmp(arg, ":/")) {
+ if (arg[2] == '\0') /* ":/" is root dir, always exists */
+ return 1;
+ name = arg + 2;
+ } else if (prefix)
+ name = prefix_filename(prefix, strlen(prefix), arg);
+ else
+ name = arg;
if (!lstat(name, &st))
return 1; /* file exists */
if (errno == ENOENT || errno == ENOTDIR)
diff --git a/t/t4208-log-magic-pathspec.sh b/t/t4208-log-magic-pathspec.sh
index 2c482b6..72300b5 100755
--- a/t/t4208-log-magic-pathspec.sh
+++ b/t/t4208-log-magic-pathspec.sh
@@ -11,11 +11,24 @@ test_expect_success 'setup' '
mkdir sub
'
-test_expect_success '"git log :/" should be ambiguous' '
- test_must_fail git log :/ 2>error &&
+test_expect_success '"git log :/" should not be ambiguous' '
+ git log :/
+'
+
+test_expect_success '"git log :/a" should be ambiguous (applied both rev and worktree)' '
+ : >a &&
+ test_must_fail git log :/a 2>error &&
grep ambiguous error
'
+test_expect_success '"git log :/a -- " should not be ambiguous' '
+ git log :/a --
+'
+
+test_expect_success '"git log -- :/a" should not be ambiguous' '
+ git log -- :/a
+'
+
test_expect_success '"git log :" should be ambiguous' '
test_must_fail git log : 2>error &&
grep ambiguous error
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 2/2] grep: avoid accepting ambiguous revision
From: Nguyễn Thái Ngọc Duy @ 2013-01-21 13:00 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1358773249-24384-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/grep.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/builtin/grep.c b/builtin/grep.c
index 0e1b6c8..8025964 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -823,6 +823,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
struct object *object = parse_object(sha1);
if (!object)
die(_("bad object %s"), arg);
+ if (!seen_dashdash)
+ verify_non_filename(prefix, arg);
add_object_array(object, arg, &list);
continue;
}
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* Re: git-cvsimport-3 and incremental imports
From: John Keeping @ 2013-01-21 13:27 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20130121124340.GA32219@thyrsus.com>
On Mon, Jan 21, 2013 at 07:43:40AM -0500, Eric S. Raymond wrote:
> John Keeping <john@keeping.me.uk>:
>> I also disagree that cvsps outputs commits *newer* than T since it will
>> also output commits *at* T, which is what I changed with the patch in my
>> previous message.
>
> Ah. OK, that is yet another bug inherited from 2.x - the code doesn't
> match the documented (and correct) behavior. Please send me a patch
> against the cvsps repo, I'll merge it.
Should now be in your inbox.
> > Perhaps it is simplest to just save a CVS_LAST_IMPORT_TIME file in
> > $GIT_DIR and not worry about it any more.
>
> Yes, I think you're right. Trying to carry that information in-band would
> probably doom us to all sorts of bug-prone complications.
I think the only way to do it without needing to save local state in the
Git repository would be to teach cvsps to read a table of refs and times
from its stdin so that we could do something like:
git for-each-ref --format='%(refname)%09%(*authordate:raw)' refs/heads/ |
cvsps -i --branch-times-from-stdin |
git fast-import
Then cvsps could create a hash table from this and use that to decide
whether a patch set is interesting or not.
John
^ permalink raw reply
* Re: GIT get corrupted on lustre
From: Erik Faye-Lund @ 2013-01-21 13:29 UTC (permalink / raw)
To: Eric Chamberland
Cc: Pyeron, Jason J CTR (US), Maxime Boissonneault, Philippe Vaucher,
git@vger.kernel.org, Sébastien Boisvert
In-Reply-To: <50F98B53.9080109@giref.ulaval.ca>
On Fri, Jan 18, 2013 at 6:50 PM, Eric Chamberland
<Eric.Chamberland@giref.ulaval.ca> wrote:
> Good idea!
>
> I did a strace and here is the output with the error:
>
> http://www.giref.ulaval.ca/~ericc/strace_git_error.txt
>
> Hope it will be insightful!
This trace doesn't seem to contain child-processes, but instead having
their stderr inlined into the log. Try using "strace -f" instead...
^ permalink raw reply
* [PATCH] git-for-each-ref.txt: 'raw' is a supported date format
From: John Keeping @ 2013-01-21 13:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Commit 7dff9b3 (Support 'raw' date format) added a raw date format.
Update the git-for-each-ref documentation to include this.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
Documentation/git-for-each-ref.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index db55a4e..d3e1df7 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -117,7 +117,7 @@ returns an empty string instead.
As a special case for the date-type fields, you may specify a format for
the date by adding one of `:default`, `:relative`, `:short`, `:local`,
-`:iso8601` or `:rfc2822` to the end of the fieldname; e.g.
+`:iso8601`, `:rfc2822` or `raw` to the end of the fieldname; e.g.
`%(taggerdate:relative)`.
--
1.8.1
^ permalink raw reply related
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