{"thread":{"id":"29109","subject":"[PATCH 1/2] parse-options: introduce OPT_SUBCOMMAND","startedAt":"2011-12-08T13:10:09Z","lastAt":"2011-12-15T21:30:38Z","messageCount":50,"participants":["Ramkumar Ramachandra","Matthieu Moy","Jonathan Nieder","Junio C Hamano","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"180576","messageId":"1323349817-15737-1-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":null,"subject":"[PATCH v2 0/6] Fix '&&' chaining in tests","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T13:10:09Z","receivedAt":"2011-12-08T13:10:09Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nThis follows-up $gmane/186481. Thanks to Jonathan and Matthieu for\ngoing through it. Changes are:\n1. Changes in response to Jonathan's review.\n2. Squash similar patches and re-order.\n\nThanks.\n\n-- Ram\n\nRamkumar Ramachandra (6):\n t3040 (subprojects-basic): modernize style\n t3030 (merge-recursive): use test_expect_code\n t1006 (cat-file): use test_cmp\n t3200 (branch): fix '&&' chaining\n t1510 (worktree): fix '&&' chaining\n test: fix '&&' chaining\n\n t/t1006-cat-file.sh | 119 +++++++++++++--------------\n t/t1007-hash-object.sh | 2 +-\n t/t1013-loose-object-format.sh | 2 +-\n t/t1300-repo-config.sh | 2 +-\n t/t1412-reflog-loop.sh | 2 +-\n t/t1501-worktree.sh | 6 +-\n t/t1510-repo-setup.sh | 4 +-\n t/t1511-rev-parse-caret.sh | 2 +-\n t/t3030-merge-recursive.sh | 72 ++---------------\n t/t3040-subprojects-basic.sh | 144 ++++++++++++++++----------------\n t/t3200-branch.sh | 4 +-\n t/t3310-notes-merge-manual-resolve.sh | 10 +-\n t/t3400-rebase.sh | 4 +-\n t/t3418-rebase-continue.sh | 4 +-\n t/t3419-rebase-patch-id.sh | 2 +-\n 15 files changed, 156 insertions(+), 223 deletions(-)\n\n-- \n1.7.7.3\n"},{"id":"180575","messageId":"1323349817-15737-2-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323349817-15737-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/2] parse-options: introduce OPT_SUBCOMMAND","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T13:10:10Z","receivedAt":"2011-12-08T13:10:10Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Several git programs take long dash-less options on the command-line\nto indicate different modes of operation like:\n\n git stash show\n git bundle verify test.bundle\n git bisect start\n\nCurrently, the parse-options framework forbids the use of\nopts->long_name and OPT_PARSE_NODASH, and the parsing has to be done\nby hand as a result. Lift this restriction, and create a new\nOPT_SUBCOMMAND; this is built on top of OPTION_BIT to allow for the\ndetection of more than one subcommand.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n parse-options.c | 5 +++--\n parse-options.h | 3 +++\n t/t0040-parse-options.sh | 31 +++++++++++++++++++++++++++++++\n test-parse-options.c | 4 ++++\n 4 files changed, 41 insertions(+), 2 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex f0098eb..079616a 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -278,6 +278,8 @@ static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,\n \t\t\tcontinue;\n \t\tif (options->short_name == arg[0] && arg[1] == '\\0')\n \t\t\treturn get_value(p, options, OPT_SHORT);\n+\t\tif (options->long_name && !strcmp(options->long_name, arg))\n+\t\t\treturn get_value(p, options, OPT_SHORT);\n \t}\n \treturn -2;\n }\n@@ -314,8 +316,7 @@ static void parse_options_check(const struct option *opts)\n \t\tif (opts->flags & PARSE_OPT_NODASH &&\n \t\t ((opts->flags & PARSE_OPT_OPTARG) ||\n \t\t !(opts->flags & PARSE_OPT_NOARG) ||\n-\t\t !(opts->flags & PARSE_OPT_NONEG) ||\n-\t\t opts->long_name))\n+\t\t !(opts->flags & PARSE_OPT_NONEG)))\n \t\t\terr |= optbug(opts, \"uses feature \"\n \t\t\t\t\t\"not supported for dashless options\");\n \t\tswitch (opts->type) {\ndiff --git a/parse-options.h b/parse-options.h\nindex 2e811dc..9267d46 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -123,6 +123,9 @@ struct option {\n #define OPT_GROUP(h) { OPTION_GROUP, 0, NULL, NULL, NULL, (h) }\n #define OPT_BIT(s, l, v, h, b) { OPTION_BIT, (s), (l), (v), NULL, (h), \\\n \t\t\t\t PARSE_OPT_NOARG, NULL, (b) }\n+#define OPT_SUBCOMMAND(l, v, h, i) { OPTION_BIT, 0, (l), (v), NULL, (h), \\\n+\t\t\t\t PARSE_OPT_NONEG | PARSE_OPT_NOARG | \\\n+\t\t\t\t PARSE_OPT_NODASH | PARSE_OPT_HIDDEN, NULL, (i) }\n #define OPT_NEGBIT(s, l, v, h, b) { OPTION_NEGBIT, (s), (l), (v), NULL, \\\n \t\t\t\t (h), PARSE_OPT_NOARG, NULL, (b) }\n #define OPT_COUNTUP(s, l, v, h) { OPTION_COUNTUP, (s), (l), (v), NULL, \\\ndiff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh\nindex a1e4616..47a402e 100755\n--- a/t/t0040-parse-options.sh\n+++ b/t/t0040-parse-options.sh\n@@ -55,6 +55,7 @@ mv expect expect.err\n \n cat > expect << EOF\n boolean: 2\n+subcommand: 0\n integer: 1729\n timestamp: 0\n string: 123\n@@ -74,6 +75,7 @@ test_expect_success 'short options' '\n \n cat > expect << EOF\n boolean: 2\n+subcommand: 0\n integer: 1729\n timestamp: 0\n string: 321\n@@ -103,6 +105,7 @@ test_expect_success 'missing required value' '\n \n cat > expect << EOF\n boolean: 1\n+subcommand: 0\n integer: 13\n timestamp: 0\n string: 123\n@@ -125,6 +128,7 @@ test_expect_success 'intermingled arguments' '\n \n cat > expect << EOF\n boolean: 0\n+subcommand: 0\n integer: 2\n timestamp: 0\n string: (not set)\n@@ -154,6 +158,7 @@ test_expect_success 'ambiguously abbreviated option' '\n \n cat > expect << EOF\n boolean: 0\n+subcommand: 0\n integer: 0\n timestamp: 0\n string: 123\n@@ -182,6 +187,7 @@ test_expect_success 'detect possible typos' '\n \n cat > expect < expect < expect < expect < expect < expect < output 2> output.err &&\n+\ttest ! -s output.err &&\n+\ttest_cmp expect output\n+'\n+\n+cat > expect <expect <\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t3040-subprojects-basic.sh | 144 +++++++++++++++++++++---------------------\n 1 files changed, 72 insertions(+), 72 deletions(-)\n\ndiff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh\nindex f6973e9..0a4ff6d 100755\n--- a/t/t3040-subprojects-basic.sh\n+++ b/t/t3040-subprojects-basic.sh\n@@ -3,81 +3,81 @@\n test_description='Basic subproject functionality'\n . ./test-lib.sh\n \n-test_expect_success 'Super project creation' \\\n- ': >Makefile &&\n- git add Makefile &&\n- git commit -m \"Superproject created\"'\n-\n-\n-cat >expected <Makefile && git add * &&\n- git commit -q -m \"subproject 1\" ) &&\n- mkdir sub2 &&\n- ( cd sub2 && git init && : >Makefile && git add * &&\n- git commit -q -m \"subproject 2\" ) &&\n- git update-index --add sub1 &&\n- git add sub2 &&\n- git commit -q -m \"subprojects added\" &&\n- git diff-tree --abbrev=5 HEAD^ HEAD |cut -d\" \" -f-3,5- >current &&\n- test_cmp expected current'\n-\n-git branch save HEAD\n-\n-test_expect_success 'check if fsck ignores the subprojects' \\\n- 'git fsck --full'\n-\n-test_expect_success 'check if commit in a subproject detected' \\\n- '( cd sub1 &&\n- echo \"all:\" >>Makefile &&\n- echo \"\ttrue\" >>Makefile &&\n- git commit -q -a -m \"make all\" ) && {\n- git diff-files --exit-code\n-\ttest $? = 1\n- }'\n-\n-test_expect_success 'check if a changed subproject HEAD can be committed' \\\n- 'git commit -q -a -m \"sub1 changed\" && {\n-\tgit diff-tree --exit-code HEAD^ HEAD\n-\ttest $? = 1\n- }'\n-\n-test_expect_success 'check if diff-index works for subproject elements' \\\n- 'git diff-index --exit-code --cached save -- sub1\n- test $? = 1'\n-\n-test_expect_success 'check if diff-tree works for subproject elements' \\\n- 'git diff-tree --exit-code HEAD^ HEAD -- sub1\n- test $? = 1'\n-\n-test_expect_success 'check if git diff works for subproject elements' \\\n- 'git diff --exit-code HEAD^ HEAD\n- test $? = 1'\n-\n-test_expect_success 'check if clone works' \\\n- 'git ls-files -s >expected &&\n- git clone -l -s . cloned &&\n- ( cd cloned && git ls-files -s ) >current &&\n- test_cmp expected current'\n-\n-test_expect_success 'removing and adding subproject' \\\n- 'git update-index --force-remove -- sub2 &&\n- mv sub2 sub3 &&\n- git add sub3 &&\n- git commit -q -m \"renaming a subproject\" && {\n-\tgit diff -M --name-status --exit-code HEAD^ HEAD\n-\ttest $? = 1\n- }'\n+test_expect_success 'setup: create superproject' '\n+\t: >Makefile &&\n+\tgit add Makefile &&\n+\tgit commit -m \"Superproject created\"\n+'\n+\n+test_expect_success 'setup: create subprojects' '\n+\tmkdir sub1 &&\n+\t( cd sub1 && git init && : >Makefile && git add * &&\n+\tgit commit -q -m \"subproject 1\" ) &&\n+\tmkdir sub2 &&\n+\t( cd sub2 && git init && : >Makefile && git add * &&\n+\tgit commit -q -m \"subproject 2\" ) &&\n+\tgit update-index --add sub1 &&\n+\tgit add sub2 &&\n+\tgit commit -q -m \"subprojects added\" &&\n+\tgit diff-tree --abbrev=5 HEAD^ HEAD |cut -d\" \" -f-3,5- >current &&\n+\tgit branch save HEAD &&\n+\tcat >expected <<-\\EOF &&\n+\t:000000 160000 00000... A\tsub1\n+\t:000000 160000 00000... A\tsub2\n+\tEOF\n+\ttest_cmp expected current\n+'\n+\n+test_expect_success 'check if fsck ignores the subprojects' '\n+\tgit fsck --full\n+'\n+\n+test_expect_success 'check if commit in a subproject detected' '\n+\t( cd sub1 &&\n+\techo \"all:\" >>Makefile &&\n+\techo \"\ttrue\" >>Makefile &&\n+\tgit commit -q -a -m \"make all\" ) &&\n+\ttest_expect_code 1 git diff-files --exit-code\n+'\n+\n+test_expect_success 'check if a changed subproject HEAD can be committed' '\n+\tgit commit -q -a -m \"sub1 changed\" &&\n+\ttest_expect_code 1 git diff-tree --exit-code HEAD^ HEAD\n+'\n+\n+test_expect_success 'check if diff-index works for subproject elements' '\n+\ttest_expect_code 1 git diff-index --exit-code --cached save -- sub1\n+'\n+\n+test_expect_success 'check if diff-tree works for subproject elements' '\n+\ttest_expect_code 1 git diff-tree --exit-code HEAD^ HEAD -- sub1\n+'\n+\n+test_expect_success 'check if git diff works for subproject elements' '\n+\ttest_expect_code 1 git diff --exit-code HEAD^ HEAD\n+'\n+\n+test_expect_success 'check if clone works' '\n+\tgit ls-files -s >expected &&\n+\tgit clone -l -s . cloned &&\n+\t( cd cloned && git ls-files -s ) >current &&\n+\ttest_cmp expected current\n+'\n+\n+test_expect_success 'removing and adding subproject' '\n+\tgit update-index --force-remove -- sub2 &&\n+\tmv sub2 sub3 &&\n+\tgit add sub3 &&\n+\tgit commit -q -m \"renaming a subproject\" &&\n+\ttest_expect_code 1 git diff -M --name-status --exit-code HEAD^ HEAD\n+'\n \n # the index must contain the object name the HEAD of the\n # subproject sub1 was at the point \"save\"\n-test_expect_success 'checkout in superproject' \\\n- 'git checkout save &&\n- git diff-index --exit-code --raw --cached save -- sub1'\n+test_expect_success 'checkout in superproject' '\n+\tgit checkout save &&\n+\tgit diff-index --exit-code --raw --cached save -- sub1\n+'\n \n # just interesting what happened...\n # git diff --name-status -M save master\n-- \n1.7.7.3\n"},{"id":"180577","messageId":"1323349817-15737-4-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323349817-15737-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/2] bundle: rewrite builtin to use parse-options","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T13:10:12Z","receivedAt":"2011-12-08T13:10:12Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The git-bundle builtin currently parses command-line options by hand;\nthis is both fragile and cryptic on failure. Since we now have an\nOPT_SUBCOMMAND, make use of it to parse the correct subcommand, while\nforbidding the use of more than one subcommand in the same invocation.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n builtin/bundle.c | 111 +++++++++++++++++++++++++++++++++++------------------\n 1 files changed, 73 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex 92a8a60..c977d9f 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -1,5 +1,6 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n+#include \"parse-options.h\"\n #include \"bundle.h\"\n \n /*\n@@ -9,57 +10,91 @@\n * bundle supporting \"fetch\", \"pull\", and \"ls-remote\".\n */\n \n-static const char builtin_bundle_usage[] =\n- \"git bundle create \\n\"\n- \" or: git bundle verify \\n\"\n- \" or: git bundle list-heads [...]\\n\"\n- \" or: git bundle unbundle [...]\";\n+static const char * builtin_bundle_usage[] = {\n+\t\"git bundle create \",\n+\t\"git bundle verify \",\n+\t\"git bundle list-heads [...]\",\n+\t\"git bundle unbundle [...]\",\n+\tNULL\n+};\n+\n+enum bundle_subcommand {\n+\tBUNDLE_NONE = 0,\n+\tBUNDLE_CREATE = 1,\n+\tBUNDLE_VERIFY = 2,\n+\tBUNDLE_LIST_HEADS = 4,\n+\tBUNDLE_UNBUNDLE = 8\n+};\n \n int cmd_bundle(int argc, const char **argv, const char *prefix)\n {\n-\tstruct bundle_header header;\n-\tconst char *cmd, *bundle_file;\n+\tint prefix_length;\n \tint bundle_fd = -1;\n-\tchar buffer[PATH_MAX];\n+\tconst char *bundle_file;\n+\tstruct bundle_header header;\n+\tenum bundle_subcommand subcommand = BUNDLE_NONE;\n \n-\tif (argc < 3)\n-\t\tusage(builtin_bundle_usage);\n+\tstruct option options[] = {\n+\t\tOPT_SUBCOMMAND(\"create\", &subcommand,\n+\t\t\t\"create a new bundle\",\n+\t\t\tBUNDLE_CREATE),\n+\t\tOPT_SUBCOMMAND(\"verify\", &subcommand,\n+\t\t\t\"verify clean application of the bundle\",\n+\t\t\tBUNDLE_VERIFY),\n+\t\tOPT_SUBCOMMAND(\"list-heads\", &subcommand,\n+\t\t\t\"list references defined in the bundle\",\n+\t\t\tBUNDLE_LIST_HEADS),\n+\t\tOPT_SUBCOMMAND(\"unbundle\", &subcommand,\n+\t\t\t\"pass objects in the bundle to 'git index-pack'\",\n+\t\t\tBUNDLE_UNBUNDLE),\n+\t\tOPT_END(),\n+\t};\n \n-\tcmd = argv[1];\n-\tbundle_file = argv[2];\n-\targc -= 2;\n-\targv += 2;\n+\targc = parse_options(argc, argv, NULL,\n+\t\t\toptions, builtin_bundle_usage,\n+\t\t\tPARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);\n \n-\tif (prefix && bundle_file[0] != '/') {\n-\t\tsnprintf(buffer, sizeof(buffer), \"%s/%s\", prefix, bundle_file);\n-\t\tbundle_file = buffer;\n-\t}\n+\tif (argc < 2)\n+\t\tusage_with_options(builtin_bundle_usage, options);\n \n-\tmemset(&header, 0, sizeof(header));\n-\tif (strcmp(cmd, \"create\") && (bundle_fd =\n-\t\t\t\tread_bundle_header(bundle_file, &header)) < 0)\n-\t\treturn 1;\n+\t/* The next parameter on the command line is bundle_file */\n+\tprefix_length = prefix ? strlen(prefix) : 0;\n+\tbundle_file = prefix_filename(prefix, prefix_length, argv[1]);\n+\targc -= 1;\n+\targv += 1;\n \n-\tif (!strcmp(cmd, \"verify\")) {\n+\t/* Read out bundle header, except in BUNDLE_CREATE case */\n+\tif (subcommand == BUNDLE_VERIFY || subcommand == BUNDLE_LIST_HEADS ||\n+\t\tsubcommand == BUNDLE_UNBUNDLE) {\n+\t\tmemset(&header, 0, sizeof(header));\n+\t\tbundle_fd = read_bundle_header(bundle_file, &header);\n+\t\tif (bundle_fd < 0)\n+\t\t\tdie_errno(_(\"Failed to open bundle file '%s'\"), bundle_file);\n+\t}\n+\n+\tswitch (subcommand) {\n+\tcase BUNDLE_CREATE:\n+\t\tif (!startup_info->have_repository)\n+\t\t\tdie(_(\"Need a repository to create a bundle.\"));\n+\t\treturn create_bundle(&header, bundle_file, argc, argv);\n+\tcase BUNDLE_VERIFY:\n \t\tclose(bundle_fd);\n \t\tif (verify_bundle(&header, 1))\n-\t\t\treturn 1;\n+\t\t\treturn -1; /* Error already reported */\n \t\tfprintf(stderr, _(\"%s is okay\\n\"), bundle_file);\n-\t\treturn 0;\n-\t}\n-\tif (!strcmp(cmd, \"list-heads\")) {\n+\t\tbreak;\n+\tcase BUNDLE_LIST_HEADS:\n \t\tclose(bundle_fd);\n-\t\treturn !!list_bundle_refs(&header, argc, argv);\n-\t}\n-\tif (!strcmp(cmd, \"create\")) {\n-\t\tif (!startup_info->have_repository)\n-\t\t\tdie(_(\"Need a repository to create a bundle.\"));\n-\t\treturn !!create_bundle(&header, bundle_file, argc, argv);\n-\t} else if (!strcmp(cmd, \"unbundle\")) {\n-\t\tif (!startup_info->have_repository)\n+\t\treturn list_bundle_refs(&header, argc, argv);\n+\tcase BUNDLE_UNBUNDLE:\n+\t\tif (!startup_info->have_repository) {\n+\t\t\tclose(bundle_fd);\n \t\t\tdie(_(\"Need a repository to unbundle.\"));\n-\t\treturn !!unbundle(&header, bundle_fd, 0) ||\n+\t\t}\n+\t\treturn unbundle(&header, bundle_fd, 0) ||\n \t\t\tlist_bundle_refs(&header, argc, argv);\n-\t} else\n-\t\tusage(builtin_bundle_usage);\n+\tdefault:\n+\t\tusage_with_options(builtin_bundle_usage, options);\n+\t}\n+\treturn 0;\n }\n-- \n1.7.7.3\n"},{"id":"180579","messageId":"1323349817-15737-5-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323349817-15737-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/6] t3030 (merge-recursive): use test_expect_code","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T13:10:13Z","receivedAt":"2011-12-08T13:10:13Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Use test_expect_code in preference to repeatedly checking exit codes\nby hand.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t3030-merge-recursive.sh | 72 ++++----------------------------------------\n 1 files changed, 6 insertions(+), 66 deletions(-)\n\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex 55ef189..a5e3da7 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -285,17 +285,7 @@ test_expect_success 'merge-recursive simple' '\n \trm -fr [abcd] &&\n \tgit checkout -f \"$c2\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c2\" \"$c1\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c2\" \"$c1\"\n '\n \n test_expect_success 'merge-recursive result' '\n@@ -334,17 +324,7 @@ test_expect_success 'merge-recursive remove conflict' '\n \trm -fr [abcd] &&\n \tgit checkout -f \"$c1\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c1\" \"$c5\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c1\" \"$c5\"\n '\n \n test_expect_success 'merge-recursive remove conflict' '\n@@ -388,17 +368,7 @@ test_expect_success 'merge-recursive d/f conflict' '\n \tgit reset --hard &&\n \tgit checkout -f \"$c1\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c1\" \"$c4\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c1\" \"$c4\"\n '\n \n test_expect_success 'merge-recursive d/f conflict result' '\n@@ -422,17 +392,7 @@ test_expect_success 'merge-recursive d/f conflict the other way' '\n \tgit reset --hard &&\n \tgit checkout -f \"$c4\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c4\" \"$c1\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c4\" \"$c1\"\n '\n \n test_expect_success 'merge-recursive d/f conflict result the other way' '\n@@ -456,17 +416,7 @@ test_expect_success 'merge-recursive d/f conflict' '\n \tgit reset --hard &&\n \tgit checkout -f \"$c1\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c1\" \"$c6\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c1\" \"$c6\"\n '\n \n test_expect_success 'merge-recursive d/f conflict result' '\n@@ -490,17 +440,7 @@ test_expect_success 'merge-recursive d/f conflict' '\n \tgit reset --hard &&\n \tgit checkout -f \"$c6\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c6\" \"$c1\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c6\" \"$c1\"\n '\n \n test_expect_success 'merge-recursive d/f conflict result' '\n-- \n1.7.7.3\n"},{"id":"180581","messageId":"1323349817-15737-6-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323349817-15737-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/6] t1006 (cat-file): use test_cmp","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T13:10:14Z","receivedAt":"2011-12-08T13:10:14Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Use test_cmp in preference to repeatedly comparing command outputs by\nhand.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t1006-cat-file.sh | 119 ++++++++++++++++++++++++---------------------------\n 1 files changed, 56 insertions(+), 63 deletions(-)\n\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex d8b7f2f..0ca6c43 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -36,66 +36,41 @@ $content\"\n '\n \n test_expect_success \"Type of $type is correct\" '\n- test $type = \"$(git cat-file -t $sha1)\"\n+\techo $type >expect &&\n+\tgit cat-file -t $sha1 >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success \"Size of $type is correct\" '\n- test $size = \"$(git cat-file -s $sha1)\"\n+\techo $size >expect &&\n+\tgit cat-file -s $sha1 >actual &&\n+\ttest_cmp expect actual\n '\n \n test -z \"$content\" ||\n test_expect_success \"Content of $type is correct\" '\n-\texpect=\"$(maybe_remove_timestamp \"$content\" $no_ts)\"\n-\tactual=\"$(maybe_remove_timestamp \"$(git cat-file $type $sha1)\" $no_ts)\"\n-\n- if test \"z$expect\" = \"z$actual\"\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Oops: expected $expect\"\n-\t\techo \"but got $actual\"\n-\t\tfalse\n- fi\n+\tmaybe_remove_timestamp \"$content\" $no_ts >expect &&\n+\tmaybe_remove_timestamp \"$(git cat-file $type $sha1)\" $no_ts >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success \"Pretty content of $type is correct\" '\n-\texpect=\"$(maybe_remove_timestamp \"$pretty_content\" $no_ts)\"\n-\tactual=\"$(maybe_remove_timestamp \"$(git cat-file -p $sha1)\" $no_ts)\"\n- if test \"z$expect\" = \"z$actual\"\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Oops: expected $expect\"\n-\t\techo \"but got $actual\"\n-\t\tfalse\n- fi\n+\tmaybe_remove_timestamp \"$pretty_content\" $no_ts >expect &&\n+\tmaybe_remove_timestamp \"$(git cat-file -p $sha1)\" $no_ts >actual &&\n+\ttest_cmp expect actual\n '\n \n test -z \"$content\" ||\n test_expect_success \"--batch output of $type is correct\" '\n-\texpect=\"$(maybe_remove_timestamp \"$batch_output\" $no_ts)\"\n-\tactual=\"$(maybe_remove_timestamp \"$(echo $sha1 | git cat-file --batch)\" $no_ts)\"\n- if test \"z$expect\" = \"z$actual\"\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Oops: expected $expect\"\n-\t\techo \"but got $actual\"\n-\t\tfalse\n- fi\n+\tmaybe_remove_timestamp \"$batch_output\" $no_ts >expect &&\n+\tmaybe_remove_timestamp \"$(echo $sha1 | git cat-file --batch)\" $no_ts >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success \"--batch-check output of $type is correct\" '\n-\texpect=\"$sha1 $type $size\"\n-\tactual=\"$(echo_without_newline $sha1 | git cat-file --batch-check)\"\n- if test \"z$expect\" = \"z$actual\"\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Oops: expected $expect\"\n-\t\techo \"but got $actual\"\n-\t\tfalse\n- fi\n+\techo \"$sha1 $type $size\" >expect &&\n+\techo_without_newline $sha1 | git cat-file --batch-check >actual &&\n+\ttest_cmp expect actual\n '\n }\n \n@@ -144,10 +119,13 @@ tag_size=$(strlen \"$tag_content\")\n \n run_tests 'tag' $tag_sha1 $tag_size \"$tag_content\" \"$tag_pretty_content\" 1\n \n-test_expect_success \\\n- \"Reach a blob from a tag pointing to it\" \\\n- \"test '$hello_content' = \\\"\\$(git cat-file blob $tag_sha1)\\\"\"\n+test_expect_success \"Reach a blob from a tag pointing to it\" '\n+ echo_without_newline \"$hello_content\" >expect &&\n+ git cat-file blob \"$tag_sha1\" >actual &&\n+ test_cmp expect actual\n+'\n \n+test_done\n for batch in batch batch-check\n do\n for opt in t s e p\n@@ -175,30 +153,41 @@ do\n done\n \n test_expect_success \"--batch-check for a non-existent named object\" '\n- test \"foobar42 missing\n-foobar84 missing\" = \\\n- \"$( ( echo foobar42; echo_without_newline foobar84; ) | git cat-file --batch-check)\"\n+ cat >expect <<\\-EOF &&\n+foobar42 missing\n+foobar84 missing\n+EOF\n+ $(echo foobar42; echo_without_newline foobar84) \\\n+ | git cat-file --batch-check >actual &&\n+ test_cmp expect actual\n '\n \n test_expect_success \"--batch-check for a non-existent hash\" '\n- test \"0000000000000000000000000000000000000042 missing\n-0000000000000000000000000000000000000084 missing\" = \\\n- \"$( ( echo 0000000000000000000000000000000000000042;\n- echo_without_newline 0000000000000000000000000000000000000084; ) \\\n- | git cat-file --batch-check)\"\n+ cat >expect <<\\-EOF &&\n+0000000000000000000000000000000000000042 missing\n+0000000000000000000000000000000000000084 missing\n+EOF\n+ $(echo 0000000000000000000000000000000000000042;\n+ echo_without_newline 0000000000000000000000000000000000000084) \\\n+ | git cat-file --batch-check >actual &&\n+ test_cmp expect actual\n '\n \n test_expect_success \"--batch for an existent and a non-existent hash\" '\n- test \"$tag_sha1 tag $tag_size\n+ cat >expect <<\\-EOF &&\n+tag_sha1 tag $tag_size\n $tag_content\n-0000000000000000000000000000000000000000 missing\" = \\\n- \"$( ( echo $tag_sha1;\n- echo_without_newline 0000000000000000000000000000000000000000; ) \\\n- | git cat-file --batch)\"\n+0000000000000000000000000000000000000000 missing\n+EOF\n+ $(echo $tag_sha1; echo_without_newline 0000000000000000000000000000000000000000) \\\n+ | git cat-file --batch >actual &&\n+ test_cmp expect_actual\n '\n \n test_expect_success \"--batch-check for an emtpy line\" '\n- test \" missing\" = \"$(echo | git cat-file --batch-check)\"\n+ echo \" missing\" >expect &&\n+ echo | git cat-file --batch-check >actual &&\n+ test_cmp expect actual\n '\n \n batch_input=\"$hello_sha1\n@@ -218,7 +207,10 @@ deadbeef missing\n missing\"\n \n test_expect_success '--batch with multiple sha1s gives correct format' '\n-\ttest \"$(maybe_remove_timestamp \"$batch_output\" 1)\" = \"$(maybe_remove_timestamp \"$(echo_without_newline \"$batch_input\" | git cat-file --batch)\" 1)\"\n+\tmaybe_remove_timestamp \"$batch_output\" 1 >expect &&\n+\tmaybe_remove_timestamp \"$(echo_without_newline \"$batch_input\" \\\n+\t| git cat-file --batch)\" 1 >actual &&\n+\ttest_cmp expect actual\n '\n \n batch_check_input=\"$hello_sha1\n@@ -237,8 +229,9 @@ deadbeef missing\n missing\"\n \n test_expect_success \"--batch-check with multiple sha1s gives correct format\" '\n- test \"$batch_check_output\" = \\\n- \"$(echo_without_newline \"$batch_check_input\" | git cat-file --batch-check)\"\n+ echo \"$batch_check_output\" >expect &&\n+ echo_without_newline \"$batch_check_input\" | git cat-file --batch-check >actual &&\n+ test_cmp expect actual\n '\n \n test_done\n-- \n1.7.7.3\n"},{"id":"180580","messageId":"1323349817-15737-7-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323349817-15737-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/6] t3200 (branch): fix '&&' chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T13:10:15Z","receivedAt":"2011-12-08T13:10:15Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Breaks in a test assertion's && chain can potentially hide failures\nfrom earlier commands in the chain. Fix these breaks.\n\nAdditionally, note that 'git branch --help' will fail when git\nmanpages aren't already installed: guard the line with a\n'test_might_fail'.\n\nHelped-by: Jonathan Nieder \nSigned-off-by: Ramkumar Ramachandra \n---\n t/t3200-branch.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex bc73c20..95066d0 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -22,7 +22,7 @@ test_expect_success \\\n \n test_expect_success \\\n 'git branch --help should not have created a bogus branch' '\n- git branch --help /dev/null 2>/dev/null;\n+ test_might_fail git branch --help /dev/null 2>/dev/null &&\n test_path_is_missing .git/refs/heads/--help\n '\n \n@@ -88,7 +88,7 @@ test_expect_success \\\n test_expect_success \\\n 'git branch -m n/n n should work' \\\n 'git branch -l n/n &&\n- git branch -m n/n n\n+ git branch -m n/n n &&\n \ttest_path_is_file .git/logs/refs/heads/n'\n \n test_expect_success 'git branch -m o/o o should fail when o/p exists' '\n-- \n1.7.7.3\n"},{"id":"180583","messageId":"1323349817-15737-8-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323349817-15737-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 5/6] t1510 (worktree): fix '&&' chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T13:10:16Z","receivedAt":"2011-12-08T13:10:16Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Breaks in a test assertion's && chain can potentially hide failures\nfrom earlier commands in the chain. Fix these breaks.\n\nAdditionally, note that 'unset' returns non-zero status when the\nvariable passed was already unset on some shells: change these\ninstances to 'safe_unset'.\n\nHelped-by: Jonathan Nieder \nSigned-off-by: Ramkumar Ramachandra \n---\n t/t1501-worktree.sh | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh\nindex 6384983..8e04e7c 100755\n--- a/t/t1501-worktree.sh\n+++ b/t/t1501-worktree.sh\n@@ -48,7 +48,7 @@ test_expect_success 'setup: helper for testing rev-parse' '\n '\n \n test_expect_success 'setup: core.worktree = relative path' '\n-\tunset GIT_WORK_TREE;\n+\tsafe_unset GIT_WORK_TREE &&\n \tGIT_DIR=repo.git &&\n \tGIT_CONFIG=\"$(pwd)\"/$GIT_DIR/config &&\n \texport GIT_DIR GIT_CONFIG &&\n@@ -89,7 +89,7 @@ test_expect_success 'subdir of work tree' '\n '\n \n test_expect_success 'setup: core.worktree = absolute path' '\n-\tunset GIT_WORK_TREE;\n+\tsafe_unset GIT_WORK_TREE &&\n \tGIT_DIR=$(pwd)/repo.git &&\n \tGIT_CONFIG=$GIT_DIR/config &&\n \texport GIT_DIR GIT_CONFIG &&\n@@ -334,7 +334,7 @@ test_expect_success 'absolute pathspec should fail gracefully' '\n '\n \n test_expect_success 'make_relative_path handles double slashes in GIT_DIR' '\n-\t>dummy_file\n+\t>dummy_file &&\n \techo git --git-dir=\"$(pwd)//repo.git\" --work-tree=\"$(pwd)\" add dummy_file &&\n \tgit --git-dir=\"$(pwd)//repo.git\" --work-tree=\"$(pwd)\" add dummy_file\n '\n-- \n1.7.7.3\n"},{"id":"180582","messageId":"1323349817-15737-9-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323349817-15737-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 6/6] test: fix '&&' chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T13:10:17Z","receivedAt":"2011-12-08T13:10:17Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Breaks in a test assertion's && chain can potentially hide failures\nfrom earlier commands in the chain. Fix instances of this in the\nfollowing tests:\n\nt3419 (rebase-patch-id)\nt3310 (notes-merge-manual-resolve)\nt3400 (rebase)\nt3418 (rebase-continue)\nt1511 (rev-parse-caret)\nt1510 (repo-setup)\nt1007 (hash-object)\nt1412 (reflog-loop)\nt1300 (repo-config)\nt1013 (loose-object-format)\n\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t1007-hash-object.sh | 2 +-\n t/t1013-loose-object-format.sh | 2 +-\n t/t1300-repo-config.sh | 2 +-\n t/t1412-reflog-loop.sh | 2 +-\n t/t1510-repo-setup.sh | 4 ++--\n t/t1511-rev-parse-caret.sh | 2 +-\n t/t3310-notes-merge-manual-resolve.sh | 10 +++++-----\n t/t3400-rebase.sh | 4 ++--\n t/t3418-rebase-continue.sh | 4 ++--\n t/t3419-rebase-patch-id.sh | 2 +-\n 10 files changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh\nindex 6d52b82..f83df8e 100755\n--- a/t/t1007-hash-object.sh\n+++ b/t/t1007-hash-object.sh\n@@ -189,7 +189,7 @@ for args in \"-w --stdin-paths\" \"--stdin-paths -w\"; do\n done\n \n test_expect_success 'corrupt tree' '\n-\techo abc >malformed-tree\n+\techo abc >malformed-tree &&\n \ttest_must_fail git hash-object -t tree malformed-tree\n '\n \ndiff --git a/t/t1013-loose-object-format.sh b/t/t1013-loose-object-format.sh\nindex 0a9cedd..fbf5f2f 100755\n--- a/t/t1013-loose-object-format.sh\n+++ b/t/t1013-loose-object-format.sh\n@@ -34,7 +34,7 @@ assert_blob_equals() {\n }\n \n test_expect_success setup '\n-\tcp -R \"$TEST_DIRECTORY/t1013/objects\" .git/\n+\tcp -R \"$TEST_DIRECTORY/t1013/objects\" .git/ &&\n \tgit --version\n '\n \ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 51caff0..0690e0e 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -38,7 +38,7 @@ cat > expect << EOF\n \tWhatEver = Second\n EOF\n test_expect_success 'similar section' '\n-\tgit config Cores.WhatEver Second\n+\tgit config Cores.WhatEver Second &&\n \ttest_cmp expect .git/config\n '\n \ndiff --git a/t/t1412-reflog-loop.sh b/t/t1412-reflog-loop.sh\nindex 647d888..3acd895 100755\n--- a/t/t1412-reflog-loop.sh\n+++ b/t/t1412-reflog-loop.sh\n@@ -20,7 +20,7 @@ test_expect_success 'setup reflog with alternating commits' '\n '\n \n test_expect_success 'reflog shows all entries' '\n-\tcat >expect <<-\\EOF\n+\tcat >expect <<-\\EOF &&\n \t\ttopic@{0} reset: moving to two\n \t\ttopic@{1} reset: moving to one\n \t\ttopic@{2} reset: moving to two\ndiff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh\nindex ec50a9a..80aedfc 100755\n--- a/t/t1510-repo-setup.sh\n+++ b/t/t1510-repo-setup.sh\n@@ -603,7 +603,7 @@ test_expect_success '#22a: core.worktree = GIT_DIR = .git dir' '\n \t# like case #6.\n \n \tsetup_repo 22a \"$here/22a/.git\" \"\" unset &&\n-\tsetup_repo 22ab . \"\" unset\n+\tsetup_repo 22ab . \"\" unset &&\n \tmkdir -p 22a/.git/sub 22a/sub &&\n \tmkdir -p 22ab/.git/sub 22ab/sub &&\n \ttry_case 22a/.git unset . \\\n@@ -742,7 +742,7 @@ test_expect_success '#28: core.worktree and core.bare conflict (gitfile case)' '\n # Case #29: GIT_WORK_TREE(+core.worktree) overrides core.bare (gitfile case).\n test_expect_success '#29: setup' '\n \tsetup_repo 29 non-existent gitfile true &&\n-\tmkdir -p 29/sub/sub 29/wt/sub\n+\tmkdir -p 29/sub/sub 29/wt/sub &&\n \t(\n \t\tcd 29 &&\n \t\tGIT_WORK_TREE=\"$here/29\" &&\ndiff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh\nindex e043cb7..eaefc77 100755\n--- a/t/t1511-rev-parse-caret.sh\n+++ b/t/t1511-rev-parse-caret.sh\n@@ -6,7 +6,7 @@ test_description='tests for ref^{stuff}'\n \n test_expect_success 'setup' '\n \techo blob >a-blob &&\n-\tgit tag -a -m blob blob-tag `git hash-object -w a-blob`\n+\tgit tag -a -m blob blob-tag `git hash-object -w a-blob` &&\n \tmkdir a-tree &&\n \techo moreblobs >a-tree/another-blob &&\n \tgit add . &&\ndiff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh\nindex 4ec4d11..4367197 100755\n--- a/t/t3310-notes-merge-manual-resolve.sh\n+++ b/t/t3310-notes-merge-manual-resolve.sh\n@@ -389,7 +389,7 @@ test_expect_success 'abort notes merge' '\n \ttest_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# m has not moved (still == y)\n-\ttest \"$(git rev-parse refs/notes/m)\" = \"$(cat pre_merge_y)\"\n+\ttest \"$(git rev-parse refs/notes/m)\" = \"$(cat pre_merge_y)\" &&\n \t# Verify that other notes refs has not changed (w, x, y and z)\n \tverify_notes w &&\n \tverify_notes x &&\n@@ -525,9 +525,9 @@ EOF\n \ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&\n \ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&\n \t# Refs are unchanged\n-\ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\"\n-\ttest \"$(git rev-parse refs/notes/y)\" = \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\"\n-\ttest \"$(git rev-parse refs/notes/m)\" != \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\"\n+\ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\" &&\n+\ttest \"$(git rev-parse refs/notes/y)\" = \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\" &&\n+\ttest \"$(git rev-parse refs/notes/m)\" != \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\" &&\n \t# Mention refs/notes/m, and its current and expected value in output\n \tgrep -q \"refs/notes/m\" output &&\n \tgrep -q \"$(git rev-parse refs/notes/m)\" output &&\n@@ -545,7 +545,7 @@ test_expect_success 'resolve situation by aborting the notes merge' '\n \ttest_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# m has not moved (still == w)\n-\ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\"\n+\ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\" &&\n \t# Verify that other notes refs has not changed (w, x, y and z)\n \tverify_notes w &&\n \tverify_notes x &&\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 6eaecec..c355533 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -172,8 +172,8 @@ test_expect_success 'fail when upstream arg is missing and not configured' '\n \n test_expect_success 'default to @{upstream} when upstream arg is missing' '\n \tgit checkout -b default topic &&\n-\tgit config branch.default.remote .\n-\tgit config branch.default.merge refs/heads/master\n+\tgit config branch.default.remote . &&\n+\tgit config branch.default.merge refs/heads/master &&\n \tgit rebase &&\n \ttest \"$(git rev-parse default~1)\" = \"$(git rev-parse master)\"\n '\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 1e855cd..2680375 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -51,7 +51,7 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '\n \ttest_commit \"commit-new-file-F3-on-topic-branch\" F3 32 &&\n \ttest_when_finished \"rm -fr test-bin funny.was.run\" &&\n \tmkdir test-bin &&\n-\tcat >test-bin/git-merge-funny <<-EOF\n+\tcat >test-bin/git-merge-funny <<-EOF &&\n \t#!$SHELL_PATH\n \tcase \"\\$1\" in --opt) ;; *) exit 2 ;; esac\n \tshift &&\n@@ -77,7 +77,7 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '\n test_expect_success 'rebase --continue remembers --rerere-autoupdate' '\n \trm -fr .git/rebase-* &&\n \tgit reset --hard commit-new-file-F3-on-topic-branch &&\n-\tgit checkout master\n+\tgit checkout master &&\n \ttest_commit \"commit-new-file-F3\" F3 3 &&\n \tgit config rerere.enabled true &&\n \ttest_must_fail git rebase -m master topic &&\ndiff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh\nindex bd8efaf..e70ac10 100755\n--- a/t/t3419-rebase-patch-id.sh\n+++ b/t/t3419-rebase-patch-id.sh\n@@ -39,7 +39,7 @@ run()\n }\n \n test_expect_success 'setup' '\n-\tgit commit --allow-empty -m initial\n+\tgit commit --allow-empty -m initial &&\n \tgit tag root\n '\n \n-- \n1.7.7.3\n"},{"id":"180584","messageId":"vpqiplrchvi.fsf@bauges.imag.fr","threadId":"29109","inReplyTo":"1323349817-15737-6-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/6] t1006 (cat-file): use test_cmp","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2011-12-08T13:28:33Z","receivedAt":"2011-12-08T13:28:33Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ramkumar Ramachandra writes:\n\n> test_expect_success \"--batch-check with multiple sha1s gives correct format\" '\n> - test \"$batch_check_output\" = \\\n> - \"$(echo_without_newline \"$batch_check_input\" | git cat-file --batch-check)\"\n> + echo \"$batch_check_output\" >expect &&\n> + echo_without_newline \"$batch_check_input\" | git cat-file\n> + --batch-check >actual &&\n> + test_cmp expect actual\n> '\n\nWhitespace damage?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"180585","messageId":"CALkWK0=NFxAqmOkObqjVmBQ-TQ=hZhWi=ZScMEGibvS2Pu+XqQ@mail.gmail.com","threadId":"29109","inReplyTo":"vpqiplrchvi.fsf@bauges.imag.fr","subject":"Re: [PATCH 3/6] t1006 (cat-file): use test_cmp","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T13:33:19Z","receivedAt":"2011-12-08T13:33:19Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Matthieu,\n\nMatthieu Moy wrote:\n> Ramkumar Ramachandra writes:\n>\n>>  test_expect_success \"--batch-check with multiple sha1s gives correct format\" '\n>> -    test \"$batch_check_output\" = \\\n>> -    \"$(echo_without_newline \"$batch_check_input\" | git cat-file --batch-check)\"\n>> +    echo \"$batch_check_output\" >expect &&\n>> +    echo_without_newline \"$batch_check_input\" | git cat-file\n>> + --batch-check >actual &&\n>> +    test_cmp expect actual\n>>  '\n>\n> Whitespace damage?\n\nOdd. Email client issue? Seems to be fine in the original message [1].\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/186558\n\n-- Ram\n"},{"id":"180588","messageId":"vpqty5bp4di.fsf@bauges.imag.fr","threadId":"29109","inReplyTo":"CALkWK0=NFxAqmOkObqjVmBQ-TQ=hZhWi=ZScMEGibvS2Pu+XqQ@mail.gmail.com","subject":"Re: [PATCH 3/6] t1006 (cat-file): use test_cmp","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2011-12-08T13:41:45Z","receivedAt":"2011-12-08T13:41:45Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ramkumar Ramachandra writes:\n\n> Hi Matthieu,\n>\n> Matthieu Moy wrote:\n>> Ramkumar Ramachandra writes:\n>>\n>>>  test_expect_success \"--batch-check with multiple sha1s gives correct format\" '\n>>> -    test \"$batch_check_output\" = \\\n>>> -    \"$(echo_without_newline \"$batch_check_input\" | git cat-file\n>>> --batch-check)\"\n>>> +    echo \"$batch_check_output\" >expect &&\n>>> +    echo_without_newline \"$batch_check_input\" | git cat-file\n>>> + --batch-check >actual &&\n>>> +    test_cmp expect actual\n>>>  '\n>>\n>> Whitespace damage?\n>\n> Odd. Email client issue?\n\nIt seems so. Weird, Gnus usually doesn't do this sort of things to me.\nSorry for the noise.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"180590","messageId":"20111208163049.GA2116@elie.hsd1.il.comcast.net","threadId":"29109","inReplyTo":"1323349817-15737-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 1/2] parse-options: introduce OPT_SUBCOMMAND","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-08T16:30:50Z","receivedAt":"2011-12-08T16:30:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nQuick thoughts:\n\nRamkumar Ramachandra wrote:\n\n> Currently, the parse-options framework forbids the use of\n> opts->long_name and OPT_PARSE_NODASH, and the parsing has to be done\n> by hand as a result. Lift this restriction\n\nThis part seems like a sane idea to me.\n\n> , and create a new\n> OPT_SUBCOMMAND; this is built on top of OPTION_BIT to allow for the\n> detection of more than one subcommand.\n\nThis part I am not convinced about. Usually each subcommand takes its\nown options, so I cannot see this OPT_SUBCOMMAND being actually useful\nfor commands like \"git stash\" or \"git remote\".\n\nHope that helps,\nJonathan\n"},{"id":"180591","messageId":"20111208163448.GA2394@elie.hsd1.il.comcast.net","threadId":"29109","inReplyTo":"1323349817-15737-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 1/6] t3040 (subprojects-basic): modernize style","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-08T16:34:48Z","receivedAt":"2011-12-08T16:34:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Put the opening quote starting each test on the same line as the\n> test_expect_* invocation. While at it:\n\nI suspect the above description, while it does describe your patch,\ndoes not describe the _reason_ that the patch exists or that someone\nwould want to apply it. Isn't it something more like:\n\n\tMake the following changes pertaining to &&-chaining, for some\n\tgood reason that I will describe:\n\n\t- ...\n\n\tWhile at it, clean up the style to fit the prevailing style.\n\tThat means:\n\n\t- Put the opening quote starting each test on the same line as\n\t...\n\nI didn't read over the patch again. Has it changed since v1?\n"},{"id":"180592","messageId":"20111208163946.GB2394@elie.hsd1.il.comcast.net","threadId":"29109","inReplyTo":"1323349817-15737-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-08T16:39:46Z","receivedAt":"2011-12-08T16:39:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> -\tif (argc < 3)\n> -\t\tusage(builtin_bundle_usage);\n> -\n> -\tcmd = argv[1];\n> -\tbundle_file = argv[2];\n> -\targc -= 2;\n> -\targv += 2;\n> +\targc = parse_options(argc, argv, NULL,\n> +\t\t\toptions, builtin_bundle_usage,\n> +\t\t\tPARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);\n\nDoesn't this make the usage completely confusing? Before, if I wanted\nto create bundle named \"verify\", I could write\n\n\tgit bundle create verify\n\nAfterwards, not only does that not work, but if I make a typo and\nwrite\n\n\tgit bundle ceate verify\n\nthen it will act like \"git bundle verify ceate\".\n\nI am starting to suspect the first half of patch 1/2 was not such a\ngreat idea, either. :) Do you have other examples of how it would be\nused?\n\nThanks for thining about these things, and hope that helps.\nJonathan\n"},{"id":"180594","messageId":"CALkWK0ksMPjjh8EJjJz6wKS6ciF9x59n3KFauJvnB-1D4d4N1w@mail.gmail.com","threadId":"29109","inReplyTo":"20111208163049.GA2116@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 1/2] parse-options: introduce OPT_SUBCOMMAND","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T16:43:34Z","receivedAt":"2011-12-08T16:43:34Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n>> , and create a new\n>> OPT_SUBCOMMAND; this is built on top of OPTION_BIT to allow for the\n>> detection of more than one subcommand.\n>\n> This part I am not convinced about.  Usually each subcommand takes its\n> own options, so I cannot see this OPT_SUBCOMMAND being actually useful\n> for commands like \"git stash\" or \"git remote\".\n\nHm, what difference does that make? We still have to parse the\nsubcommand, and subsequently use an if-else construct to parse more\noptions depending on the subcommand, no?\n\n-- Ram\n"},{"id":"180595","messageId":"CALkWK0mmjKSzSbxq2i7=JvcB4LTro-MYDCwQLUUwqcf8qS0zPA@mail.gmail.com","threadId":"29109","inReplyTo":"20111208163946.GB2394@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T16:47:39Z","receivedAt":"2011-12-08T16:47:39Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n>\n>> -     if (argc < 3)\n>> -             usage(builtin_bundle_usage);\n>> -\n>> -     cmd = argv[1];\n>> -     bundle_file = argv[2];\n>> -     argc -= 2;\n>> -     argv += 2;\n>> +     argc = parse_options(argc, argv, NULL,\n>> +                     options, builtin_bundle_usage,\n>> +                     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);\n>\n> Doesn't this make the usage completely confusing?  Before, if I wanted\n> to create bundle named \"verify\", I could write\n>\n>        git bundle create verify\n>\n> Afterwards, not only does that not work, but if I make a typo and\n> write\n>\n>        git bundle ceate verify\n>\n> then it will act like \"git bundle verify ceate\".\n\nNo it won't -- it'll just print out the usage because \"verify\" and\n\"create\" are mutually exclusive options. Sure, you can't create a\nbundle named \"verify\", but that's the compromise you'll have to make\nif you don't want to type out \"--\" with each option, no?\n\n> I am starting to suspect the first half of patch 1/2 was not such a\n> great idea, either. :)  Do you have other examples of how it would be\n> used?\n\nHehe, my lack of foresight is disturbing as usual. I'm not yet sure\nit'll be useful.\n\n-- Ram\n"},{"id":"180596","messageId":"20111208165512.GC2394@elie.hsd1.il.comcast.net","threadId":"29109","inReplyTo":"1323349817-15737-6-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/6] t1006 (cat-file): use test_cmp","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-08T16:55:12Z","receivedAt":"2011-12-08T16:55:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Use test_cmp in preference to repeatedly comparing command outputs by\n> hand.\n\nThat could mean one of several things.\n\nIt could mean:\n\n\t1. Use test_cmp instead of open-coding it.\n\t2. Use test_cmp instead of using our knowledge of the underlying\n\t filesystem to retrieve the files from the block device, instead\n\t of relying on the perfectly good operating system facilities\n\t that could take care of it for us\n\t3. Use test_cmp instead of calling a human over to compare command\n\t outputs by eye, which idiomatically might be described as \"by\n\t hand\".\n\nWhat I mean is, I actually don't have much of a clue what you mean by\n\"by hand\". Usually it means \"not automated sufficiently\", but I think\nthat is not the entire problem here (since\n\n\ttest \"$expect\" = \"$actual\"\n\nlooks no less automatic than\n\n\tprintf '%s\\n' \"$expect\" >expect &&\n\tprintf '%s\\n' \"$actual\" >actual &&\n\ttest_cmp expect actual\n\nto me).\n\n> \n> Signed-off-by: Ramkumar Ramachandra \n[...]\n> --- a/t/t1006-cat-file.sh\n> +++ b/t/t1006-cat-file.sh\n> @@ -36,66 +36,41 @@ $content\"\n[...]\n> -\texpect=\"$(maybe_remove_timestamp \"$content\" $no_ts)\"\n> -\tactual=\"$(maybe_remove_timestamp \"$(git cat-file $type $sha1)\" $no_ts)\"\n> -\n> - if test \"z$expect\" = \"z$actual\"\n> -\tthen\n> -\t\t: happy\n> -\telse\n> -\t\techo \"Oops: expected $expect\"\n> -\t\techo \"but got $actual\"\n> -\t\tfalse\n> - fi\n> +\tmaybe_remove_timestamp \"$content\" $no_ts >expect &&\n> +\tmaybe_remove_timestamp \"$(git cat-file $type $sha1)\" $no_ts >actual &&\n> +\ttest_cmp expect actual\n\nMost of the early part of patch proper looks sane from a quick glance.\nWow, the whitespace is a little strange in the original.\n\n[...]\n> @@ -175,30 +153,41 @@ do\n> done\n> \n> test_expect_success \"--batch-check for a non-existent named object\" '\n> - test \"foobar42 missing\n> -foobar84 missing\" = \\\n> - \"$( ( echo foobar42; echo_without_newline foobar84; ) | git cat-file --batch-check)\"\n> + cat >expect <<\\-EOF &&\n> +foobar42 missing\n> +foobar84 missing\n> +EOF\n> + $(echo foobar42; echo_without_newline foobar84) \\\n> + | git cat-file --batch-check >actual &&\n> + test_cmp expect actual\n\nStyle: the | character goes at the end of the first line (think of it\nas a way to save backslashes until they're needed).\n\nHow could this $(...) command substitution possibly work?\n\nLater tests have the same problem, so I'm stopping here.\n\nCiao,\nJonathan\n"},{"id":"180597","messageId":"20111208170319.GD2394@elie.hsd1.il.comcast.net","threadId":"29109","inReplyTo":"CALkWK0mmjKSzSbxq2i7=JvcB4LTro-MYDCwQLUUwqcf8qS0zPA@mail.gmail.com","subject":"Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-08T17:03:19Z","receivedAt":"2011-12-08T17:03:19Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Sure, you can't create a\n> bundle named \"verify\", but that's the compromise you'll have to make\n> if you don't want to type out \"--\" with each option, no?\n\nNo, that's not a compromise I'll have to make. I'm not making it today.\n\nHaving to type \"--\" or prefix with \"./\" to escape ordinary filenames\nthat do not start with \"-\" would be completely weird. This is\nimportant to me: I want you to see it, rather than relying on me as an\nauthority figure to say it (after all, I'm not such a great authority\nanyway --- I make plenty of mistakes).\n"},{"id":"180598","messageId":"20111208170716.GE2394@elie.hsd1.il.comcast.net","threadId":"29109","inReplyTo":"1323349817-15737-7-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 4/6] t3200 (branch): fix '&&' chaining","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-08T17:07:16Z","receivedAt":"2011-12-08T17:07:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Breaks in a test assertion's && chain can potentially hide failures\n> from earlier commands in the chain. Fix these breaks.\n>\n> Additionally, note that 'git branch --help' will fail when git\n\nThis is not \"Additionally, while we're here\" but rather \"In order to\ndo so\".\n\n> manpages aren't already installed: guard the line with a\n> 'test_might_fail'.\n>\n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> t/t3200-branch.sh | 4 ++--\n> 1 files changed, 2 insertions(+), 2 deletions(-)\n\nFor what it's worth,\nAcked-by: Jonathan Nieder \n"},{"id":"180599","messageId":"20111208171213.GF2394@elie.hsd1.il.comcast.net","threadId":"29109","inReplyTo":"1323349817-15737-8-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 5/6] t1510 (worktree): fix '&&' chaining","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-08T17:12:13Z","receivedAt":"2011-12-08T17:12:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Breaks in a test assertion's && chain can potentially hide failures\n> from earlier commands in the chain. Fix these breaks.\n>\n> Additionally, note that 'unset' returns non-zero status when the\n> variable passed was already unset on some shells: change these\n> instances to 'safe_unset'.\n\nThis is also not \"Additionally\", as in \"As a separate change that\nmaybe should have been another patch but I am too lazy\". Rather, it\nis a necessary change that is part of the same task. So I would\nwrite:\n\n\tBreaks in a test assertion's && chain can potentially hide failures\n\tfrom earlier commands in the chain. Fix these breaks.\n\n\t'unset' returns non-zero status when the variable passed was already\n\tunset on some shells, so now that the status is tested we need to\n\tchange these instances to 'safe_unset'.\n\nErm, sane_unset, not safe_unset. Did you even test this?\n\n>\n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> t/t1501-worktree.sh | 6 +++---\n> 1 files changed, 3 insertions(+), 3 deletions(-)\n\nFor what it's worth, after those changes,\nAcked-by: Jonathan Nieder \n"},{"id":"180600","messageId":"20111208171805.GG2394@elie.hsd1.il.comcast.net","threadId":"29109","inReplyTo":"1323349817-15737-9-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 6/6] test: fix '&&' chaining","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-08T17:18:05Z","receivedAt":"2011-12-08T17:18:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Breaks in a test assertion's && chain can potentially hide failures\n> from earlier commands in the chain. Fix instances of this in the\n> following tests:\n> \n> t3419 (rebase-patch-id)\n> t3310 (notes-merge-manual-resolve)\n[...]\n\nThe reader can read the diffstat, so I am not sure this list is very\nuseful.\n\nWith the space gained, it might be helpful to mention that this patch\nonly adds \" &&\" to the ends of lines and that any other kind of change\nhere would be unintentional, to put the reader's mind at ease.\n\n[...]\n>\n> Signed-off-by: Ramkumar Ramachandra \n\nThe patch proper looks good, so if this is tested,\nAcked-by: Jonathan Nieder \n"},{"id":"180601","messageId":"CALkWK0m89D02aku8J0OXbpyrovHCOpsYS4Qpx2jH-pFG4rOG2A@mail.gmail.com","threadId":"29109","inReplyTo":"20111208170319.GD2394@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T17:38:00Z","receivedAt":"2011-12-08T17:38:00Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n>\n>> Sure, you can't create a\n>> bundle named \"verify\", but that's the compromise you'll have to make\n>> if you don't want to type out \"--\" with each option, no?\n>\n> Having to type \"--\" or prefix with \"./\" to escape ordinary filenames\n> that do not start with \"-\" would be completely weird.\n\nHm, do you have any suggestions to work around this? Can we use\nsomething like a parse-stopper after the the first subcommand is\nencountered, and treat the next argument as a non-subcommand (filename\nor whatever else)?\n\n-- Ram\n"},{"id":"180603","messageId":"20111208175913.GK2394@elie.hsd1.il.comcast.net","threadId":"29109","inReplyTo":"CALkWK0m89D02aku8J0OXbpyrovHCOpsYS4Qpx2jH-pFG4rOG2A@mail.gmail.com","subject":"Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-08T17:59:14Z","receivedAt":"2011-12-08T17:59:14Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Jonathan Nieder wrote:\n\n>> Having to type \"--\" or prefix with \"./\" to escape ordinary filenames\n>> that do not start with \"-\" would be completely weird.\n>\n> Hm, do you have any suggestions to work around this? Can we use\n> something like a parse-stopper after the the first subcommand is\n> encountered, and treat the next argument as a non-subcommand (filename\n> or whatever else)?\n\nWhat's the desired behavior? Then we can talk about how to implement it.\n\nIf the goal is \"use parse-options for a command that has subcommands\",\nsee builtin/notes.c.\n"},{"id":"180609","messageId":"CALkWK0nCuFgS8PKkQLMzqpBsOWouSs5y=CEKS1r0x0=LkhqC9A@mail.gmail.com","threadId":"29109","inReplyTo":"20111208175913.GK2394@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T19:39:21Z","receivedAt":"2011-12-08T19:39:21Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder wrote:\n> What's the desired behavior?  Then we can talk about how to implement it.\n>\n> If the goal is \"use parse-options for a command that has subcommands\",\n> see builtin/notes.c.\n\nUses strcmp() to match argv[0]. And you can't specify the options for\na certain subcommand before the subcommand itself on the command-line,\nalthough I don't consider this a serious limitation. I was going for\nsomething prettier with subcommand-specific help text, albeit a\nserious limitation. I'll try working towards this for a few more\nhours to see if anything useful comes out of it -- otherwise, I'll\njust drop this patch and focus on eliminating the ugliness in\nbuiltin/revert.c around '--continue', '--quit' parsing.\n\nThat being said, do you see value in lifting the restriction on\nopts->long_name and PARSE_OPTS_NODASH not allowed together? The\nrestriction seems quite arbitrary, but I can't justify lifting it\nunless I can show some valid usecase.\n\nThanks.\n\n-- Ram\n"},{"id":"180613","messageId":"7vzkf2hm94.fsf@alter.siamese.dyndns.org","threadId":"29109","inReplyTo":"1323349817-15737-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v2 0/6] Fix '&&' chaining in tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-08T19:55:03Z","receivedAt":"2011-12-08T19:55:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> This follows-up $gmane/186481.\n\nI take that you meant \"replaces\". It was confusing especially because you\nseem to have included a few unrelated patches in the thread.\n"},{"id":"180622","messageId":"7viplqhbgs.fsf@alter.siamese.dyndns.org","threadId":"29109","inReplyTo":"CALkWK0nCuFgS8PKkQLMzqpBsOWouSs5y=CEKS1r0x0=LkhqC9A@mail.gmail.com","subject":"Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-08T23:48:03Z","receivedAt":"2011-12-08T23:48:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> That being said, do you see value in lifting the restriction on\n> opts->long_name and PARSE_OPTS_NODASH not allowed together? The\n> restriction seems quite arbitrary, but I can't justify lifting it\n> unless I can show some valid usecase.\n\nTrue and true.\n\nAs to the first \"true\", it is because the nodash was introduced only to\nparse \"find ... \\( ... \\)\" style parentheses as if they are part of option\nsequence, and that use case only needed a single letter.\n\nAs to the second \"true\", it is because so far we didn't need anything\nlonger.\n\nI do not think the name of a subcommand is not a good use case example for\nit, by the way. Unlike parentheses on the command line of \"find\" that can\ncome anywhere and there can be more than one, the subcommand must be the\nfirst thing on the command line and only one subcommand is given at one\ncommand invocation.\n"},{"id":"180623","messageId":"7vehwehaw1.fsf@alter.siamese.dyndns.org","threadId":"29109","inReplyTo":"20111208171213.GF2394@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 5/6] t1510 (worktree): fix '&&' chaining","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-09T00:00:30Z","receivedAt":"2011-12-09T00:00:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> This is also not \"Additionally\", as in \"As a separate change that\n> maybe should have been another patch but I am too lazy\". Rather, it\n> is a necessary change that is part of the same task. So I would\n> write:\n>\n> \tBreaks in a test assertion's && chain can potentially hide failures\n> \tfrom earlier commands in the chain. Fix these breaks.\n>\n> \t'unset' returns non-zero status when the variable passed was already\n> \tunset on some shells, so now that the status is tested we need to\n> \tchange these instances to 'safe_unset'.\n>\n> Erm, sane_unset, not safe_unset. Did you even test this?\n\nThanks for careful reading and many constructive review comments.\n"},{"id":"180629","messageId":"CALkWK0m3XtrU_mZ7MTGpo-MZ8s6mW1HR3WwJd_btaV6HRvZV8A@mail.gmail.com","threadId":"29109","inReplyTo":"7vzkf2hm94.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/6] Fix '&&' chaining in tests","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T05:23:01Z","receivedAt":"2011-12-09T05:23:01Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n> Ramkumar Ramachandra writes:\n>\n>> This follows-up $gmane/186481.\n>\n> I take that you meant \"replaces\".  It was confusing especially because you\n> seem to have included a few unrelated patches in the thread.\n\nYeah. Sorry- stray files were lying around from the previous 'git\nformat-patch' invocation. Which brings me to: I wonder if it makes\nsense to (optionally) check that the directory is empty when executing\n'git format-patch -o '. I sometimes forget to do it by hand.\n\n-- Ram\n"},{"id":"180636","messageId":"CALkWK0nbp465915ysrBXi46mur1dutBDtPNjwW=RdyPV03crzg@mail.gmail.com","threadId":"29109","inReplyTo":"20111208163448.GA2394@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 1/6] t3040 (subprojects-basic): modernize style","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T07:56:49Z","receivedAt":"2011-12-09T07:56:49Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n>\n>> Put the opening quote starting each test on the same line as the\n>> test_expect_* invocation.  While at it:\n>\n> I suspect the above description, while it does describe your patch,\n> does not describe the _reason_ that the patch exists or that someone\n> would want to apply it.  Isn't it something more like:\n> [...]\n\nRight, fixed.\n\n> I didn't read over the patch again.  Has it changed since v1?\n\nNo. I refrained from making other style changes and/ or combining tests.\n\n-- Ram\n"},{"id":"180645","messageId":"1323430158-14885-1-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323349817-15737-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v3 0/6] Fix '&&' chaining in tests","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T11:29:12Z","receivedAt":"2011-12-09T11:29:12Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"This replaces the previous iteration of the series at $gmane/186553.\nThanks to Jonathan for reviewing.\n\n-- Ram\n\nRamkumar Ramachandra (6):\n t3040 (subprojects-basic): fix '&&' chaining, modernize style\n t3030 (merge-recursive): use test_expect_code\n t1006 (cat-file): use test_cmp\n t3200 (branch): fix '&&' chaining\n t1510 (worktree): fix '&&' chaining\n tests: fix '&&' chaining\n\n t/t1006-cat-file.sh | 119 +++++++++++++--------------\n t/t1007-hash-object.sh | 2 +-\n t/t1013-loose-object-format.sh | 2 +-\n t/t1300-repo-config.sh | 2 +-\n t/t1412-reflog-loop.sh | 2 +-\n t/t1501-worktree.sh | 6 +-\n t/t1510-repo-setup.sh | 4 +-\n t/t1511-rev-parse-caret.sh | 2 +-\n t/t3030-merge-recursive.sh | 72 ++---------------\n t/t3040-subprojects-basic.sh | 144 ++++++++++++++++----------------\n t/t3200-branch.sh | 4 +-\n t/t3310-notes-merge-manual-resolve.sh | 10 +-\n t/t3400-rebase.sh | 4 +-\n t/t3418-rebase-continue.sh | 4 +-\n t/t3419-rebase-patch-id.sh | 2 +-\n 15 files changed, 156 insertions(+), 223 deletions(-)\n\n-- \n1.7.7.3\n"},{"id":"180647","messageId":"1323430158-14885-2-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323430158-14885-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/6] t3040 (subprojects-basic): fix '&&' chaining, modernize style","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T11:29:13Z","receivedAt":"2011-12-09T11:29:13Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Breaks in a test assertion's && chain can potentially hide failures\nfrom earlier commands in the chain. Fix instances of this. While at\nit, clean up the style to fit the prevailing style. This means:\n\n- Put the opening quote starting each test on the same line as the\n test_expect_* invocation.\n\n- Indent the file with tabs, not spaces.\n\n- Use test_expect_code() in preference to checking the exit status of\n various statements by hand.\n\n- Guard commands that prepare test input for individual tests in the\n same test_expect_success, so that their scope is clearer and errors\n at that stage can be caught.\n\n- Use <<-\\EOF in preference to <\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t3040-subprojects-basic.sh | 144 +++++++++++++++++++++---------------------\n 1 files changed, 72 insertions(+), 72 deletions(-)\n\ndiff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh\nindex f6973e9..0a4ff6d 100755\n--- a/t/t3040-subprojects-basic.sh\n+++ b/t/t3040-subprojects-basic.sh\n@@ -3,81 +3,81 @@\n test_description='Basic subproject functionality'\n . ./test-lib.sh\n \n-test_expect_success 'Super project creation' \\\n- ': >Makefile &&\n- git add Makefile &&\n- git commit -m \"Superproject created\"'\n-\n-\n-cat >expected <Makefile && git add * &&\n- git commit -q -m \"subproject 1\" ) &&\n- mkdir sub2 &&\n- ( cd sub2 && git init && : >Makefile && git add * &&\n- git commit -q -m \"subproject 2\" ) &&\n- git update-index --add sub1 &&\n- git add sub2 &&\n- git commit -q -m \"subprojects added\" &&\n- git diff-tree --abbrev=5 HEAD^ HEAD |cut -d\" \" -f-3,5- >current &&\n- test_cmp expected current'\n-\n-git branch save HEAD\n-\n-test_expect_success 'check if fsck ignores the subprojects' \\\n- 'git fsck --full'\n-\n-test_expect_success 'check if commit in a subproject detected' \\\n- '( cd sub1 &&\n- echo \"all:\" >>Makefile &&\n- echo \"\ttrue\" >>Makefile &&\n- git commit -q -a -m \"make all\" ) && {\n- git diff-files --exit-code\n-\ttest $? = 1\n- }'\n-\n-test_expect_success 'check if a changed subproject HEAD can be committed' \\\n- 'git commit -q -a -m \"sub1 changed\" && {\n-\tgit diff-tree --exit-code HEAD^ HEAD\n-\ttest $? = 1\n- }'\n-\n-test_expect_success 'check if diff-index works for subproject elements' \\\n- 'git diff-index --exit-code --cached save -- sub1\n- test $? = 1'\n-\n-test_expect_success 'check if diff-tree works for subproject elements' \\\n- 'git diff-tree --exit-code HEAD^ HEAD -- sub1\n- test $? = 1'\n-\n-test_expect_success 'check if git diff works for subproject elements' \\\n- 'git diff --exit-code HEAD^ HEAD\n- test $? = 1'\n-\n-test_expect_success 'check if clone works' \\\n- 'git ls-files -s >expected &&\n- git clone -l -s . cloned &&\n- ( cd cloned && git ls-files -s ) >current &&\n- test_cmp expected current'\n-\n-test_expect_success 'removing and adding subproject' \\\n- 'git update-index --force-remove -- sub2 &&\n- mv sub2 sub3 &&\n- git add sub3 &&\n- git commit -q -m \"renaming a subproject\" && {\n-\tgit diff -M --name-status --exit-code HEAD^ HEAD\n-\ttest $? = 1\n- }'\n+test_expect_success 'setup: create superproject' '\n+\t: >Makefile &&\n+\tgit add Makefile &&\n+\tgit commit -m \"Superproject created\"\n+'\n+\n+test_expect_success 'setup: create subprojects' '\n+\tmkdir sub1 &&\n+\t( cd sub1 && git init && : >Makefile && git add * &&\n+\tgit commit -q -m \"subproject 1\" ) &&\n+\tmkdir sub2 &&\n+\t( cd sub2 && git init && : >Makefile && git add * &&\n+\tgit commit -q -m \"subproject 2\" ) &&\n+\tgit update-index --add sub1 &&\n+\tgit add sub2 &&\n+\tgit commit -q -m \"subprojects added\" &&\n+\tgit diff-tree --abbrev=5 HEAD^ HEAD |cut -d\" \" -f-3,5- >current &&\n+\tgit branch save HEAD &&\n+\tcat >expected <<-\\EOF &&\n+\t:000000 160000 00000... A\tsub1\n+\t:000000 160000 00000... A\tsub2\n+\tEOF\n+\ttest_cmp expected current\n+'\n+\n+test_expect_success 'check if fsck ignores the subprojects' '\n+\tgit fsck --full\n+'\n+\n+test_expect_success 'check if commit in a subproject detected' '\n+\t( cd sub1 &&\n+\techo \"all:\" >>Makefile &&\n+\techo \"\ttrue\" >>Makefile &&\n+\tgit commit -q -a -m \"make all\" ) &&\n+\ttest_expect_code 1 git diff-files --exit-code\n+'\n+\n+test_expect_success 'check if a changed subproject HEAD can be committed' '\n+\tgit commit -q -a -m \"sub1 changed\" &&\n+\ttest_expect_code 1 git diff-tree --exit-code HEAD^ HEAD\n+'\n+\n+test_expect_success 'check if diff-index works for subproject elements' '\n+\ttest_expect_code 1 git diff-index --exit-code --cached save -- sub1\n+'\n+\n+test_expect_success 'check if diff-tree works for subproject elements' '\n+\ttest_expect_code 1 git diff-tree --exit-code HEAD^ HEAD -- sub1\n+'\n+\n+test_expect_success 'check if git diff works for subproject elements' '\n+\ttest_expect_code 1 git diff --exit-code HEAD^ HEAD\n+'\n+\n+test_expect_success 'check if clone works' '\n+\tgit ls-files -s >expected &&\n+\tgit clone -l -s . cloned &&\n+\t( cd cloned && git ls-files -s ) >current &&\n+\ttest_cmp expected current\n+'\n+\n+test_expect_success 'removing and adding subproject' '\n+\tgit update-index --force-remove -- sub2 &&\n+\tmv sub2 sub3 &&\n+\tgit add sub3 &&\n+\tgit commit -q -m \"renaming a subproject\" &&\n+\ttest_expect_code 1 git diff -M --name-status --exit-code HEAD^ HEAD\n+'\n \n # the index must contain the object name the HEAD of the\n # subproject sub1 was at the point \"save\"\n-test_expect_success 'checkout in superproject' \\\n- 'git checkout save &&\n- git diff-index --exit-code --raw --cached save -- sub1'\n+test_expect_success 'checkout in superproject' '\n+\tgit checkout save &&\n+\tgit diff-index --exit-code --raw --cached save -- sub1\n+'\n \n # just interesting what happened...\n # git diff --name-status -M save master\n-- \n1.7.7.3\n"},{"id":"180646","messageId":"1323430158-14885-3-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323430158-14885-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/6] t3030 (merge-recursive): use test_expect_code","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T11:29:14Z","receivedAt":"2011-12-09T11:29:14Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Use test_expect_code in preference to repeatedly checking exit codes\nby hand.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t3030-merge-recursive.sh | 72 ++++----------------------------------------\n 1 files changed, 6 insertions(+), 66 deletions(-)\n\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex 55ef189..a5e3da7 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -285,17 +285,7 @@ test_expect_success 'merge-recursive simple' '\n \trm -fr [abcd] &&\n \tgit checkout -f \"$c2\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c2\" \"$c1\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c2\" \"$c1\"\n '\n \n test_expect_success 'merge-recursive result' '\n@@ -334,17 +324,7 @@ test_expect_success 'merge-recursive remove conflict' '\n \trm -fr [abcd] &&\n \tgit checkout -f \"$c1\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c1\" \"$c5\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c1\" \"$c5\"\n '\n \n test_expect_success 'merge-recursive remove conflict' '\n@@ -388,17 +368,7 @@ test_expect_success 'merge-recursive d/f conflict' '\n \tgit reset --hard &&\n \tgit checkout -f \"$c1\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c1\" \"$c4\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c1\" \"$c4\"\n '\n \n test_expect_success 'merge-recursive d/f conflict result' '\n@@ -422,17 +392,7 @@ test_expect_success 'merge-recursive d/f conflict the other way' '\n \tgit reset --hard &&\n \tgit checkout -f \"$c4\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c4\" \"$c1\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c4\" \"$c1\"\n '\n \n test_expect_success 'merge-recursive d/f conflict result the other way' '\n@@ -456,17 +416,7 @@ test_expect_success 'merge-recursive d/f conflict' '\n \tgit reset --hard &&\n \tgit checkout -f \"$c1\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c1\" \"$c6\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c1\" \"$c6\"\n '\n \n test_expect_success 'merge-recursive d/f conflict result' '\n@@ -490,17 +440,7 @@ test_expect_success 'merge-recursive d/f conflict' '\n \tgit reset --hard &&\n \tgit checkout -f \"$c6\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c6\" \"$c1\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c6\" \"$c1\"\n '\n \n test_expect_success 'merge-recursive d/f conflict result' '\n-- \n1.7.7.3\n"},{"id":"180648","messageId":"1323430158-14885-4-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323430158-14885-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/6] t1006 (cat-file): use test_cmp","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T11:29:15Z","receivedAt":"2011-12-09T11:29:15Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"In testing, a common paradigm involves checking the expected output\nwith the actual output: test-lib provides a test_cmp to show the diff\nbetween the two outputs. So, use this function in preference to\ncalling a human over to compare command outputs by eye.\n\nHelped-by: Jonathan Nieder \nSigned-off-by: Ramkumar Ramachandra \n---\n t/t1006-cat-file.sh | 119 ++++++++++++++++++++++++---------------------------\n 1 files changed, 56 insertions(+), 63 deletions(-)\n\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex d8b7f2f..5be3ce9 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -36,66 +36,41 @@ $content\"\n '\n \n test_expect_success \"Type of $type is correct\" '\n- test $type = \"$(git cat-file -t $sha1)\"\n+\techo $type >expect &&\n+\tgit cat-file -t $sha1 >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success \"Size of $type is correct\" '\n- test $size = \"$(git cat-file -s $sha1)\"\n+\techo $size >expect &&\n+\tgit cat-file -s $sha1 >actual &&\n+\ttest_cmp expect actual\n '\n \n test -z \"$content\" ||\n test_expect_success \"Content of $type is correct\" '\n-\texpect=\"$(maybe_remove_timestamp \"$content\" $no_ts)\"\n-\tactual=\"$(maybe_remove_timestamp \"$(git cat-file $type $sha1)\" $no_ts)\"\n-\n- if test \"z$expect\" = \"z$actual\"\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Oops: expected $expect\"\n-\t\techo \"but got $actual\"\n-\t\tfalse\n- fi\n+\tmaybe_remove_timestamp \"$content\" $no_ts >expect &&\n+\tmaybe_remove_timestamp \"$(git cat-file $type $sha1)\" $no_ts >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success \"Pretty content of $type is correct\" '\n-\texpect=\"$(maybe_remove_timestamp \"$pretty_content\" $no_ts)\"\n-\tactual=\"$(maybe_remove_timestamp \"$(git cat-file -p $sha1)\" $no_ts)\"\n- if test \"z$expect\" = \"z$actual\"\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Oops: expected $expect\"\n-\t\techo \"but got $actual\"\n-\t\tfalse\n- fi\n+\tmaybe_remove_timestamp \"$pretty_content\" $no_ts >expect &&\n+\tmaybe_remove_timestamp \"$(git cat-file -p $sha1)\" $no_ts >actual &&\n+\ttest_cmp expect actual\n '\n \n test -z \"$content\" ||\n test_expect_success \"--batch output of $type is correct\" '\n-\texpect=\"$(maybe_remove_timestamp \"$batch_output\" $no_ts)\"\n-\tactual=\"$(maybe_remove_timestamp \"$(echo $sha1 | git cat-file --batch)\" $no_ts)\"\n- if test \"z$expect\" = \"z$actual\"\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Oops: expected $expect\"\n-\t\techo \"but got $actual\"\n-\t\tfalse\n- fi\n+\tmaybe_remove_timestamp \"$batch_output\" $no_ts >expect &&\n+\tmaybe_remove_timestamp \"$(echo $sha1 | git cat-file --batch)\" $no_ts >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success \"--batch-check output of $type is correct\" '\n-\texpect=\"$sha1 $type $size\"\n-\tactual=\"$(echo_without_newline $sha1 | git cat-file --batch-check)\"\n- if test \"z$expect\" = \"z$actual\"\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Oops: expected $expect\"\n-\t\techo \"but got $actual\"\n-\t\tfalse\n- fi\n+\techo \"$sha1 $type $size\" >expect &&\n+\techo_without_newline $sha1 | git cat-file --batch-check >actual &&\n+\ttest_cmp expect actual\n '\n }\n \n@@ -144,10 +119,13 @@ tag_size=$(strlen \"$tag_content\")\n \n run_tests 'tag' $tag_sha1 $tag_size \"$tag_content\" \"$tag_pretty_content\" 1\n \n-test_expect_success \\\n- \"Reach a blob from a tag pointing to it\" \\\n- \"test '$hello_content' = \\\"\\$(git cat-file blob $tag_sha1)\\\"\"\n+test_expect_success \"Reach a blob from a tag pointing to it\" '\n+ echo_without_newline \"$hello_content\" >expect &&\n+ git cat-file blob \"$tag_sha1\" >actual &&\n+ test_cmp expect actual\n+'\n \n+test_done\n for batch in batch batch-check\n do\n for opt in t s e p\n@@ -175,30 +153,41 @@ do\n done\n \n test_expect_success \"--batch-check for a non-existent named object\" '\n- test \"foobar42 missing\n-foobar84 missing\" = \\\n- \"$( ( echo foobar42; echo_without_newline foobar84; ) | git cat-file --batch-check)\"\n+ cat >expect <<\\-EOF &&\n+foobar42 missing\n+foobar84 missing\n+EOF\n+ echo foobar42; echo_without_newline foobar84 | \\\n+ git cat-file --batch-check >actual &&\n+ test_cmp expect actual\n '\n \n test_expect_success \"--batch-check for a non-existent hash\" '\n- test \"0000000000000000000000000000000000000042 missing\n-0000000000000000000000000000000000000084 missing\" = \\\n- \"$( ( echo 0000000000000000000000000000000000000042;\n- echo_without_newline 0000000000000000000000000000000000000084; ) \\\n- | git cat-file --batch-check)\"\n+ cat >expect <<\\-EOF &&\n+0000000000000000000000000000000000000042 missing\n+0000000000000000000000000000000000000084 missing\n+EOF\n+ echo 0000000000000000000000000000000000000042;\n+ echo_without_newline 0000000000000000000000000000000000000084 | \\\n+ git cat-file --batch-check >actual &&\n+ test_cmp expect actual\n '\n \n test_expect_success \"--batch for an existent and a non-existent hash\" '\n- test \"$tag_sha1 tag $tag_size\n+ cat >expect <<\\-EOF &&\n+tag_sha1 tag $tag_size\n $tag_content\n-0000000000000000000000000000000000000000 missing\" = \\\n- \"$( ( echo $tag_sha1;\n- echo_without_newline 0000000000000000000000000000000000000000; ) \\\n- | git cat-file --batch)\"\n+0000000000000000000000000000000000000000 missing\n+EOF\n+ echo $tag_sha1; echo_without_newline 0000000000000000000000000000000000000000 | \\\n+ git cat-file --batch >actual &&\n+ test_cmp expect_actual\n '\n \n test_expect_success \"--batch-check for an emtpy line\" '\n- test \" missing\" = \"$(echo | git cat-file --batch-check)\"\n+ echo \" missing\" >expect &&\n+ echo | git cat-file --batch-check >actual &&\n+ test_cmp expect actual\n '\n \n batch_input=\"$hello_sha1\n@@ -218,7 +207,10 @@ deadbeef missing\n missing\"\n \n test_expect_success '--batch with multiple sha1s gives correct format' '\n-\ttest \"$(maybe_remove_timestamp \"$batch_output\" 1)\" = \"$(maybe_remove_timestamp \"$(echo_without_newline \"$batch_input\" | git cat-file --batch)\" 1)\"\n+\tmaybe_remove_timestamp \"$batch_output\" 1 >expect &&\n+\tmaybe_remove_timestamp \"echo_without_newline \"$batch_input\" | \\\n+\tgit cat-file --batch\" 1 >actual &&\n+\ttest_cmp expect actual\n '\n \n batch_check_input=\"$hello_sha1\n@@ -237,8 +229,9 @@ deadbeef missing\n missing\"\n \n test_expect_success \"--batch-check with multiple sha1s gives correct format\" '\n- test \"$batch_check_output\" = \\\n- \"$(echo_without_newline \"$batch_check_input\" | git cat-file --batch-check)\"\n+ echo \"$batch_check_output\" >expect &&\n+ echo_without_newline \"$batch_check_input\" | git cat-file --batch-check >actual &&\n+ test_cmp expect actual\n '\n \n test_done\n-- \n1.7.7.3\n"},{"id":"180649","messageId":"1323430158-14885-5-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323430158-14885-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/6] t3200 (branch): fix '&&' chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T11:29:16Z","receivedAt":"2011-12-09T11:29:16Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Breaks in a test assertion's && chain can potentially hide failures\nfrom earlier commands in the chain. Fix these breaks.\n\n'git branch --help' will fail when git manpages aren't already\ninstalled; now that its status is tested, guard these instances with\n'test_might_fail'.\n\nAcked-by: Jonathan Nieder \nSigned-off-by: Ramkumar Ramachandra \n---\n t/t3200-branch.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex bc73c20..95066d0 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -22,7 +22,7 @@ test_expect_success \\\n \n test_expect_success \\\n 'git branch --help should not have created a bogus branch' '\n- git branch --help /dev/null 2>/dev/null;\n+ test_might_fail git branch --help /dev/null 2>/dev/null &&\n test_path_is_missing .git/refs/heads/--help\n '\n \n@@ -88,7 +88,7 @@ test_expect_success \\\n test_expect_success \\\n 'git branch -m n/n n should work' \\\n 'git branch -l n/n &&\n- git branch -m n/n n\n+ git branch -m n/n n &&\n \ttest_path_is_file .git/logs/refs/heads/n'\n \n test_expect_success 'git branch -m o/o o should fail when o/p exists' '\n-- \n1.7.7.3\n"},{"id":"180650","messageId":"1323430158-14885-6-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323430158-14885-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 5/6] t1510 (worktree): fix '&&' chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T11:29:17Z","receivedAt":"2011-12-09T11:29:17Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Breaks in a test assertion's && chain can potentially hide failures\nfrom earlier commands in the chain. Fix these breaks.\n\n'unset' returns non-zero status when the variable passed was already\nunset on some shells; now that its status is tested, change these\ninstances to 'sane_unset'.\n\nAcked-by: Jonathan Nieder \nSigned-off-by: Ramkumar Ramachandra \n---\n t/t1501-worktree.sh | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh\nindex 6384983..e661147 100755\n--- a/t/t1501-worktree.sh\n+++ b/t/t1501-worktree.sh\n@@ -48,7 +48,7 @@ test_expect_success 'setup: helper for testing rev-parse' '\n '\n \n test_expect_success 'setup: core.worktree = relative path' '\n-\tunset GIT_WORK_TREE;\n+\tsane_unset GIT_WORK_TREE &&\n \tGIT_DIR=repo.git &&\n \tGIT_CONFIG=\"$(pwd)\"/$GIT_DIR/config &&\n \texport GIT_DIR GIT_CONFIG &&\n@@ -89,7 +89,7 @@ test_expect_success 'subdir of work tree' '\n '\n \n test_expect_success 'setup: core.worktree = absolute path' '\n-\tunset GIT_WORK_TREE;\n+\tsane_unset GIT_WORK_TREE &&\n \tGIT_DIR=$(pwd)/repo.git &&\n \tGIT_CONFIG=$GIT_DIR/config &&\n \texport GIT_DIR GIT_CONFIG &&\n@@ -334,7 +334,7 @@ test_expect_success 'absolute pathspec should fail gracefully' '\n '\n \n test_expect_success 'make_relative_path handles double slashes in GIT_DIR' '\n-\t>dummy_file\n+\t>dummy_file &&\n \techo git --git-dir=\"$(pwd)//repo.git\" --work-tree=\"$(pwd)\" add dummy_file &&\n \tgit --git-dir=\"$(pwd)//repo.git\" --work-tree=\"$(pwd)\" add dummy_file\n '\n-- \n1.7.7.3\n"},{"id":"180651","messageId":"1323430158-14885-7-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323430158-14885-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 6/6] tests: fix '&&' chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T11:29:18Z","receivedAt":"2011-12-09T11:29:18Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Breaks in a test assertion's && chain can potentially hide failures\nfrom earlier commands in the chain. Fix instances of this by adding\n'&&' at the end of lines where they're missing; this patch doesn't\nintend to make any other changes.\n\nAcked-by: Jonathan Nieder \nSigned-off-by: Ramkumar Ramachandra \n---\n t/t1007-hash-object.sh | 2 +-\n t/t1013-loose-object-format.sh | 2 +-\n t/t1300-repo-config.sh | 2 +-\n t/t1412-reflog-loop.sh | 2 +-\n t/t1510-repo-setup.sh | 4 ++--\n t/t1511-rev-parse-caret.sh | 2 +-\n t/t3310-notes-merge-manual-resolve.sh | 10 +++++-----\n t/t3400-rebase.sh | 4 ++--\n t/t3418-rebase-continue.sh | 4 ++--\n t/t3419-rebase-patch-id.sh | 2 +-\n 10 files changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh\nindex 6d52b82..f83df8e 100755\n--- a/t/t1007-hash-object.sh\n+++ b/t/t1007-hash-object.sh\n@@ -189,7 +189,7 @@ for args in \"-w --stdin-paths\" \"--stdin-paths -w\"; do\n done\n \n test_expect_success 'corrupt tree' '\n-\techo abc >malformed-tree\n+\techo abc >malformed-tree &&\n \ttest_must_fail git hash-object -t tree malformed-tree\n '\n \ndiff --git a/t/t1013-loose-object-format.sh b/t/t1013-loose-object-format.sh\nindex 0a9cedd..fbf5f2f 100755\n--- a/t/t1013-loose-object-format.sh\n+++ b/t/t1013-loose-object-format.sh\n@@ -34,7 +34,7 @@ assert_blob_equals() {\n }\n \n test_expect_success setup '\n-\tcp -R \"$TEST_DIRECTORY/t1013/objects\" .git/\n+\tcp -R \"$TEST_DIRECTORY/t1013/objects\" .git/ &&\n \tgit --version\n '\n \ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 51caff0..0690e0e 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -38,7 +38,7 @@ cat > expect << EOF\n \tWhatEver = Second\n EOF\n test_expect_success 'similar section' '\n-\tgit config Cores.WhatEver Second\n+\tgit config Cores.WhatEver Second &&\n \ttest_cmp expect .git/config\n '\n \ndiff --git a/t/t1412-reflog-loop.sh b/t/t1412-reflog-loop.sh\nindex 647d888..3acd895 100755\n--- a/t/t1412-reflog-loop.sh\n+++ b/t/t1412-reflog-loop.sh\n@@ -20,7 +20,7 @@ test_expect_success 'setup reflog with alternating commits' '\n '\n \n test_expect_success 'reflog shows all entries' '\n-\tcat >expect <<-\\EOF\n+\tcat >expect <<-\\EOF &&\n \t\ttopic@{0} reset: moving to two\n \t\ttopic@{1} reset: moving to one\n \t\ttopic@{2} reset: moving to two\ndiff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh\nindex ec50a9a..80aedfc 100755\n--- a/t/t1510-repo-setup.sh\n+++ b/t/t1510-repo-setup.sh\n@@ -603,7 +603,7 @@ test_expect_success '#22a: core.worktree = GIT_DIR = .git dir' '\n \t# like case #6.\n \n \tsetup_repo 22a \"$here/22a/.git\" \"\" unset &&\n-\tsetup_repo 22ab . \"\" unset\n+\tsetup_repo 22ab . \"\" unset &&\n \tmkdir -p 22a/.git/sub 22a/sub &&\n \tmkdir -p 22ab/.git/sub 22ab/sub &&\n \ttry_case 22a/.git unset . \\\n@@ -742,7 +742,7 @@ test_expect_success '#28: core.worktree and core.bare conflict (gitfile case)' '\n # Case #29: GIT_WORK_TREE(+core.worktree) overrides core.bare (gitfile case).\n test_expect_success '#29: setup' '\n \tsetup_repo 29 non-existent gitfile true &&\n-\tmkdir -p 29/sub/sub 29/wt/sub\n+\tmkdir -p 29/sub/sub 29/wt/sub &&\n \t(\n \t\tcd 29 &&\n \t\tGIT_WORK_TREE=\"$here/29\" &&\ndiff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh\nindex e043cb7..eaefc77 100755\n--- a/t/t1511-rev-parse-caret.sh\n+++ b/t/t1511-rev-parse-caret.sh\n@@ -6,7 +6,7 @@ test_description='tests for ref^{stuff}'\n \n test_expect_success 'setup' '\n \techo blob >a-blob &&\n-\tgit tag -a -m blob blob-tag `git hash-object -w a-blob`\n+\tgit tag -a -m blob blob-tag `git hash-object -w a-blob` &&\n \tmkdir a-tree &&\n \techo moreblobs >a-tree/another-blob &&\n \tgit add . &&\ndiff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh\nindex 4ec4d11..4367197 100755\n--- a/t/t3310-notes-merge-manual-resolve.sh\n+++ b/t/t3310-notes-merge-manual-resolve.sh\n@@ -389,7 +389,7 @@ test_expect_success 'abort notes merge' '\n \ttest_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# m has not moved (still == y)\n-\ttest \"$(git rev-parse refs/notes/m)\" = \"$(cat pre_merge_y)\"\n+\ttest \"$(git rev-parse refs/notes/m)\" = \"$(cat pre_merge_y)\" &&\n \t# Verify that other notes refs has not changed (w, x, y and z)\n \tverify_notes w &&\n \tverify_notes x &&\n@@ -525,9 +525,9 @@ EOF\n \ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&\n \ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&\n \t# Refs are unchanged\n-\ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\"\n-\ttest \"$(git rev-parse refs/notes/y)\" = \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\"\n-\ttest \"$(git rev-parse refs/notes/m)\" != \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\"\n+\ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\" &&\n+\ttest \"$(git rev-parse refs/notes/y)\" = \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\" &&\n+\ttest \"$(git rev-parse refs/notes/m)\" != \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\" &&\n \t# Mention refs/notes/m, and its current and expected value in output\n \tgrep -q \"refs/notes/m\" output &&\n \tgrep -q \"$(git rev-parse refs/notes/m)\" output &&\n@@ -545,7 +545,7 @@ test_expect_success 'resolve situation by aborting the notes merge' '\n \ttest_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# m has not moved (still == w)\n-\ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\"\n+\ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\" &&\n \t# Verify that other notes refs has not changed (w, x, y and z)\n \tverify_notes w &&\n \tverify_notes x &&\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 6eaecec..c355533 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -172,8 +172,8 @@ test_expect_success 'fail when upstream arg is missing and not configured' '\n \n test_expect_success 'default to @{upstream} when upstream arg is missing' '\n \tgit checkout -b default topic &&\n-\tgit config branch.default.remote .\n-\tgit config branch.default.merge refs/heads/master\n+\tgit config branch.default.remote . &&\n+\tgit config branch.default.merge refs/heads/master &&\n \tgit rebase &&\n \ttest \"$(git rev-parse default~1)\" = \"$(git rev-parse master)\"\n '\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 1e855cd..2680375 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -51,7 +51,7 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '\n \ttest_commit \"commit-new-file-F3-on-topic-branch\" F3 32 &&\n \ttest_when_finished \"rm -fr test-bin funny.was.run\" &&\n \tmkdir test-bin &&\n-\tcat >test-bin/git-merge-funny <<-EOF\n+\tcat >test-bin/git-merge-funny <<-EOF &&\n \t#!$SHELL_PATH\n \tcase \"\\$1\" in --opt) ;; *) exit 2 ;; esac\n \tshift &&\n@@ -77,7 +77,7 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '\n test_expect_success 'rebase --continue remembers --rerere-autoupdate' '\n \trm -fr .git/rebase-* &&\n \tgit reset --hard commit-new-file-F3-on-topic-branch &&\n-\tgit checkout master\n+\tgit checkout master &&\n \ttest_commit \"commit-new-file-F3\" F3 3 &&\n \tgit config rerere.enabled true &&\n \ttest_must_fail git rebase -m master topic &&\ndiff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh\nindex bd8efaf..e70ac10 100755\n--- a/t/t3419-rebase-patch-id.sh\n+++ b/t/t3419-rebase-patch-id.sh\n@@ -39,7 +39,7 @@ run()\n }\n \n test_expect_success 'setup' '\n-\tgit commit --allow-empty -m initial\n+\tgit commit --allow-empty -m initial &&\n \tgit tag root\n '\n \n-- \n1.7.7.3\n"},{"id":"180653","messageId":"m34nx9j2fc.fsf@localhost.localdomain","threadId":"29109","inReplyTo":"7viplqhbgs.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-12-09T13:33:18Z","receivedAt":"2011-12-09T13:33:18Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano writes:\n\n> Ramkumar Ramachandra writes:\n> \n> > That being said, do you see value in lifting the restriction on\n> > opts->long_name and PARSE_OPTS_NODASH not allowed together? The\n> > restriction seems quite arbitrary, but I can't justify lifting it\n> > unless I can show some valid usecase.\n> \n> True and true.\n> \n> As to the first \"true\", it is because the nodash was introduced only to\n> parse \"find ... \\( ... \\)\" style parentheses as if they are part of option\n> sequence, and that use case only needed a single letter.\n> \n> As to the second \"true\", it is because so far we didn't need anything\n> longer.\n> \n> I do not think the name of a subcommand is not a good use case example for\n> it, by the way. Unlike parentheses on the command line of \"find\" that can\n> come anywhere and there can be more than one, the subcommand must be the\n> first thing on the command line and only one subcommand is given at one\n> command invocation.\n\nWell, I think it doesn't have to be true: there can be some options\nlike e.g. '-n' / '--dry-run' that are common to all subcommands, and\nin my opinion they could come before subcommand name.\n\nBut if restriction that subcommand name must be first simplifies code,\nthen let's do it this way.\n\n\nI agree that subcommands are and must be mutually exclusive --\notherwise they better be implemented as options, not subcommands.\n\n-- \nJakub Narębski\n"},{"id":"180706","messageId":"7v7h25ebni.fsf@alter.siamese.dyndns.org","threadId":"29109","inReplyTo":"m34nx9j2fc.fsf@localhost.localdomain","subject":"Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-09T18:24:48Z","receivedAt":"2011-12-09T18:24:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Well, I think it doesn't have to be true: there can be some options\n> like e.g. '-n' / '--dry-run' that are common to all subcommands, and\n> in my opinion they could come before subcommand name.\n\nThat is just a crazy talk.\n\nIf you have three subcommands a, b and c, and only a and b shares -n, c\nmust learn to reject the option that does not apply. If you have more\nsubcommands and you need to add an option to one of them, you are forcing\nlogic to reject that new option to all other subcommands.\n\nThat is not a proper way to share option specification. Different\nsubcommands support different options, and if you want to share some\noptions among them, you add a new facility to _allow_ them to share _some_\noptions, while still allowing them to keep their own option specification\ntable. Otherwise the resulting mess will be unmaintainable.\n"},{"id":"180679","messageId":"7vd3bxfvob.fsf@alter.siamese.dyndns.org","threadId":"29109","inReplyTo":"CALkWK0nbp465915ysrBXi46mur1dutBDtPNjwW=RdyPV03crzg@mail.gmail.com","subject":"Re: [PATCH 1/6] t3040 (subprojects-basic): modernize style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-09T18:26:44Z","receivedAt":"2011-12-09T18:26:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n>> I suspect the above description, while it does describe your patch,\n>> does not describe the _reason_ that the patch exists or that someone\n>> would want to apply it.  Isn't it something more like:\n>> [...]\n>\n> Right, fixed.\n\nDoes the \"fixed\" mean \"I fixed it locally; please expect to see it in the\nnext re-roll?\"\n\nJust a quick question, not asking you to always spell it that way.\n"},{"id":"180680","messageId":"CALkWK0nDtdzsqVZQ9F8w8oWQZp__EXo5_LDEpPmiRBfx=J6Rhw@mail.gmail.com","threadId":"29109","inReplyTo":"7vd3bxfvob.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/6] t3040 (subprojects-basic): modernize style","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T18:32:39Z","receivedAt":"2011-12-09T18:32:39Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Junio,\n\nJunio C Hamano wrote:\n> Ramkumar Ramachandra writes:\n>>\n>> Right, fixed.\n>\n> Does the \"fixed\" mean \"I fixed it locally; please expect to see it in the\n> next re-roll?\"\n\nYes, it means exactly that :)\nI respond/ archive the review emails as soon as I fix the\ncorresponding issue locally -- it's the only way to make sure that I\ndon't miss anything (although I miss a few comments despite that).\n\n-- Ram\n"},{"id":"180682","messageId":"7v8vmlfuw1.fsf@alter.siamese.dyndns.org","threadId":"29109","inReplyTo":"1323430158-14885-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/6] t1006 (cat-file): use test_cmp","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-09T18:43:42Z","receivedAt":"2011-12-09T18:43:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> In testing, a common paradigm involves checking the expected output\n> with the actual output: test-lib provides a test_cmp to show the diff\n> between the two outputs. So, use this function in preference to\n> calling a human over to compare command outputs by eye.\n>\n> Helped-by: Jonathan Nieder \n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> t/t1006-cat-file.sh | 119 ++++++++++++++++++++++++---------------------------\n> 1 files changed, 56 insertions(+), 63 deletions(-)\n>\n> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n> index d8b7f2f..5be3ce9 100755\n> --- a/t/t1006-cat-file.sh\n> +++ b/t/t1006-cat-file.sh\n> @@ -144,10 +119,13 @@ tag_size=$(strlen \"$tag_content\")\n> \n> run_tests 'tag' $tag_sha1 $tag_size \"$tag_content\" \"$tag_pretty_content\" 1\n> \n> -test_expect_success \\\n> - \"Reach a blob from a tag pointing to it\" \\\n> - \"test '$hello_content' = \\\"\\$(git cat-file blob $tag_sha1)\\\"\"\n> +test_expect_success \"Reach a blob from a tag pointing to it\" '\n> + echo_without_newline \"$hello_content\" >expect &&\n> + git cat-file blob \"$tag_sha1\" >actual &&\n> + test_cmp expect actual\n> +'\n> \n> +test_done\n> for batch in batch batch-check\n> do\n> for opt in t s e p\n\nWhat is that test_done about?\n"},{"id":"180683","messageId":"CALkWK0kYMttjVgEee3KQaLXOQX86+YnZrm8GsKfu2zjEi_RoYg@mail.gmail.com","threadId":"29109","inReplyTo":"7v8vmlfuw1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/6] t1006 (cat-file): use test_cmp","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T18:47:31Z","receivedAt":"2011-12-09T18:47:31Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n> Ramkumar Ramachandra writes:\n>\n>> In testing, a common paradigm involves checking the expected output\n>> with the actual output: test-lib provides a test_cmp to show the diff\n>> between the two outputs.  So, use this function in preference to\n>> calling a human over to compare command outputs by eye.\n>>\n>> Helped-by: Jonathan Nieder \n>> Signed-off-by: Ramkumar Ramachandra \n>> ---\n>>  t/t1006-cat-file.sh |  119 ++++++++++++++++++++++++---------------------------\n>>  1 files changed, 56 insertions(+), 63 deletions(-)\n>>\n>> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n>> index d8b7f2f..5be3ce9 100755\n>> --- a/t/t1006-cat-file.sh\n>> +++ b/t/t1006-cat-file.sh\n>> @@ -144,10 +119,13 @@ tag_size=$(strlen \"$tag_content\")\n>>\n>>  run_tests 'tag' $tag_sha1 $tag_size \"$tag_content\" \"$tag_pretty_content\" 1\n>>\n>> -test_expect_success \\\n>> -    \"Reach a blob from a tag pointing to it\" \\\n>> -    \"test '$hello_content' = \\\"\\$(git cat-file blob $tag_sha1)\\\"\"\n>> +test_expect_success \"Reach a blob from a tag pointing to it\" '\n>> +    echo_without_newline \"$hello_content\" >expect &&\n>> +    git cat-file blob \"$tag_sha1\" >actual &&\n>> +    test_cmp expect actual\n>> +'\n>>\n>> +test_done\n>>  for batch in batch batch-check\n>>  do\n>>      for opt in t s e p\n>\n> What is that test_done about?\n\n:facepalm: crept in while testing -- I often (temporarily) use a\ntest_done after the test I want to inspect so the others aren't\nunnecessarily executed hiding the error.\n\nThanks for catching this.\n\n-- Ram\n"},{"id":"181228","messageId":"1323967528-10537-1-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"20111208175913.GK2394@elie.hsd1.il.comcast.net","subject":"[PATCH 0/2] Improve git-bundle builtin","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-15T16:45:26Z","receivedAt":"2011-12-15T16:45:26Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nMy OPT_SUBCOMMAND idea crashed and burned; I decided to salvage some\nof the work that went into improving the git-bundle builtin and put it\ninto another series along with some additional tests. I hope this\nbenefits people who use git-bundle to do incremental backups on their\nservers or otherwise.\n\nA couple of thoughts:\n\n1. There's a SP between the OBJID and the ref name in list-heads as\nopposed to the TAB used by other git commands such as ls-remote,\ndiff-tree. Will fixing it break someone's parser somewhere?\n\n2. Is it worth fixing the \"--stdin\" tests? What is the usecase? A\nquick blame points to f62e0a39 (t5704 (bundle): add tests for bundle\n--stdin, 2010-04-19): Jonathan, Joey?\n\nCheers.\n\n-- Ram\n\nRamkumar Ramachandra (2):\n t5704 (bundle): rewrite for larger coverage\n bundle: rewrite builtin to use parse-options\n\n builtin/bundle.c | 91 +++++++++++++++++++++++++++++---------------------\n t/t5704-bundle.sh | 95 ++++++++++++++++++++++++++++++++++++++--------------\n 2 files changed, 122 insertions(+), 64 deletions(-)\n\n-- \n1.7.4.1\n"},{"id":"181229","messageId":"1323967528-10537-2-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323967528-10537-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/2] t5704 (bundle): rewrite for larger coverage","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-15T16:45:27Z","receivedAt":"2011-12-15T16:45:27Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Rewrite the git-bundle testsuite to exercise more of its\nfunctionality.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t5704-bundle.sh | 95 ++++++++++++++++++++++++++++++++++++++--------------\n 1 files changed, 69 insertions(+), 26 deletions(-)\n\ndiff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\nindex 728ccd8..09ff4f1 100755\n--- a/t/t5704-bundle.sh\n+++ b/t/t5704-bundle.sh\n@@ -1,56 +1,99 @@\n #!/bin/sh\n \n-test_description='some bundle related tests'\n+test_description='Test git-bundle'\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n+\tgit config core.logAllRefUpdates false &&\n+\ttest_commit initial &&\n+\tgit checkout -b branch &&\n+\ttest_commit second &&\n+\ttest_commit third &&\n+\tgit checkout master &&\n+\ttest_commit fourth file\n+'\n \n-\t: > file &&\n-\tgit add file &&\n-\ttest_tick &&\n-\tgit commit -m initial &&\n-\ttest_tick &&\n-\tgit tag -m tag tag &&\n-\t: > file2 &&\n-\tgit add file2 &&\n-\t: > file3 &&\n-\ttest_tick &&\n-\tgit commit -m second &&\n-\tgit add file3 &&\n-\ttest_tick &&\n-\tgit commit -m third\n+test_expect_success 'create succeeds' '\n+\tgit bundle create bundle second third &&\n+\tcat >expect <<-\\EOF &&\n+\tOBJID\trefs/tags/third\n+\tOBJID\trefs/tags/second\n+\tEOF\n+\t{\n+\t\tgit ls-remote bundle |\n+\t\tsed \"s/$_x40/OBJID/g\"\n+\t} >actual &&\n+\ttest_cmp expect actual\n+'\n \n+test_expect_success 'verify succeeds' '\n+\tgit bundle create bundle second third &&\n+\tgit bundle verify bundle\n '\n \n-test_expect_success 'tags can be excluded by rev-list options' '\n+test_expect_success 'list-heads succeeds' '\n+\tgit bundle create bundle second third &&\n+\tcat >expect <<-\\EOF &&\n+\tOBJID refs/tags/second\n+\tOBJID refs/tags/third\n+\tEOF\n+\t{\n+\t\tgit bundle list-heads bundle |\n+\t\tsed \"s/$_x40/OBJID/g\"\n+\t} >actual &&\n+\ttest_cmp expect actual\n+'\n \n-\tgit bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&\n-\tgit ls-remote bundle > output &&\n-\t! grep tag output\n+test_expect_success 'create dies on invalid bundle filename' '\n+\tmkdir adir &&\n+\ttest_expect_code 128 git bundle create adir --all\n+'\n \n+test_expect_success 'disallow stray command-line options' '\n+\ttest_must_fail git bundle create --junk bundle second third\n '\n \n-test_expect_success 'die if bundle file cannot be created' '\n+test_expect_failure 'disallow stray command-line arguments' '\n+\tgit bundle create bundle second third &&\n+\ttest_must_fail git bundle verify bundle junk\n+'\n \n-\tmkdir adir &&\n-\ttest_must_fail git bundle create adir --all\n+test_expect_success 'create accepts rev-list options to narrow refs' '\n+\tgit bundle create bundle --all -- file &&\n+\tcat >expect <<-\\EOF &&\n+\tOBJID\tHEAD\n+\tOBJID\trefs/tags/fourth\n+\tOBJID\trefs/heads/master\n+\tEOF\n+\t{\n+\t\tgit ls-remote bundle |\n+\t\tsed \"s/$_x40/OBJID/g\"\n+\t} >actual &&\n+\ttest_cmp expect actual\n+'\n \n+test_expect_success 'unbundle succeeds' '\n+\tsaved=$(git rev-parse third) &&\n+\tgit bundle create bundle second third fourth &&\n+\tgit tag -d second third fourth &&\n+\tgit branch -D branch &&\n+\tgit reset --hard initial &&\n+\tgit prune &&\n+\ttest_must_fail git reset --hard $saved &&\n+\tgit bundle unbundle bundle &&\n+\tgit reset --hard $saved\n '\n \n test_expect_failure 'bundle --stdin' '\n-\n \techo master | git bundle create stdin-bundle.bdl --stdin &&\n \tgit ls-remote stdin-bundle.bdl >output &&\n \tgrep master output\n-\n '\n \n test_expect_failure 'bundle --stdin ' '\n-\n \techo master | git bundle create hybrid-bundle.bdl --stdin tag &&\n \tgit ls-remote hybrid-bundle.bdl >output &&\n \tgrep master output\n-\n '\n \n test_done\n-- \n1.7.4.1\n"},{"id":"181230","messageId":"1323967528-10537-3-git-send-email-artagnon@gmail.com","threadId":"29109","inReplyTo":"1323967528-10537-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/2] bundle: rewrite builtin to use parse-options","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-15T16:45:28Z","receivedAt":"2011-12-15T16:45:28Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The git-bundle builtin currently parses command-line options by hand;\nthis is fragile, and reports cryptic errors on failure. Use the\nparse-options library to do the parsing instead.\n\nEncouraged-by: Jonathan Nieder \nSigned-off-by: Ramkumar Ramachandra \n---\n builtin/bundle.c | 91 +++++++++++++++++++++++++++++++----------------------\n t/t5704-bundle.sh | 2 +-\n 2 files changed, 54 insertions(+), 39 deletions(-)\n\ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex 92a8a60..13ed770 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -1,5 +1,6 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n+#include \"parse-options.h\"\n #include \"bundle.h\"\n \n /*\n@@ -9,57 +10,71 @@\n * bundle supporting \"fetch\", \"pull\", and \"ls-remote\".\n */\n \n-static const char builtin_bundle_usage[] =\n- \"git bundle create \\n\"\n- \" or: git bundle verify \\n\"\n- \" or: git bundle list-heads [...]\\n\"\n- \" or: git bundle unbundle [...]\";\n+static const char * builtin_bundle_usage[] = {\n+\t\"git bundle create \",\n+\t\"git bundle verify \",\n+\t\"git bundle list-heads [...]\",\n+\t\"git bundle unbundle [...]\",\n+\tNULL\n+};\n \n int cmd_bundle(int argc, const char **argv, const char *prefix)\n {\n-\tstruct bundle_header header;\n-\tconst char *cmd, *bundle_file;\n+\tint prefix_length;\n \tint bundle_fd = -1;\n-\tchar buffer[PATH_MAX];\n+\tconst char *subcommand, *bundle_file;\n+\tstruct bundle_header header;\n+\tstruct option options[] = { OPT_END() };\n \n-\tif (argc < 3)\n-\t\tusage(builtin_bundle_usage);\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\tbuiltin_bundle_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n \n-\tcmd = argv[1];\n-\tbundle_file = argv[2];\n-\targc -= 2;\n-\targv += 2;\n+\tif (argc < 2)\n+\t\tusage_with_options(builtin_bundle_usage, options);\n+\tsubcommand = argv[0];\n \n-\tif (prefix && bundle_file[0] != '/') {\n-\t\tsnprintf(buffer, sizeof(buffer), \"%s/%s\", prefix, bundle_file);\n-\t\tbundle_file = buffer;\n-\t}\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\tbuiltin_bundle_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n+\n+\t/* Disallow stray arguments */\n+\tif ((strcmp(subcommand, \"create\") && argc > 2) ||\n+\t\t(!strcmp(subcommand, \"verify\") && argc > 1))\n+\t\tusage_with_options(builtin_bundle_usage, options);\n \n-\tmemset(&header, 0, sizeof(header));\n-\tif (strcmp(cmd, \"create\") && (bundle_fd =\n-\t\t\t\tread_bundle_header(bundle_file, &header)) < 0)\n-\t\treturn 1;\n+\tprefix_length = prefix ? strlen(prefix) : 0;\n+\tbundle_file = prefix_filename(prefix, prefix_length, argv[0]);\n \n-\tif (!strcmp(cmd, \"verify\")) {\n+\t/* Read out bundle header, except in the \"create\" case */\n+\tif (strcmp(subcommand, \"create\")) {\n+\t\tmemset(&header, 0, sizeof(header));\n+\t\tbundle_fd = read_bundle_header(bundle_file, &header);\n+\t\tif (bundle_fd < 0)\n+\t\t\tdie_errno(_(\"Failed to open bundle file '%s'\"), bundle_file);\n+\t}\n+\n+\tif (!strcmp(subcommand, \"create\")) {\n+\t\tif (!startup_info->have_repository)\n+\t\t\tdie(_(\"Need a repository to create a bundle.\"));\n+\t\treturn create_bundle(&header, bundle_file, argc, argv);\n+\t} else if (!strcmp(subcommand, \"verify\")) {\n \t\tclose(bundle_fd);\n \t\tif (verify_bundle(&header, 1))\n-\t\t\treturn 1;\n+\t\t\treturn -1; /* Error already reported */\n \t\tfprintf(stderr, _(\"%s is okay\\n\"), bundle_file);\n-\t\treturn 0;\n-\t}\n-\tif (!strcmp(cmd, \"list-heads\")) {\n+\t} else if (!strcmp(subcommand, \"list-heads\")) {\n \t\tclose(bundle_fd);\n-\t\treturn !!list_bundle_refs(&header, argc, argv);\n-\t}\n-\tif (!strcmp(cmd, \"create\")) {\n-\t\tif (!startup_info->have_repository)\n-\t\t\tdie(_(\"Need a repository to create a bundle.\"));\n-\t\treturn !!create_bundle(&header, bundle_file, argc, argv);\n-\t} else if (!strcmp(cmd, \"unbundle\")) {\n-\t\tif (!startup_info->have_repository)\n+\t\treturn list_bundle_refs(&header, argc, argv);\n+\t} else if (!strcmp(subcommand, \"unbundle\")) {\n+\t\tif (!startup_info->have_repository) {\n+\t\t\tclose(bundle_fd);\n \t\t\tdie(_(\"Need a repository to unbundle.\"));\n-\t\treturn !!unbundle(&header, bundle_fd, 0) ||\n+\t\t}\n+\t\treturn unbundle(&header, bundle_fd, 0) ||\n \t\t\tlist_bundle_refs(&header, argc, argv);\n-\t} else\n-\t\tusage(builtin_bundle_usage);\n+\t} else {\n+\t\tclose(bundle_fd);\n+\t\tusage_with_options(builtin_bundle_usage, options);\n+\t}\n+\n+\treturn 0;\n }\ndiff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\nindex 09ff4f1..8e3f677 100755\n--- a/t/t5704-bundle.sh\n+++ b/t/t5704-bundle.sh\n@@ -53,7 +53,7 @@ test_expect_success 'disallow stray command-line options' '\n \ttest_must_fail git bundle create --junk bundle second third\n '\n \n-test_expect_failure 'disallow stray command-line arguments' '\n+test_expect_success 'disallow stray command-line arguments' '\n \tgit bundle create bundle second third &&\n \ttest_must_fail git bundle verify bundle junk\n '\n-- \n1.7.4.1\n"},{"id":"181248","messageId":"20111215205340.GA16160@elie.hsd1.il.comcast.net","threadId":"29109","inReplyTo":"1323967528-10537-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 0/2] Improve git-bundle builtin","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-15T20:54:56Z","receivedAt":"2011-12-15T20:54:56Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Ram,\n\nRamkumar Ramachandra wrote:\n\n> 1. There's a SP between the OBJID and the ref name in list-heads as\n> opposed to the TAB used by other git commands such as ls-remote,\n> diff-tree. Will fixing it break someone's parser somewhere?\n\nI don't know. Would there be any advantage at all to changing the\noutput format of the tool? Bad idea.\n\nIf the goal is to avoid confusion, perhaps a note in the documentation\nwould help.\n\n> 2. Is it worth fixing the \"--stdin\" tests? What is the usecase?\n\nIs \"script that wants to list which revs to bundle, possibly exceeding\nthe command-line length limit\" not enough of a use case? Yes, I think\nit is very much worth fixing.\n\nThanks for looking at this.\nJonathan\n"},{"id":"181252","messageId":"20111215211632.GB16160@elie.hsd1.il.comcast.net","threadId":"29109","inReplyTo":"1323967528-10537-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 1/2] t5704 (bundle): rewrite for larger coverage","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-15T21:16:32Z","receivedAt":"2011-12-15T21:16:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nRamkumar Ramachandra wrote:\n\n> Rewrite\n\nAlways a scary word. Very rarely justified, especially when the\noriginal and rewritten versions of something are not going to be able\nto coexist for a period while the bugs are ironed out. It doesn't\nleave me optimistic.\n\n> the git-bundle testsuite to exercise more of its\n> functionality.\n\nLuckily, this goal suggests that I am going to see some new tests\nadded, without the existing coverage being removed or mangled, so\nmaybe I can ignore the fears awakened by the word \"Rewrite\". Let's\nsee...\n\n[...]\n> --- a/t/t5704-bundle.sh\n> +++ b/t/t5704-bundle.sh\n> @@ -1,56 +1,99 @@\n> #!/bin/sh\n> \n> -test_description='some bundle related tests'\n> +test_description='Test git-bundle'\n\nWhy?\n\n> . ./test-lib.sh\n> \n> test_expect_success 'setup' '\n> +\tgit config core.logAllRefUpdates false &&\n\nWhy?\n\n> +\ttest_commit initial &&\n> +\tgit checkout -b branch &&\n> +\ttest_commit second &&\n> +\ttest_commit third &&\n> +\tgit checkout master &&\n> +\ttest_commit fourth file\n> +'\n\nNo explicit tags in the setup this time. Now all commits are referred\nto by tags, which worsens the coverage, since if some future change\ncaused commits not referred to by a tag to be dropped, it would be\nmissed. Paraphrasing\n\n\t>file &&\n\tgit add file &&\n\ttest_tick &&\n\tgit commit -m initial &&\n\tgit tag -m initial initial\n\nto\n\n\ttest_commit initial file\n\nwhen not preparing to make some other change in the same place and if\nthe original was not too confusing feels like gratuitous churn.\n\n[...]\n> +test_expect_success 'create succeeds' '\n> +\tgit bundle create bundle second third &&\n> +\tcat >expect <<-\\EOF &&\n> +\tOBJID\trefs/tags/third\n> +\tOBJID\trefs/tags/second\n> +\tEOF\n> +\t{\n> +\t\tgit ls-remote bundle |\n> +\t\tsed \"s/$_x40/OBJID/g\"\n> +\t} >actual &&\n> +\ttest_cmp expect actual\n> +'\n\nA new test. What assertion is it testing? Why censor out the\nobject names when comparing the expected object names to the\nactual ones, instead of computing the appropriate object names\nfor the expected result? Is this new test useful, or does it\ncover ground already tested in t5510-fetch.sh?\n\n> +test_expect_success 'verify succeeds' '\n> +\tgit bundle create bundle second third &&\n> +\tgit bundle verify bundle\n> '\n\nA test for \"git bundle verify\" is a welcome addition.\n\n> +test_expect_success 'list-heads succeeds' '\n> +\tgit bundle create bundle second third &&\n> +\tcat >expect <<-\\EOF &&\n> +\tOBJID refs/tags/second\n> +\tOBJID refs/tags/third\n> +\tEOF\n> +\t{\n> +\t\tgit bundle list-heads bundle |\n> +\t\tsed \"s/$_x40/OBJID/g\"\n> +\t} >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n\nBased on 'git grep -e \"git bundle list-heads\" -- t', there don't seem\nto be any existing tests for \"git bundle list-heads\" except for\nt5510-fetch.sh, but I'm not sure what this adds on top of that one.\n\n> -test_expect_success 'tags can be excluded by rev-list options' '\n> -\n> -\tgit bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&\n> -\tgit ls-remote bundle > output &&\n> -\t! grep tag output\n\nDropped?\n\n> +test_expect_success 'create dies on invalid bundle filename' '\n> +\tmkdir adir &&\n> +\ttest_expect_code 128 git bundle create adir --all\n> +'\n\nHow is \"invalid bundle filename\" a clearer explanation than \"bundle\nfile cannot be created\"?\n\n> \n> +test_expect_success 'disallow stray command-line options' '\n> +\ttest_must_fail git bundle create --junk bundle second third\n> '\n\nOk. Might also be useful to check that no \"--junk\" or \"bundle\" file\nis created.\n\n> +test_expect_failure 'disallow stray command-line arguments' '\n> +\tgit bundle create bundle second third &&\n> +\ttest_must_fail git bundle verify bundle junk\n> +'\n\nIn this case, \"stray command-line arguments\" actually means \"extra\narguments to 'verify'\", I guess?\n\nWhat happens if I run \"git bundle verify *.bundle\" in a directory\nwith multiple bundles? What should happen?\n\n> +test_expect_success 'create accepts rev-list options to narrow refs' '\n> +\tgit bundle create bundle --all -- file &&\n\nI don't understand what \"options to narrow refs\" means. Does that\nmean options like --remotes=origin which yield refs from some subset\nof the ref namespace, unlike --all?\n\n[...]\n> +test_expect_success 'unbundle succeeds' '\n\nA test for \"git bundle unbundle\" is a welcome addition.\n\n[...]\n> test_expect_failure 'bundle --stdin' '\n> -\n> \techo master | git bundle create stdin-bundle.bdl --stdin &&\n> \tgit ls-remote stdin-bundle.bdl >output &&\n> \tgrep master output\n> -\n> '\n\nSeems like a gratuitous change to mix into a patch that introduces\nfunctional changes.\n\nI found this hard to review, since it doesn't seem very focussed ---\nit mixes style cleanups, removal of code, and introduction of new\ncode. I'd be way happier to see a new patch that just adds new tests\nto the script without potentially breaking anything on the way. Then\nif the style cleanups still seem important to you, they can be\nreviewed as a separate patch.\n\nHoping that clarifies a little,\nJonathan\n"},{"id":"181255","messageId":"20111215212901.GC16160@elie.hsd1.il.comcast.net","threadId":"29109","inReplyTo":"1323967528-10537-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-15T21:29:01Z","receivedAt":"2011-12-15T21:29:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> The git-bundle builtin currently parses command-line options by hand;\n> this is fragile, and reports cryptic errors on failure. Use the\n> parse-options library to do the parsing instead.\n\nI don't understand how this is fragile. I haven't actually run into\nerror messages from \"git bundle\" I found to be cryptic, but if they\nare, they surely can be improved locally. Could you give an example\nor something?\n\n> Encouraged-by: Jonathan Nieder \n\nNo, not encouraged.\n\nBut parseoptification does have some nice benefits, so let's see how\nthe patch looks...\n\n[...]\n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> builtin/bundle.c | 91 +++++++++++++++++++++++++++++++----------------------\n> t/t5704-bundle.sh | 2 +-\n> 2 files changed, 54 insertions(+), 39 deletions(-)\n> \n> diff --git a/builtin/bundle.c b/builtin/bundle.c\n> index 92a8a60..13ed770 100644\n> --- a/builtin/bundle.c\n> +++ b/builtin/bundle.c\n[...]\n> @@ -9,57 +10,71 @@\n[...]\n> int cmd_bundle(int argc, const char **argv, const char *prefix)\n> {\n> -\tstruct bundle_header header;\n> -\tconst char *cmd, *bundle_file;\n> +\tint prefix_length;\n> \tint bundle_fd = -1;\n> -\tchar buffer[PATH_MAX];\n> -\tif (argc < 3)\n> -\t\tusage(builtin_bundle_usage);\n> +\tconst char *subcommand, *bundle_file;\n> +\tstruct bundle_header header;\n> +\tstruct option options[] = { OPT_END() };\n> +\n> +\targc = parse_options(argc, argv, prefix, options,\n> +\t\t\tbuiltin_bundle_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n\nNo, just no. Using parse-options with an empty option table is\ncomplete overkill for handling the \"-h\" option. Without a lot more\njustification, this doesn't make it seem more sane or readable at all.\n\nStopping here. I wouldn't mind seeing \"git bundle\" being\nparseoptified, but not if the result looks like this.\n\nI _do_ think that a systematic option-parsing library that handles\nsubcommands would be something possible and probably useful for git.\nIts input might include a table with subcommand names, an option table\nfor each, and a function to call when that subcommand is used:\n\n\tstruct parseopt_subcommand subcmds[] = {\n\t\t{ \"list\", no_options, notes_list },\n\t\t{ \"add\", add_options, notes_add },\n\t\t{ \"copy\", copy_options, notes_copy },\n\t\t{ \"append\", append_options, notes_append },\n\t\t{ \"edit\", no_options, notes_edit },\n\t\t{ \"show\", no_options, notes_show },\n\t\t...\n\t};\n\nThen \"git notes -h\" might be able to automatically generate a table of\nsynopses, and \"gite notes --help-all\" might print help for all\nsubcommands. Something like that.\n\nHope that helps,\nJonathan\n"},{"id":"181256","messageId":"7vobv9msjl.fsf@alter.siamese.dyndns.org","threadId":"29109","inReplyTo":"1323967528-10537-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 0/2] Improve git-bundle builtin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-15T21:30:38Z","receivedAt":"2011-12-15T21:30:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> A couple of thoughts:\n>\n> 1. There's a SP between the OBJID and the ref name in list-heads as\n> opposed to the TAB used by other git commands such as ls-remote,\n> diff-tree. Will fixing it break someone's parser somewhere?\n\nIf somebody will get broken when you change something, that change is not\n\"fixing\" but just \"changing\". Why do you even want to \"fix\" it?\n\n> 2. Is it worth fixing the \"--stdin\" tests? What is the usecase?\n\nHaving to pack too many refs that would not fit on a command line length\nlimit, which is not an unusual requirement.\n"}]}