From 8af69cf3e214ee9df3f78bd508465b75881b17a8 Mon Sep 17 00:00:00 2001 From: Doan Tran Cong Danh Date: Wed, 16 Oct 2019 12:18:40 +0700 Subject: [PATCH 1/2] t3301: test diagnose messages for too few/many paramters Commit bbb1b8a35a ("notes: check number of parameters to "git notes copy"", 2010-06-28) added a test for too many or too few of parameters provided to `git notes copy'. However, the test only ensures that the command will fail but it doesn't really check if it fails because of number of parameters. If we accidentally lifted the check inside our code base, the test may still have failed because the provided parameter is not a valid ref. Correct it. Signed-off-by: Doan Tran Cong Danh Signed-off-by: Junio C Hamano --- t/t3301-notes.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh index d3fa298c6a..d7767e4438 100755 --- a/t/t3301-notes.sh +++ b/t/t3301-notes.sh @@ -1167,8 +1167,10 @@ test_expect_success 'GIT_NOTES_REWRITE_REF overrides config' ' ' test_expect_success 'git notes copy diagnoses too many or too few parameters' ' - test_must_fail git notes copy && - test_must_fail git notes copy one two three + test_must_fail git notes copy 2>error && + test_i18ngrep "too few parameters" error && + test_must_fail git notes copy one two three 2>error && + test_i18ngrep "too many parameters" error ' test_expect_success 'git notes get-ref expands refs/heads/master to refs/notes/refs/heads/master' ' From d58deb9c4e151d4d8380cd14223391ce0d58f588 Mon Sep 17 00:00:00 2001 From: Doan Tran Cong Danh Date: Wed, 16 Oct 2019 12:18:41 +0700 Subject: [PATCH 2/2] notes: fix minimum number of parameters to "copy" subcommand The builtin/notes.c::copy() function is prepared to handle either one or two arguments given from the command line; when one argument is given, to-obj defaults to HEAD. bbb1b8a3 ("notes: check number of parameters to "git notes copy"", 2010-06-28) tried to make sure "git notes copy" (with *no* other argument) does not dereference NULL by checking the number of parameters, but it incorrectly insisted that we need two arguments, instead of either one or two. This disabled the defaulting to-obj to HEAD. Correct it. Signed-off-by: Doan Tran Cong Danh Signed-off-by: Junio C Hamano --- Documentation/git-notes.txt | 6 +++--- builtin/notes.c | 2 +- t/t3301-notes.sh | 40 +++++++++++++++++++++++++++++++++++-- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt index f56a5a9197..ced2e8280e 100644 --- a/Documentation/git-notes.txt +++ b/Documentation/git-notes.txt @@ -10,7 +10,7 @@ SYNOPSIS [verse] 'git notes' [list []] 'git notes' add [-f] [--allow-empty] [-F | -m | (-c | -C) ] [] -'git notes' copy [-f] ( --stdin | ) +'git notes' copy [-f] ( --stdin | [] ) 'git notes' append [--allow-empty] [-F | -m | (-c | -C) ] [] 'git notes' edit [--allow-empty] [] 'git notes' show [] @@ -68,8 +68,8 @@ add:: subcommand). copy:: - Copy the notes for the first object onto the second object. - Abort if the second object already has notes, or if the first + Copy the notes for the first object onto the second object (defaults to + HEAD). Abort if the second object already has notes, or if the first object has none (use -f to overwrite existing notes to the second object). This subcommand is equivalent to: `git notes add [-f] -C $(git notes list ) ` diff --git a/builtin/notes.c b/builtin/notes.c index 02e97f55c5..95456f3165 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -513,7 +513,7 @@ static int copy(int argc, const char **argv, const char *prefix) } } - if (argc < 2) { + if (argc < 1) { error(_("too few parameters")); usage_with_options(git_notes_copy_usage, options); } diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh index d7767e4438..d66a5f6faa 100755 --- a/t/t3301-notes.sh +++ b/t/t3301-notes.sh @@ -864,6 +864,24 @@ test_expect_success 'append to note from other note with "git notes append -c"' ' test_expect_success 'copy note with "git notes copy"' ' + commit=$(git rev-parse 4th) && + cat >expect <<-EOF && + commit $commit + Author: A U Thor + Date: Thu Apr 7 15:16:13 2005 -0700 + + ${indent}4th + + Notes: + ${indent}This is a blob object + EOF + git notes copy 8th 4th && + git log 3rd..4th >actual && + test_cmp expect actual && + test "$(git note list 4th)" = "$(git note list 8th)" +' + +test_expect_success 'copy note with "git notes copy" with default' ' test_commit 11th && commit=$(git rev-parse HEAD) && cat >expect <<-EOF && @@ -878,7 +896,7 @@ test_expect_success 'copy note with "git notes copy"' ' ${indent} ${indent}yet another note EOF - git notes copy HEAD^ HEAD && + git notes copy HEAD^ && git log -1 >actual && test_cmp expect actual && test "$(git notes list HEAD)" = "$(git notes list HEAD^)" @@ -892,6 +910,24 @@ test_expect_success 'prevent overwrite with "git notes copy"' ' ' test_expect_success 'allow overwrite with "git notes copy -f"' ' + commit=$(git rev-parse HEAD) && + cat >expect <<-EOF && + commit $commit + Author: A U Thor + Date: Thu Apr 7 15:23:13 2005 -0700 + + ${indent}11th + + Notes: + ${indent}This is a blob object + EOF + git notes copy -f HEAD~3 HEAD && + git log -1 >actual && + test_cmp expect actual && + test "$(git notes list HEAD)" = "$(git notes list HEAD~3)" +' + +test_expect_success 'allow overwrite with "git notes copy -f" with default' ' commit=$(git rev-parse HEAD) && cat >expect <<-EOF && commit $commit @@ -905,7 +941,7 @@ test_expect_success 'allow overwrite with "git notes copy -f"' ' ${indent} ${indent}yet another note EOF - git notes copy -f HEAD~2 HEAD && + git notes copy -f HEAD~2 && git log -1 >actual && test_cmp expect actual && test "$(git notes list HEAD)" = "$(git notes list HEAD~2)"