* Re: [PATCH v2 06/10] sequencer.c: teach append_signoff how to detect duplicate s-o-b
From: Brandon Casey @ 2013-01-28 0:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: gitster, pclouds, git, Brandon Casey
In-Reply-To: <20130122083825.GG6085@elie.Belkin>
On Tue, Jan 22, 2013 at 12:38 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Brandon Casey wrote:
>
>> 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.
> [...]
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -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)
>
> Isn't the behavior of passing '1' here just a bug in "format-patch -s"?
I think that is an open question.
> Style: callers will be easier to read if the function takes a flag
> word with named bits, as in:
>
> #define APPEND_SIGNOFF_DEDUP (1 << 0)
> void append_signoff(..., int ignore_footer, unsigned flag)
Can't I just be lazy!!! Ok, you win. :b
-Brandon
^ permalink raw reply
* [PATCH v2 0/4] Auto-generate mergetool lists
From: David Aguilar @ 2013-01-28 0:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
This is round two of this series.
I think this touched on everything brought up in the code review.
4/4 could use a review as I'm not completely familiar with the
makefile dependencies, though it seems to work correctly.
David Aguilar (4):
mergetool--lib: Simplify command expressions
mergetool--lib: Improve the help text in guess_merge_tool()
mergetool--lib: Add functions for finding available tools
doc: Generate a list of valid merge tools
Documentation/.gitignore | 1 +
Documentation/Makefile | 22 +++++++-
Documentation/diff-config.txt | 13 ++---
Documentation/merge-config.txt | 12 ++---
git-mergetool--lib.sh | 116 ++++++++++++++++++++++-------------------
5 files changed, 96 insertions(+), 68 deletions(-)
--
1.8.0.13.g3ff16bb
^ permalink raw reply
* [PATCH v2 1/4] mergetool--lib: Simplify command expressions
From: David Aguilar @ 2013-01-28 0:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359334346-5879-1-git-send-email-davvid@gmail.com>
Update variable assignments to always use $(command "$arg")
in their RHS instead of "$(command "$arg")" as the latter
is harder to read. Make get_merge_tool_cmd() simpler by
avoiding "echo" and $(command) substitutions completely.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
I reworded the commit message to be more clear.
git-mergetool--lib.sh | 40 ++++++++++++++++------------------------
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 1d0fb12..9a5aae9 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -32,17 +32,10 @@ check_unchanged () {
fi
}
-valid_tool_config () {
- if test -n "$(get_merge_tool_cmd "$1")"
- then
- return 0
- else
- return 1
- fi
-}
-
valid_tool () {
- setup_tool "$1" || valid_tool_config "$1"
+ setup_tool "$1" && return 0
+ cmd=$(get_merge_tool_cmd "$1")
+ test -n "$cmd"
}
setup_tool () {
@@ -96,14 +89,13 @@ setup_tool () {
}
get_merge_tool_cmd () {
- # Prints the custom command for a merge tool
merge_tool="$1"
if diff_mode
then
- echo "$(git config difftool.$merge_tool.cmd ||
- git config mergetool.$merge_tool.cmd)"
+ git config "difftool.$merge_tool.cmd" ||
+ git config "mergetool.$merge_tool.cmd"
else
- echo "$(git config mergetool.$merge_tool.cmd)"
+ git config "mergetool.$merge_tool.cmd"
fi
}
@@ -114,7 +106,7 @@ run_merge_tool () {
GIT_PREFIX=${GIT_PREFIX:-.}
export GIT_PREFIX
- merge_tool_path="$(get_merge_tool_path "$1")" || exit
+ merge_tool_path=$(get_merge_tool_path "$1") || exit
base_present="$2"
status=0
@@ -145,7 +137,7 @@ run_merge_tool () {
# Run a either a configured or built-in diff tool
run_diff_cmd () {
- merge_tool_cmd="$(get_merge_tool_cmd "$1")"
+ merge_tool_cmd=$(get_merge_tool_cmd "$1")
if test -n "$merge_tool_cmd"
then
( eval $merge_tool_cmd )
@@ -158,11 +150,11 @@ run_diff_cmd () {
# Run a either a configured or built-in merge tool
run_merge_cmd () {
- merge_tool_cmd="$(get_merge_tool_cmd "$1")"
+ merge_tool_cmd=$(get_merge_tool_cmd "$1")
if test -n "$merge_tool_cmd"
then
- trust_exit_code="$(git config --bool \
- mergetool."$1".trustExitCode || echo false)"
+ trust_exit_code=$(git config --bool \
+ "mergetool.$1.trustExitCode" || echo false)
if test "$trust_exit_code" = "false"
then
touch "$BACKUP"
@@ -253,7 +245,7 @@ guess_merge_tool () {
# Loop over each candidate and stop when a valid merge tool is found.
for i in $tools
do
- merge_tool_path="$(translate_merge_tool_path "$i")"
+ merge_tool_path=$(translate_merge_tool_path "$i")
if type "$merge_tool_path" >/dev/null 2>&1
then
echo "$i"
@@ -300,9 +292,9 @@ get_merge_tool_path () {
fi
if test -z "$merge_tool_path"
then
- merge_tool_path="$(translate_merge_tool_path "$merge_tool")"
+ merge_tool_path=$(translate_merge_tool_path "$merge_tool")
fi
- if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
+ if test -z $(get_merge_tool_cmd "$merge_tool") &&
! type "$merge_tool_path" >/dev/null 2>&1
then
echo >&2 "The $TOOL_MODE tool $merge_tool is not available as"\
@@ -314,11 +306,11 @@ get_merge_tool_path () {
get_merge_tool () {
# Check if a merge tool has been configured
- merge_tool="$(get_configured_merge_tool)"
+ merge_tool=$(get_configured_merge_tool)
# Try to guess an appropriate merge tool if no tool has been set.
if test -z "$merge_tool"
then
- merge_tool="$(guess_merge_tool)" || exit
+ merge_tool=$(guess_merge_tool) || exit
fi
echo "$merge_tool"
}
--
1.8.0.13.g3ff16bb
^ permalink raw reply related
* [PATCH v2 2/4] mergetool--lib: Improve the help text in guess_merge_tool()
From: David Aguilar @ 2013-01-28 0:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359334346-5879-2-git-send-email-davvid@gmail.com>
This code path is only activated when the user does not have a valid
configured tool. Add a message to guide new users towards configuring a
default tool.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
This now uses a cat << here-doc.
git-mergetool--lib.sh | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 9a5aae9..db3eb58 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -240,8 +240,13 @@ show_tool_help () {
guess_merge_tool () {
list_merge_tool_candidates
- echo >&2 "merge tool candidates: $tools"
+ cat >&2 <<-EOF
+This message is displayed because '$TOOL_MODE.tool' is not configured.
+See 'git ${TOOL_MODE}tool --tool-help' or 'git help config' for more details.
+'git ${TOOL_MODE}tool' will now attempt to use one of the following tools:
+$tools
+EOF
# Loop over each candidate and stop when a valid merge tool is found.
for i in $tools
do
--
1.8.0.13.g3ff16bb
^ permalink raw reply related
* [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools
From: David Aguilar @ 2013-01-28 0:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359334346-5879-3-git-send-email-davvid@gmail.com>
Refactor show_tool_help() so that the tool-finding logic is broken out
into a separate show_tool_names() function.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
filter_tools renamed to show_tool_names() and simplfied
to use ls -1. show_tool_names() now has a preamble as discussed.
git-mergetool--lib.sh | 68 +++++++++++++++++++++++++++++----------------------
1 file changed, 39 insertions(+), 29 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index db3eb58..fe068f6 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -2,6 +2,35 @@
# git-mergetool--lib is a library for common merge tool functions
MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
+mode_ok () {
+ diff_mode && can_diff ||
+ merge_mode && can_merge
+}
+
+is_available () {
+ merge_tool_path=$(translate_merge_tool_path "$1") &&
+ type "$merge_tool_path" >/dev/null 2>&1
+}
+
+show_tool_names () {
+ condition=${1:-true} per_line_prefix=${2:-} preamble=${3:-}
+
+ ( cd "$MERGE_TOOLS_DIR" && ls -1 * ) |
+ while read toolname
+ do
+ if setup_tool "$toolname" 2>/dev/null &&
+ (eval "$condition" "$toolname")
+ then
+ if test -n "$preamble"
+ then
+ echo "$preamble"
+ preamble=
+ fi
+ printf "%s%s\n" "$per_line_prefix" "$tool"
+ fi
+ done
+}
+
diff_mode() {
test "$TOOL_MODE" = diff
}
@@ -199,35 +228,21 @@ list_merge_tool_candidates () {
}
show_tool_help () {
- unavailable= available= LF='
-'
- for i in "$MERGE_TOOLS_DIR"/*
- do
- tool=$(basename "$i")
- setup_tool "$tool" 2>/dev/null || continue
-
- merge_tool_path=$(translate_merge_tool_path "$tool")
- if type "$merge_tool_path" >/dev/null 2>&1
- then
- available="$available$tool$LF"
- else
- unavailable="$unavailable$tool$LF"
- fi
- done
-
- cmd_name=${TOOL_MODE}tool
+ tool_opt="'git ${TOOL_MODE}tool --tool-<tool>'"
+ available=$(show_tool_names 'mode_ok && is_available' '\t\t' \
+ "$tool_opt may be set to one of the following:")
+ unavailable=$(show_tool_names 'mode_ok && ! is_available' '\t\t' \
+ "The following tools are valid, but not currently available:")
if test -n "$available"
then
- echo "'git $cmd_name --tool=<tool>' may be set to one of the following:"
- echo "$available" | sort | sed -e 's/^/ /'
+ echo "$available"
else
echo "No suitable tool for 'git $cmd_name --tool=<tool>' found."
fi
if test -n "$unavailable"
then
echo
- echo 'The following tools are valid, but not currently available:'
- echo "$unavailable" | sort | sed -e 's/^/ /'
+ echo "$unavailable"
fi
if test -n "$unavailable$available"
then
@@ -248,17 +263,12 @@ See 'git ${TOOL_MODE}tool --tool-help' or 'git help config' for more details.
$tools
EOF
# Loop over each candidate and stop when a valid merge tool is found.
- for i in $tools
+ for tool in $tools
do
- merge_tool_path=$(translate_merge_tool_path "$i")
- if type "$merge_tool_path" >/dev/null 2>&1
- then
- echo "$i"
- return 0
- fi
+ is_available "$tool" && echo "$tool" && return 0
done
- echo >&2 "No known merge resolution program available."
+ echo >&2 "No known ${TOOL_MODE} tool is available."
return 1
}
--
1.8.0.13.g3ff16bb
^ permalink raw reply related
* [PATCH v2 4/4] doc: Generate a list of valid merge tools
From: David Aguilar @ 2013-01-28 0:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359334346-5879-4-git-send-email-davvid@gmail.com>
Use the show_tool_names() function to build lists of all
the built-in tools supported by difftool and mergetool.
This frees us from needing to update the documentation
whenever a new tool is added.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Adjusted to use show_tool_names() and reworked the makefile dependencies.
I could use another set of eyes on the Makefile..
Documentation/.gitignore | 1 +
Documentation/Makefile | 22 ++++++++++++++++++++--
Documentation/diff-config.txt | 13 +++++++------
Documentation/merge-config.txt | 12 ++++++------
git-mergetool--lib.sh | 3 ++-
5 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/Documentation/.gitignore b/Documentation/.gitignore
index d62aebd..2c8b2d6 100644
--- a/Documentation/.gitignore
+++ b/Documentation/.gitignore
@@ -9,4 +9,5 @@ gitman.info
howto-index.txt
doc.dep
cmds-*.txt
+mergetools-*.txt
manpage-base-url.xsl
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 267dfe1..834ec25 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -202,7 +202,11 @@ install-html: html
#
# Determine "include::" file references in asciidoc files.
#
-doc.dep : $(wildcard *.txt) build-docdep.perl
+docdep_prereqs = \
+ mergetools-list.made $(mergetools_txt) \
+ cmd-list.made $(cmds_txt)
+
+doc.dep : $(docdep_prereqs) $(wildcard *.txt) build-docdep.perl
$(QUIET_GEN)$(RM) $@+ $@ && \
$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
mv $@+ $@
@@ -226,13 +230,27 @@ cmd-list.made: cmd-list.perl ../command-list.txt $(MAN1_TXT)
$(PERL_PATH) ./cmd-list.perl ../command-list.txt $(QUIET_STDERR) && \
date >$@
+mergetools_txt = mergetools-diff.txt mergetools-merge.txt
+
+$(mergetools_txt): mergetools-list.made
+
+mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
+ $(QUIET_GEN)$(RM) $@ && \
+ $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
+ . ../git-mergetool--lib.sh && \
+ show_tool_names can_diff "* "' > mergetools-diff.txt && \
+ $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
+ . ../git-mergetool--lib.sh && \
+ show_tool_names can_merge "* "' > mergetools-merge.txt && \
+ date > $@
+
clean:
$(RM) *.xml *.xml+ *.html *.html+ *.1 *.5 *.7
$(RM) *.texi *.texi+ *.texi++ git.info gitman.info
$(RM) *.pdf
$(RM) howto-index.txt howto/*.html doc.dep
$(RM) technical/api-*.html technical/api-index.txt
- $(RM) $(cmds_txt) *.made
+ $(RM) $(cmds_txt) $(mergetools_txt) *.made
$(RM) manpage-base-url.xsl
$(MAN_HTML): %.html : %.txt
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 67a90a8..7c968d1 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -132,9 +132,10 @@ diff.<driver>.cachetextconv::
conversion outputs. See linkgit:gitattributes[5] for details.
diff.tool::
- The diff tool to be used by linkgit:git-difftool[1]. This
- option overrides `merge.tool`, and has the same valid built-in
- values as `merge.tool` minus "tortoisemerge" and plus
- "kompare". Any other value is treated as a custom diff tool,
- and there must be a corresponding `difftool.<tool>.cmd`
- option.
+ Controls which diff tool is used by linkgit:git-difftool[1].
+ This variable overrides the value configured in `merge.tool`.
+ The list below shows the valid built-in values.
+ Any other value is treated as a custom diff tool and requires
+ that a corresponding difftool.<tool>.cmd variable is defined.
+
+include::mergetools-diff.txt[]
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index 861bd6f..5f40e71 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -52,12 +52,12 @@ merge.stat::
at the end of the merge. True by default.
merge.tool::
- Controls which merge resolution program is used by
- linkgit:git-mergetool[1]. Valid built-in values are: "araxis",
- "bc3", "diffuse", "ecmerge", "emerge", "gvimdiff", "kdiff3", "meld",
- "opendiff", "p4merge", "tkdiff", "tortoisemerge", "vimdiff"
- and "xxdiff". Any other value is treated is custom merge tool
- and there must be a corresponding mergetool.<tool>.cmd option.
+ Controls which merge tool is used by linkgit:git-mergetool[1].
+ The list below shows the valid built-in values.
+ Any other value is treated as a custom merge tool and requires
+ that a corresponding mergetool.<tool>.cmd variable is defined.
+
+include::mergetools-merge.txt[]
merge.verbosity::
Controls the amount of output shown by the recursive merge
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index fe068f6..f665bee 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -1,6 +1,7 @@
#!/bin/sh
# git-mergetool--lib is a library for common merge tool functions
-MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
+
+: ${MERGE_TOOLS_DIR=$(git --exec-path)/mergetools}
mode_ok () {
diff_mode && can_diff ||
--
1.8.0.13.g3ff16bb
^ permalink raw reply related
* [PATCH v3 00/11] unify appending of sob
From: Brandon Casey @ 2013-01-28 1:11 UTC (permalink / raw)
To: git; +Cc: jrnieder, pclouds, gitster, Brandon Casey
Round 3.
-Brandon
Brandon Casey (9):
sequencer.c: rework search for start of footer to improve clarity
commit, cherry-pick -s: remove broken support for multiline rfc2822
fields
t/test-lib-functions.sh: allow to specify the tag name to test_commit
t/t3511: add some tests of 'cherry-pick -s' functionality
sequencer.c: recognize "(cherry picked from ..." as part of s-o-b
footer
sequencer.c: always separate "(cherry picked from" from commit body
sequencer.c: teach append_signoff how to detect duplicate s-o-b
sequencer.c: teach append_signoff to avoid adding a duplicate newline
Unify appending signoff in format-patch, commit and sequencer
Nguyễn Thái Ngọc Duy (2):
t4014: more tests about appending s-o-b lines
format-patch: update append_signoff prototype
builtin/commit.c | 2 +-
builtin/log.c | 13 +--
log-tree.c | 92 ++---------------
revision.h | 2 +-
sequencer.c | 150 ++++++++++++++++++---------
sequencer.h | 4 +-
t/t3511-cherry-pick-x.sh | 219 +++++++++++++++++++++++++++++++++++++++
t/t4014-format-patch.sh | 263 +++++++++++++++++++++++++++++++++++++++++++++++
t/test-lib-functions.sh | 8 +-
9 files changed, 600 insertions(+), 153 deletions(-)
create mode 100755 t/t3511-cherry-pick-x.sh
--
1.8.1.1.450.g0327af3
^ permalink raw reply
* [PATCH v3 01/11] sequencer.c: rework search for start of footer to improve clarity
From: Brandon Casey @ 2013-01-28 1:11 UTC (permalink / raw)
To: git; +Cc: jrnieder, pclouds, gitster, Brandon Casey
In-Reply-To: <1359335515-13818-1-git-send-email-drafnel@gmail.com>
This code sequence is somewhat difficult to read. Let's rewrite it
using more descriptive variable names to try to make it easier to
understand.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
sequencer.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index aef5e8a..cd211b2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1024,16 +1024,19 @@ int sequencer_pick_revisions(struct replay_opts *opts)
static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
{
int ch;
- int hit = 0;
+ int last_char_was_nl, this_char_is_nl;
int i, j, k;
int len = sb->len - ignore_footer;
int first = 1;
const char *buf = sb->buf;
+ /* find start of last paragraph */
+ last_char_was_nl = 0;
for (i = len - 1; i > 0; i--) {
- if (hit && buf[i] == '\n')
+ this_char_is_nl = (buf[i] == '\n');
+ if (last_char_was_nl && this_char_is_nl)
break;
- hit = (buf[i] == '\n');
+ last_char_was_nl = this_char_is_nl;
}
while (i < len - 1 && buf[i] == '\n')
--
1.8.1.1.450.g0327af3
^ permalink raw reply related
* [PATCH v3 02/11] commit, cherry-pick -s: remove broken support for multiline rfc2822 fields
From: Brandon Casey @ 2013-01-28 1:11 UTC (permalink / raw)
To: git; +Cc: jrnieder, pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1359335515-13818-1-git-send-email-drafnel@gmail.com>
Starting with c1e01b0c (commit: More generous accepting of RFC-2822 footer
lines, 2009-10-28), "git commit -s" carefully parses the last paragraph of
each commit message to check if it consists only of RFC2822-style headers,
in which case the signoff will be added as a new line in the same list:
Reported-by: Reporter <reporter@example.com>
Signed-off-by: Author <author@example.com>
Acked-by: Lieutenant <lt@example.com>
It even included support for accepting indented continuation lines for
multiline fields. Unfortunately the multiline field support is broken
because it checks whether buf[k] (the first character of the *next* line)
instead of buf[i] is a whitespace character. The result is that any footer
with a continuation line is not accepted, since the last continuation line
neither starts with an RFC2822 field name nor is followed by a continuation
line.
That this has remained broken for so long is good evidence that nobody
actually needed multiline fields. Rip out the broken continuation support.
There should be no functional change.
[Thanks to Jonathan Nieder for the excellent commit message]
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 cd211b2..39a752b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1027,7 +1027,6 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
int last_char_was_nl, this_char_is_nl;
int i, j, k;
int len = sb->len - ignore_footer;
- int first = 1;
const char *buf = sb->buf;
/* find start of last paragraph */
@@ -1047,11 +1046,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.450.g0327af3
^ permalink raw reply related
* [PATCH v3 03/11] t/test-lib-functions.sh: allow to specify the tag name to test_commit
From: Brandon Casey @ 2013-01-28 1:11 UTC (permalink / raw)
To: git; +Cc: jrnieder, pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1359335515-13818-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>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
---
t/test-lib-functions.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fa62d01..61d0804 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -135,12 +135,12 @@ 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, and tag the resulting commit with the given tag name.
#
-# Both <file> and <contents> default to <message>.
+# <file>, <contents>, and <tag> all default to <message>.
test_commit () {
notick= &&
@@ -168,7 +168,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.450.g0327af3
^ permalink raw reply related
* [PATCH v3 04/11] t/t3511: add some tests of 'cherry-pick -s' functionality
From: Brandon Casey @ 2013-01-28 1:11 UTC (permalink / raw)
To: git; +Cc: jrnieder, pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1359335515-13818-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..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.1.450.g0327af3
^ permalink raw reply related
* [PATCH v3 05/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
From: Brandon Casey @ 2013-01-28 1:11 UTC (permalink / raw)
To: git; +Cc: jrnieder, pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1359335515-13818-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>
Acked-by: Jonathan Nieder <jrnieder@gmail.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 39a752b..0b5cd18 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 last_char_was_nl, this_char_is_nl;
- int i, j, k;
+ int i, k;
int len = sb->len - ignore_footer;
const char *buf = sb->buf;
@@ -1046,15 +1072,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;
}
@@ -1071,7 +1091,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.1.450.g0327af3
^ permalink raw reply related
* [PATCH v3 06/11] sequencer.c: always separate "(cherry picked from" from commit body
From: Brandon Casey @ 2013-01-28 1:11 UTC (permalink / raw)
To: git; +Cc: jrnieder, pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1359335515-13818-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.
This commit is mostly a code movement, but notice that
has_conforming_footer() was modified, in addition to being moved.
Introduce tests to test this functionality.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
sequencer.c | 120 +++++++++++++++++++++++++----------------------
t/t3511-cherry-pick-x.sh | 53 +++++++++++++++++++++
2 files changed, 116 insertions(+), 57 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 0b5cd18..46d51b2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -20,6 +20,67 @@
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 last_char_was_nl, this_char_is_nl;
+ int i, k;
+ int len = sb->len - ignore_footer;
+ const char *buf = sb->buf;
+
+ /* find start of last paragraph */
+ last_char_was_nl = 0;
+ for (i = len - 1; i > 0; i--) {
+ this_char_is_nl = (buf[i] == '\n');
+ if (last_char_was_nl && this_char_is_nl)
+ break;
+ last_char_was_nl = this_char_is_nl;
+ }
+
+ /* require at least one blank line */
+ if (!last_char_was_nl || 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 +558,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,63 +1085,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 last_char_was_nl, this_char_is_nl;
- int i, k;
- int len = sb->len - ignore_footer;
- const char *buf = sb->buf;
-
- /* find start of last paragraph */
- last_char_was_nl = 0;
- for (i = len - 1; i > 0; i--) {
- this_char_is_nl = (buf[i] == '\n');
- if (last_char_was_nl && this_char_is_nl)
- break;
- last_char_was_nl = this_char_is_nl;
- }
-
- 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 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.1.450.g0327af3
^ permalink raw reply related
* [PATCH v3 08/11] sequencer.c: teach append_signoff to avoid adding a duplicate newline
From: Brandon Casey @ 2013-01-28 1:11 UTC (permalink / raw)
To: git; +Cc: jrnieder, pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1359335515-13818-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 015cb58..404b786 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1118,11 +1118,15 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
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.450.g0327af3
^ permalink raw reply related
* [PATCH v3 07/11] sequencer.c: teach append_signoff how to detect duplicate s-o-b
From: Brandon Casey @ 2013-01-28 1:11 UTC (permalink / raw)
To: git; +Cc: jrnieder, pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1359335515-13818-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 | 47 +++++++++++++++++++++++++++++++++++++----------
sequencer.h | 4 +++-
t/t3511-cherry-pick-x.sh | 2 +-
4 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 38b9a9c..67ea9e7 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 46d51b2..015cb58 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 last_char_was_nl, this_char_is_nl;
int i, k;
int len = sb->len - ignore_footer;
const char *buf = sb->buf;
+ int found_sob = 0;
/* find start of last paragraph */
last_char_was_nl = 0;
@@ -70,14 +78,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;
}
@@ -299,7 +318,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;
@@ -558,7 +577,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));
@@ -1085,9 +1104,11 @@ 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 has_footer = 0;
int i;
strbuf_addstr(&sob, sign_off_header);
@@ -1096,10 +1117,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..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.1.450.g0327af3
^ permalink raw reply related
* [PATCH v3 09/11] t4014: more tests about appending s-o-b lines
From: Brandon Casey @ 2013-01-28 1:11 UTC (permalink / raw)
To: git; +Cc: jrnieder, pclouds, gitster, Brandon Casey
In-Reply-To: <1359335515-13818-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.450.g0327af3
^ permalink raw reply related
* [PATCH v3 10/11] format-patch: update append_signoff prototype
From: Brandon Casey @ 2013-01-28 1:11 UTC (permalink / raw)
To: git; +Cc: jrnieder, pclouds, gitster, Brandon Casey
In-Reply-To: <1359335515-13818-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 | 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.1.450.g0327af3
^ permalink raw reply related
* [PATCH v3 11/11] Unify appending signoff in format-patch, commit and sequencer
From: Brandon Casey @ 2013-01-28 1:11 UTC (permalink / raw)
To: git; +Cc: jrnieder, pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1359335515-13818-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 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.450.g0327af3
^ permalink raw reply related
* [PATCH v3 1/4] branch: no detached HEAD check when editing another branch's description
From: Nguyễn Thái Ngọc Duy @ 2013-01-28 1:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matthieu Moy, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1359118225-14356-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Split out from v3's 2/3 as Jonathan suggested.
builtin/branch.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..ea6498b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -850,11 +850,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
const char *branch_name;
struct strbuf branch_ref = STRBUF_INIT;
- if (detached)
- die("Cannot give description to detached HEAD");
- if (!argc)
+ if (!argc) {
+ if (detached)
+ die("Cannot give description to detached HEAD");
branch_name = head;
- else if (argc == 1)
+ } else if (argc == 1)
branch_name = argv[0];
else
usage_with_options(builtin_branch_usage, options);
--
1.8.1.1.459.g5970e58
^ permalink raw reply related
* [PATCH v3 3/4] branch: give a more helpful message on redundant arguments
From: Nguyễn Thái Ngọc Duy @ 2013-01-28 1:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matthieu Moy, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1359335896-24406-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/branch.c | 4 ++--
t/t3200-branch.sh | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 30c4545..ca61c5b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -859,7 +859,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
} else if (argc == 1)
branch_name = argv[0];
else
- usage_with_options(builtin_branch_usage, options);
+ die(_("cannot edit description of more than one branch"));
strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
if (!ref_exists(branch_ref.buf)) {
@@ -881,7 +881,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
else if (argc == 2)
rename_branch(argv[0], argv[1], rename > 1);
else
- usage_with_options(builtin_branch_usage, options);
+ die(_("too many branches for a rename operation"));
} else if (new_upstream) {
struct branch *branch = branch_get(argv[0]);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 80e6be3..f3e0e4a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -73,8 +73,8 @@ test_expect_success \
test_expect_success \
'git branch -m dumps usage' \
- 'test_expect_code 129 git branch -m 2>err &&
- test_i18ngrep "[Uu]sage: git branch" err'
+ 'test_expect_code 128 git branch -m 2>err &&
+ test_i18ngrep "too many branches for a rename operation" err'
test_expect_success \
'git branch -m m m/m should work' \
--
1.8.1.1.459.g5970e58
^ permalink raw reply related
* [PATCH v3 2/4] branch: reject -D/-d without branch name
From: Nguyễn Thái Ngọc Duy @ 2013-01-28 1:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matthieu Moy, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1359335896-24406-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/branch.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index ea6498b..30c4545 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -837,9 +837,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
colopts = 0;
}
- if (delete)
+ if (delete) {
+ if (!argc)
+ die(_("branch name required"));
return delete_branches(argc, argv, delete > 1, kinds, quiet);
- else if (list) {
+ } else if (list) {
int ret = print_ref_list(kinds, detached, verbose, abbrev,
with_commit, argv);
print_columns(&output, colopts, NULL);
--
1.8.1.1.459.g5970e58
^ permalink raw reply related
* [PATCH v3 4/4] branch: mark more strings for translation
From: Nguyễn Thái Ngọc Duy @ 2013-01-28 1:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matthieu Moy, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1359335896-24406-1-git-send-email-pclouds@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
On Sun, Jan 27, 2013 at 6:55 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> For what it's worth, assuming this passes tests,
It passes the tests. Although I doubt the tests are written to catch
this.
builtin/branch.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index ca61c5b..597b578 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -466,7 +466,7 @@ static void add_verbose_info(struct strbuf *out, struct ref_item *item,
int verbose, int abbrev)
{
struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
- const char *sub = " **** invalid ref ****";
+ const char *sub = _(" **** invalid ref ****");
struct commit *commit = item->commit;
if (commit && !parse_commit(commit)) {
@@ -590,7 +590,7 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
struct commit *filter;
filter = lookup_commit_reference_gently(merge_filter_ref, 0);
if (!filter)
- die("object '%s' does not point to a commit",
+ die(_("object '%s' does not point to a commit"),
sha1_to_hex(merge_filter_ref));
filter->object.flags |= UNINTERESTING;
@@ -854,7 +854,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!argc) {
if (detached)
- die("Cannot give description to detached HEAD");
+ die(_("Cannot give description to detached HEAD"));
branch_name = head;
} else if (argc == 1)
branch_name = argv[0];
@@ -866,10 +866,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
strbuf_release(&branch_ref);
if (!argc)
- return error("No commit on branch '%s' yet.",
+ return error(_("No commit on branch '%s' yet."),
branch_name);
else
- return error("No such branch '%s'.", branch_name);
+ return error(_("No branch named '%s'."),
+ branch_name);
}
strbuf_release(&branch_ref);
--
1.8.1.1.459.g5970e58
^ permalink raw reply related
* Re: [PATCH v2 04/10] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
From: Jonathan Nieder @ 2013-01-28 1:24 UTC (permalink / raw)
To: Brandon Casey; +Cc: gitster, pclouds, git, Brandon Casey
In-Reply-To: <1358757627-16682-5-git-send-email-drafnel@gmail.com>
Hi,
Brandon Casey wrote:
> 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>
My last review was based on a misunderstanding of
ends_rfc2822_footer(). This time I can unreservedly say I like this.
[...]
> --- 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 ";
The static/nonstatic mismatch looks strange. Not about this patch,
but probably rest_is_empty() from builtin-commit.c should move here
some day.
[...]
> @@ -1021,11 +1022,36 @@ int sequencer_pick_revisions(struct replay_opts *opts)
[...]
> +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] == ')'));
These two cases are based on whether the commit message ended with
a '(cherry picked from' with no trailing newline, I guess?
I wonder if switching the convention for 'k' would make this simpler:
const char *line, *eol;
line = buf + i;
eol = memchr(buf, '\n', len - i);
if (!eol)
eol = buf + len;
if (!is_rfc2822_field(line, eol - line) &&
!is_cherry_picked_from_line(line, eol - line))
return 0;
I notice you just sent a new version of the series. I'll try this out
on top of that.
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH v3 01/11] sequencer.c: rework search for start of footer to improve clarity
From: Jonathan Nieder @ 2013-01-28 1:27 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, pclouds, gitster
In-Reply-To: <1359335515-13818-2-git-send-email-drafnel@gmail.com>
Brandon Casey wrote:
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1024,16 +1024,19 @@ int sequencer_pick_revisions(struct replay_opts *opts)
> static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
> {
> int ch;
> - int hit = 0;
> + int last_char_was_nl, this_char_is_nl;
> int i, j, k;
> int len = sb->len - ignore_footer;
> int first = 1;
> const char *buf = sb->buf;
>
> + /* find start of last paragraph */
> + last_char_was_nl = 0;
> for (i = len - 1; i > 0; i--) {
> - if (hit && buf[i] == '\n')
> + this_char_is_nl = (buf[i] == '\n');
> + if (last_char_was_nl && this_char_is_nl)
> break;
> - hit = (buf[i] == '\n');
> + last_char_was_nl = this_char_is_nl;
I would have been tempted to write
char prev;
prev = 0;
for (i = len - 1; i > 0; i--) {
char ch = buf[i];
if (prev == '\n' && ch == '\n') /* paragraph break */
break;
prev = ch;
}
but your rewrite is just as clear. For what it's worth,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
^ permalink raw reply
* Re: [PATCH v3 02/11] commit, cherry-pick -s: remove broken support for multiline rfc2822 fields
From: Jonathan Nieder @ 2013-01-28 1:29 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, pclouds, gitster, Brandon Casey
In-Reply-To: <1359335515-13818-3-git-send-email-drafnel@gmail.com>
Brandon Casey wrote:
> sequencer.c | 6 ------
> 1 file changed, 6 deletions(-)
Nice simplification.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
^ 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