{"thread":{"id":"52040","subject":"[PATCH 1/1] notes: copy notes to HEAD by default","startedAt":"2019-10-15T16:37:16Z","lastAt":"2019-10-18T00:45:08Z","messageCount":8,"participants":["Doan Tran Cong Danh","Junio C Hamano","Danh Doan"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"384105","messageId":"20191015163631.21808-2-congdanhqx@gmail.com","threadId":"52040","inReplyTo":"20191015163631.21808-1-congdanhqx@gmail.com","subject":"[PATCH 1/1] notes: copy notes to HEAD by default","fromName":"Doan Tran Cong Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2019-10-15T16:36:31Z","receivedAt":"2019-10-15T16:37:16Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"The target objects for copying notes was defaulted to HEAD from very\nearly stage of git-notes.\n\nHowever, that default was limited by commit bbb1b8a35a, (\"notes: check\nnumber of parameters to \"git notes copy\"\", 2010-06-28).\n\nLift that limitation by adjust the check for numbers of arguments.\n\nSigned-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com>\n---\n Documentation/git-notes.txt | 6 +++---\n builtin/notes.c             | 2 +-\n t/t3301-notes.sh            | 4 ++++\n 3 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\nindex f56a5a9197..ced2e8280e 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n [verse]\n 'git notes' [list [<object>]]\n 'git notes' add [-f] [--allow-empty] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]\n-'git notes' copy [-f] ( --stdin | <from-object> <to-object> )\n+'git notes' copy [-f] ( --stdin | <from-object> [<to-object>] )\n 'git notes' append [--allow-empty] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]\n 'git notes' edit [--allow-empty] [<object>]\n 'git notes' show [<object>]\n@@ -68,8 +68,8 @@ add::\n \tsubcommand).\n \n copy::\n-\tCopy the notes for the first object onto the second object.\n-\tAbort if the second object already has notes, or if the first\n+\tCopy the notes for the first object onto the second object (defaults to\n+\tHEAD). Abort if the second object already has notes, or if the first\n \tobject has none (use -f to overwrite existing notes to the\n \tsecond object). This subcommand is equivalent to:\n \t`git notes add [-f] -C $(git notes list <from-object>) <to-object>`\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 02e97f55c5..95456f3165 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -513,7 +513,7 @@ static int copy(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\tif (argc < 2) {\n+\tif (argc < 1) {\n \t\terror(_(\"too few parameters\"));\n \t\tusage_with_options(git_notes_copy_usage, options);\n \t}\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex d3fa298c6a..a8f9a0f36c 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -908,6 +908,10 @@ test_expect_success 'allow overwrite with \"git notes copy -f\"' '\n \tgit notes copy -f HEAD~2 HEAD &&\n \tgit log -1 >actual &&\n \ttest_cmp expect actual &&\n+\ttest \"$(git notes list HEAD)\" = \"$(git notes list HEAD~2)\" &&\n+\tgit notes copy -f HEAD~2 &&\n+\tgit log -1 >actual &&\n+\ttest_cmp expect actual &&\n \ttest \"$(git notes list HEAD)\" = \"$(git notes list HEAD~2)\"\n '\n \n-- \n2.23.0\n\n"},{"id":"384107","messageId":"20191015163631.21808-1-congdanhqx@gmail.com","threadId":"52040","inReplyTo":null,"subject":"[PATCH 0/1] Allow default value for target of git-notes copy","fromName":"Doan Tran Cong Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2019-10-15T16:36:30Z","receivedAt":"2019-10-15T16:44:10Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"As an alternate for the incoming patch,\nthis snippet could be used to eliminate some dead code in builtin/notes.c\nSince `argc' will always be 2 in the removed condition.\n\nAlso:\n\n\nSigned-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com>\n---\n\n builtin/notes.c | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 02e97f55c5..641ae66f75 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -488,7 +488,6 @@ static int copy(int argc, const char **argv, const char *prefix)\n {\n \tint retval = 0, force = 0, from_stdin = 0;\n \tconst struct object_id *from_note, *note;\n-\tconst char *object_ref;\n \tstruct object_id object, from_obj;\n \tstruct notes_tree *t;\n \tconst char *rewrite_cmd = NULL;\n@@ -525,10 +524,8 @@ static int copy(int argc, const char **argv, const char *prefix)\n \tif (get_oid(argv[0], &from_obj))\n \t\tdie(_(\"failed to resolve '%s' as a valid ref.\"), argv[0]);\n \n-\tobject_ref = 1 < argc ? argv[1] : \"HEAD\";\n-\n-\tif (get_oid(object_ref, &object))\n-\t\tdie(_(\"failed to resolve '%s' as a valid ref.\"), object_ref);\n+\tif (get_oid(argv[1], &object))\n+\t\tdie(_(\"failed to resolve '%s' as a valid ref.\"), argv[1]);\n \n \tt = init_notes_check(\"copy\", NOTES_INIT_WRITABLE);\n \tnote = get_note(t, &object);\n-- \n2.23.0\n\n"},{"id":"384152","messageId":"xmqqpnixa1oh.fsf@gitster-ct.c.googlers.com","threadId":"52040","inReplyTo":"20191015163631.21808-2-congdanhqx@gmail.com","subject":"Re: [PATCH 1/1] notes: copy notes to HEAD by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-16T02:01:34Z","receivedAt":"2019-10-16T02:01:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Doan Tran Cong Danh <congdanhqx@gmail.com> writes:\n\n> The target objects for copying notes was defaulted to HEAD from very\n> early stage of git-notes.\n>\n> However, that default was limited by commit bbb1b8a35a, (\"notes: check\n> number of parameters to \"git notes copy\"\", 2010-06-28).\n\nSorry, I don't quite get the above.  The said commit made sure 'git\nnotes copy' gets the right number of arguments, saying \"\"\"Otherwise\nwe may segfault with too few parameters.\"\"\"  I take that as a sign\nthat before that commit it was not defaulting to HEAD but attempting\nto access the missing argv[2] (or whatever the index the <to-object>\nshould be at) and dereferencing a NULL?\n\n    ... goes and digs ...\n\nI think v1.6.6.1-458-g74884b524e is the commit that made the command\nline parsing into the current shape, i.e. one parse_options() call\nin each of the subcommand that gets dispatched, and you are right\nthat with that version a single argument given on the command line\nis taken as the <from-object> and <to-object> defaults to HEAD.\n\nSo... what happend between that vesrion and v1.7.1-200-gbbb1b8a35a?\n\n    ... goes and looks at bbb1b8a35a again ...\n\nAh, I think there is an off-by-one.  When not from-stdin and not\nusing rewrite-cmd, before that patch, we did not even check if\nfrom-obj exists, so in that sense, the commit had a right idea that\nit must check for \"too few parameters\", but it shouldn't have\ninsisted that we have at least two.  It is OK to have just one,\ni.e. only the from-obj, for our purpose.\n\n>  copy::\n> -\tCopy the notes for the first object onto the second object.\n> -\tAbort if the second object already has notes, or if the first\n> +\tCopy the notes for the first object onto the second object (defaults to\n> +\tHEAD). Abort if the second object already has notes, or if the first\n>  \tobject has none (use -f to overwrite existing notes to the\n>  \tsecond object). This subcommand is equivalent to:\n>  \t`git notes add [-f] -C $(git notes list <from-object>) <to-object>`\n\nThis is OK.\n\n> diff --git a/builtin/notes.c b/builtin/notes.c\n> index 02e97f55c5..95456f3165 100644\n> --- a/builtin/notes.c\n> +++ b/builtin/notes.c\n> @@ -513,7 +513,7 @@ static int copy(int argc, const char **argv, const char *prefix)\n>  \t\t}\n>  \t}\n>  \n> -\tif (argc < 2) {\n> +\tif (argc < 1) {\n>  \t\terror(_(\"too few parameters\"));\n>  \t\tusage_with_options(git_notes_copy_usage, options);\n>  \t}\n\nSo is this.\n\n> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\n> index d3fa298c6a..a8f9a0f36c 100755\n> --- a/t/t3301-notes.sh\n> +++ b/t/t3301-notes.sh\n> @@ -908,6 +908,10 @@ test_expect_success 'allow overwrite with \"git notes copy -f\"' '\n>  \tgit notes copy -f HEAD~2 HEAD &&\n>  \tgit log -1 >actual &&\n>  \ttest_cmp expect actual &&\n> +\ttest \"$(git notes list HEAD)\" = \"$(git notes list HEAD~2)\" &&\n> +\tgit notes copy -f HEAD~2 &&\n> +\tgit log -1 >actual &&\n> +\ttest_cmp expect actual &&\n>  \ttest \"$(git notes list HEAD)\" = \"$(git notes list HEAD~2)\"\n>  '\n\nThis I am not sure is a good test to add to, especially as a fix to\nbbb1b8a, which added this test:\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 64f32ad94d..2d67a40fc1 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -1044,4 +1044,10 @@ test_expect_success 'GIT_NOTES_REWRITE_REF overrides config' '\n \tgit log -1 > output &&\n \ttest_cmp expect output\n '\n+\n+test_expect_success 'git notes copy diagnoses too many or too few parameters' '\n+\ttest_must_fail git notes copy &&\n+\ttest_must_fail git notes copy one two three\n+'\n+\n test_done\n\nThe lack of testing that \"git notes copy <from-obj>\" succeeding is\nwhy the off-by-one bug was not noticed, so I think that test (which\nstill exists to this day) is the right place to add a test to\nprotect this fix.\n\nAs to the log message, here is how I would explain/justify the\nchange, if I were writing it.\n\n\tnotes: fix minimum number of parameters to \"copy\" subcommand\n\n\tThe builtin/notes.::copy() function is prepared to handle\n\tonly one argument given from the command line; in such a\n\tcase, to-obj defaults to HEAD.\n\n\tWhen bbb1b8a3 (\"notes: check number of parameters to \"git\n\tnotes copy\"\", 2010-06-28) tried to make sure \"git notes\n\tcopy\" (with *no* other argument) does not dereference NULL\n\tby making sure we did not get too few parameters, it\n\tincorrectly insisted that we get two arguments, instead of\n\tjust one.  This disabled the defaulting to-obj to HEAD.\n\tCorrect it.\n\nThanks.\n"},{"id":"384161","messageId":"20191016051718.GA2034@danh.dev","threadId":"52040","inReplyTo":"xmqqpnixa1oh.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] notes: copy notes to HEAD by default","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2019-10-16T05:17:18Z","receivedAt":"2019-10-16T05:17:23Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2019-10-16 11:01:34 +0900, Junio C Hamano wrote:\n> Doan Tran Cong Danh <congdanhqx@gmail.com> writes:\n> \n> > The target objects for copying notes was defaulted to HEAD from very\n> > early stage of git-notes.\n> >\n> > However, that default was limited by commit bbb1b8a35a, (\"notes: check\n> > number of parameters to \"git notes copy\"\", 2010-06-28).\n> \n> Sorry, I don't quite get the above.  The said commit made sure 'git\n> notes copy' gets the right number of arguments, saying \"\"\"Otherwise\n> we may segfault with too few parameters.\"\"\"  I take that as a sign\n> that before that commit it was not defaulting to HEAD but attempting\n> to access the missing argv[2] (or whatever the index the <to-object>\n> should be at) and dereferencing a NULL?\n> \n>     ... goes and digs ...\n> \n> I think v1.6.6.1-458-g74884b524e is the commit that made the command\n> line parsing into the current shape, i.e. one parse_options() call\n> in each of the subcommand that gets dispatched, and you are right\n> that with that version a single argument given on the command line\n> is taken as the <from-object> and <to-object> defaults to HEAD.\n> \n> So... what happend between that vesrion and v1.7.1-200-gbbb1b8a35a?\n> \n>     ... goes and looks at bbb1b8a35a again ...\n> \n> Ah, I think there is an off-by-one.  When not from-stdin and not\n> using rewrite-cmd, before that patch, we did not even check if\n> from-obj exists, so in that sense, the commit had a right idea that\n> it must check for \"too few parameters\", but it shouldn't have\n> insisted that we have at least two.  It is OK to have just one,\n> i.e. only the from-obj, for our purpose.\n\nYes, this is my intention.\n\n> > diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\n> > index d3fa298c6a..a8f9a0f36c 100755\n> > --- a/t/t3301-notes.sh\n> > +++ b/t/t3301-notes.sh\n> > @@ -908,6 +908,10 @@ test_expect_success 'allow overwrite with \"git notes copy -f\"' '\n> >  \tgit notes copy -f HEAD~2 HEAD &&\n> >  \tgit log -1 >actual &&\n> >  \ttest_cmp expect actual &&\n> > +\ttest \"$(git notes list HEAD)\" = \"$(git notes list HEAD~2)\" &&\n> > +\tgit notes copy -f HEAD~2 &&\n> > +\tgit log -1 >actual &&\n> > +\ttest_cmp expect actual &&\n> >  \ttest \"$(git notes list HEAD)\" = \"$(git notes list HEAD~2)\"\n> >  '\n> \n> This I am not sure is a good test to add to, especially as a fix to\n\nI was writing this patch just before my bed time, just to get some\ncomments on the directions, e.g:\n- loosen the argc requirements; or\n- do the code cleanup\n\n> bbb1b8a, which added this test:\n> \n> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\n> index 64f32ad94d..2d67a40fc1 100755\n> --- a/t/t3301-notes.sh\n> +++ b/t/t3301-notes.sh\n> @@ -1044,4 +1044,10 @@ test_expect_success 'GIT_NOTES_REWRITE_REF overrides config' '\n>  \tgit log -1 > output &&\n>  \ttest_cmp expect output\n>  '\n> +\n> +test_expect_success 'git notes copy diagnoses too many or too few parameters' '\n> +\ttest_must_fail git notes copy &&\n> +\ttest_must_fail git notes copy one two three\n> +'\n> +\n>  test_done\n> \n> The lack of testing that \"git notes copy <from-obj>\" succeeding is\n> why the off-by-one bug was not noticed, so I think that test (which\n> still exists to this day) is the right place to add a test to\n> protect this fix.\n\nI don't think this is a good place to add this test either,\nsince the test description specificaly said it diagnoses too many or\ntoo few parameters.\n\nAnyway, the test `git notes copy one two three` still fails if we\naccidentally allow 3 arguments since one two three isn't valid ref.\nI'm gonna add a statement to assert the diagnose message.\n\nSince I don't want to update the commit id (e.g. 10th, 11th, etc..)\nof other test cases, I think it'd be better to modify the current test:\n\n- for the test case without '-f' flag, remove HEAD if it's the target,\n  and add a test-case to copy to somewhere else. Well, all of our current\n  test cases only test with HEAD as target-object.\n- for the test case with '-f' flag, I think I'll keep my current\n  approach.\n\n> \n> As to the log message, here is how I would explain/justify the\n> change, if I were writing it.\n\nI'll update the commit message.\n\n-- \nDanh\n"},{"id":"384162","messageId":"20191016051841.19896-1-congdanhqx@gmail.com","threadId":"52040","inReplyTo":"20191015163631.21808-1-congdanhqx@gmail.com","subject":"[PATCH 1/2] t3301: test diagnose messages for too few/many paramters","fromName":"Doan Tran Cong Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2019-10-16T05:18:40Z","receivedAt":"2019-10-16T05:18:56Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"From commit bbb1b8a35a, (\"notes: check number of parameters to \"git\nnotes copy\"\", 2010-06-28), we have a test for too many or too few\nof parameters provided to `git notes copy'.\n\nHowever, the test is only ensure the command will fail but it doesn't\nreally check if it fails because of number of parameters.\n\nIf we accidentally lifted the check inside our code base, the test may\nstill failed because the provided parameter is not a valid ref.\n\nCorrect it.\n\nSigned-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com>\n---\n t/t3301-notes.sh | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex d3fa298c6a..d7767e4438 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -1167,8 +1167,10 @@ test_expect_success 'GIT_NOTES_REWRITE_REF overrides config' '\n '\n \n test_expect_success 'git notes copy diagnoses too many or too few parameters' '\n-\ttest_must_fail git notes copy &&\n-\ttest_must_fail git notes copy one two three\n+\ttest_must_fail git notes copy 2>error &&\n+\ttest_i18ngrep \"too few parameters\" error &&\n+\ttest_must_fail git notes copy one two three 2>error &&\n+\ttest_i18ngrep \"too many parameters\" error\n '\n \n test_expect_success 'git notes get-ref expands refs/heads/master to refs/notes/refs/heads/master' '\n-- \n2.23.0\n\n"},{"id":"384163","messageId":"20191016051841.19896-2-congdanhqx@gmail.com","threadId":"52040","inReplyTo":"20191016051841.19896-1-congdanhqx@gmail.com","subject":"[PATCH 2/2] notes: fix minimum number of parameters to \"copy\" subcommand","fromName":"Doan Tran Cong Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2019-10-16T05:18:41Z","receivedAt":"2019-10-16T05:19:01Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"The builtin/notes.c::copy() function is prepared to handle either\none or two arguments given from the command line;\nin case of one argument is given, to-obj defaults to HEAD.\n\nWhen bbb1b8a3 (\"notes: check number of parameters to \"git notes copy\"\",\n2010-06-28) tried to make sure \"git notes copy\" (with *no* other\nargument) does not dereference NULL by making sure we did not get\ntoo few parameters, it incorrectly insisted that we need two arguments,\ninstead of either one or two.\nThis disabled the defaulting to-obj to HEAD.\nCorrect it.\n\nSigned-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com>\n---\n Documentation/git-notes.txt |  6 +++---\n builtin/notes.c             |  2 +-\n t/t3301-notes.sh            | 40 +++++++++++++++++++++++++++++++++++--\n 3 files changed, 42 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\nindex f56a5a9197..ced2e8280e 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n [verse]\n 'git notes' [list [<object>]]\n 'git notes' add [-f] [--allow-empty] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]\n-'git notes' copy [-f] ( --stdin | <from-object> <to-object> )\n+'git notes' copy [-f] ( --stdin | <from-object> [<to-object>] )\n 'git notes' append [--allow-empty] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]\n 'git notes' edit [--allow-empty] [<object>]\n 'git notes' show [<object>]\n@@ -68,8 +68,8 @@ add::\n \tsubcommand).\n \n copy::\n-\tCopy the notes for the first object onto the second object.\n-\tAbort if the second object already has notes, or if the first\n+\tCopy the notes for the first object onto the second object (defaults to\n+\tHEAD). Abort if the second object already has notes, or if the first\n \tobject has none (use -f to overwrite existing notes to the\n \tsecond object). This subcommand is equivalent to:\n \t`git notes add [-f] -C $(git notes list <from-object>) <to-object>`\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 02e97f55c5..95456f3165 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -513,7 +513,7 @@ static int copy(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\tif (argc < 2) {\n+\tif (argc < 1) {\n \t\terror(_(\"too few parameters\"));\n \t\tusage_with_options(git_notes_copy_usage, options);\n \t}\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex d7767e4438..d66a5f6faa 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -864,6 +864,24 @@ test_expect_success 'append to note from other note with \"git notes append -c\"'\n '\n \n test_expect_success 'copy note with \"git notes copy\"' '\n+\tcommit=$(git rev-parse 4th) &&\n+\tcat >expect <<-EOF &&\n+\t\tcommit $commit\n+\t\tAuthor: A U Thor <author@example.com>\n+\t\tDate:   Thu Apr 7 15:16:13 2005 -0700\n+\n+\t\t${indent}4th\n+\n+\t\tNotes:\n+\t\t${indent}This is a blob object\n+\tEOF\n+\tgit notes copy 8th 4th &&\n+\tgit log 3rd..4th >actual &&\n+\ttest_cmp expect actual &&\n+\ttest \"$(git note list 4th)\" = \"$(git note list 8th)\"\n+'\n+\n+test_expect_success 'copy note with \"git notes copy\" with default' '\n \ttest_commit 11th &&\n \tcommit=$(git rev-parse HEAD) &&\n \tcat >expect <<-EOF &&\n@@ -878,7 +896,7 @@ test_expect_success 'copy note with \"git notes copy\"' '\n \t\t${indent}\n \t\t${indent}yet another note\n \tEOF\n-\tgit notes copy HEAD^ HEAD &&\n+\tgit notes copy HEAD^ &&\n \tgit log -1 >actual &&\n \ttest_cmp expect actual &&\n \ttest \"$(git notes list HEAD)\" = \"$(git notes list HEAD^)\"\n@@ -892,6 +910,24 @@ test_expect_success 'prevent overwrite with \"git notes copy\"' '\n '\n \n test_expect_success 'allow overwrite with \"git notes copy -f\"' '\n+\tcommit=$(git rev-parse HEAD) &&\n+\tcat >expect <<-EOF &&\n+\t\tcommit $commit\n+\t\tAuthor: A U Thor <author@example.com>\n+\t\tDate:   Thu Apr 7 15:23:13 2005 -0700\n+\n+\t\t${indent}11th\n+\n+\t\tNotes:\n+\t\t${indent}This is a blob object\n+\tEOF\n+\tgit notes copy -f HEAD~3 HEAD &&\n+\tgit log -1 >actual &&\n+\ttest_cmp expect actual &&\n+\ttest \"$(git notes list HEAD)\" = \"$(git notes list HEAD~3)\"\n+'\n+\n+test_expect_success 'allow overwrite with \"git notes copy -f\" with default' '\n \tcommit=$(git rev-parse HEAD) &&\n \tcat >expect <<-EOF &&\n \t\tcommit $commit\n@@ -905,7 +941,7 @@ test_expect_success 'allow overwrite with \"git notes copy -f\"' '\n \t\t${indent}\n \t\t${indent}yet another note\n \tEOF\n-\tgit notes copy -f HEAD~2 HEAD &&\n+\tgit notes copy -f HEAD~2 &&\n \tgit log -1 >actual &&\n \ttest_cmp expect actual &&\n \ttest \"$(git notes list HEAD)\" = \"$(git notes list HEAD~2)\"\n-- \n2.23.0\n\n"},{"id":"384327","messageId":"xmqqeeza7urg.fsf@gitster-ct.c.googlers.com","threadId":"52040","inReplyTo":"20191016051841.19896-1-congdanhqx@gmail.com","subject":"Re: [PATCH 1/2] t3301: test diagnose messages for too few/many paramters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-18T00:38:27Z","receivedAt":"2019-10-18T00:38:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Doan Tran Cong Danh <congdanhqx@gmail.com> writes:\n\n> If we accidentally lifted the check inside our code base, the test may\n> still failed because the provided parameter is not a valid ref.\n\nMakes sense.  Another option would be to use a valid ref to make\nsure there are no other possible reason for the command to fail,\nwhich would make the test robust against us accidentally switching\nthe order of the check to see if they are refs first and then\ncomplain about too many or too few arguments ;-)  But I think what\nwe have here is fine as-is (I'll copy-edit the proposed log message\nfor grammos, though).\n\nThanks.\n\n>\n> Correct it.\n>\n> Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com>\n> ---\n>  t/t3301-notes.sh | 6 ++++--\n>  1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\n> index d3fa298c6a..d7767e4438 100755\n> --- a/t/t3301-notes.sh\n> +++ b/t/t3301-notes.sh\n> @@ -1167,8 +1167,10 @@ test_expect_success 'GIT_NOTES_REWRITE_REF overrides config' '\n>  '\n>  \n>  test_expect_success 'git notes copy diagnoses too many or too few parameters' '\n> -\ttest_must_fail git notes copy &&\n> -\ttest_must_fail git notes copy one two three\n> +\ttest_must_fail git notes copy 2>error &&\n> +\ttest_i18ngrep \"too few parameters\" error &&\n> +\ttest_must_fail git notes copy one two three 2>error &&\n> +\ttest_i18ngrep \"too many parameters\" error\n>  '\n>  \n>  test_expect_success 'git notes get-ref expands refs/heads/master to refs/notes/refs/heads/master' '\n"},{"id":"384328","messageId":"xmqqa79y7ugk.fsf@gitster-ct.c.googlers.com","threadId":"52040","inReplyTo":"20191016051841.19896-2-congdanhqx@gmail.com","subject":"Re: [PATCH 2/2] notes: fix minimum number of parameters to \"copy\" subcommand","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-18T00:44:59Z","receivedAt":"2019-10-18T00:45:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Doan Tran Cong Danh <congdanhqx@gmail.com> writes:\n\n>  Documentation/git-notes.txt |  6 +++---\n>  builtin/notes.c             |  2 +-\n>  t/t3301-notes.sh            | 40 +++++++++++++++++++++++++++++++++++--\n>  3 files changed, 42 insertions(+), 6 deletions(-)\n\nThanks, makes sense.\n\n"}]}