{"thread":{"id":"27902","subject":"[PATCH 0/2 v2] ls-tree: exit with non-zero status on error","startedAt":"2011-07-24T01:07:48Z","lastAt":"2011-07-24T13:42:06Z","messageCount":7,"participants":["Jon Seymour","Junio C Hamano","Jens Lehmann"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"171903","messageId":"1311469670-15392-1-git-send-email-jon.seymour@gmail.com","threadId":"27902","inReplyTo":null,"subject":"[PATCH 0/2 v2] ls-tree: exit with non-zero status on error","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2011-07-24T01:07:48Z","receivedAt":"2011-07-24T01:07:48Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"While working with a damaged repository, I noticed that git ls-tree was reporting an error \neven though it set a zero exit code. \n\nThis patch uses the return code from read_tree_recursive instead.\n\n| v2: Amended test per Jens Lehmann's suggestion.\n\nJon Seymour (2):\n Add a test to check that git ls-tree sets non-zero exit code on\n error.\n Ensure git ls-tree exits with a non-zero exit code if\n read_tree_recursive fails.\n\n builtin/ls-tree.c | 6 +++---\n t/t3103-ls-tree-missing-tree.sh | 19 +++++++++++++++++++\n 2 files changed, 22 insertions(+), 3 deletions(-)\n create mode 100755 t/t3103-ls-tree-missing-tree.sh\n\n-- \n1.7.6.347.g6a5a9c\n"},{"id":"171904","messageId":"1311469670-15392-2-git-send-email-jon.seymour@gmail.com","threadId":"27902","inReplyTo":"1311469670-15392-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH 1/2] Add a test to check that git ls-tree sets non-zero exit code on error.","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2011-07-24T01:07:49Z","receivedAt":"2011-07-24T01:07:49Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"Expected to fail at this commit, fixed by subsequent commit.\n\nImproved-by: Jens Lehmann \nSigned-off-by: Jon Seymour \n---\n t/t3103-ls-tree-missing-tree.sh | 19 +++++++++++++++++++\n 1 files changed, 19 insertions(+), 0 deletions(-)\n create mode 100755 t/t3103-ls-tree-missing-tree.sh\n\ndiff --git a/t/t3103-ls-tree-missing-tree.sh b/t/t3103-ls-tree-missing-tree.sh\nnew file mode 100755\nindex 0000000..cd17fa7\n--- /dev/null\n+++ b/t/t3103-ls-tree-missing-tree.sh\n@@ -0,0 +1,19 @@\n+#!/bin/sh\n+\n+test_description='ls-tree exits with non-zero status if it also reports an error'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\tmkdir a &&\n+\ttouch a/one &&\n+\tgit add a/one &&\n+\tgit commit -m test\n+'\n+\n+test_expect_failure 'ls-tree fails with non-zero exit code on broken tree' '\n+\trm -f .git/objects/5f/cffbd6e4c5c5b8d81f5e9314b20e338e3ffff5 &&\n+\ttest_must_fail git ls-tree -r HEAD\n+'\n+\n+test_done\n-- \n1.7.6.347.g96e0b\n"},{"id":"171905","messageId":"1311469670-15392-3-git-send-email-jon.seymour@gmail.com","threadId":"27902","inReplyTo":"1311469670-15392-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH 2/2] Ensure git ls-tree exits with a non-zero exit code if read_tree_recursive fails.","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2011-07-24T01:07:50Z","receivedAt":"2011-07-24T01:07:50Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"In the case of a corrupt repository, git ls-tree may report an error but\npresently it exits with a code of 0.\n\nThis change uses the return code of read_tree_recursive instead.\n\nImproved-by: Jens Lehmann \nSigned-off-by: Jon Seymour \n---\n builtin/ls-tree.c | 6 +++---\n t/t3103-ls-tree-missing-tree.sh | 2 +-\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex f08c5b0..6d6c992 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -120,7 +120,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n {\n \tunsigned char sha1[20];\n \tstruct tree *tree;\n-\tint i, full_tree = 0;\n+\tint i, full_tree = 0, err;\n \tconst struct option ls_tree_options[] = {\n \t\tOPT_BIT('d', NULL, &ls_options, \"only show trees\",\n \t\t\tLS_TREE_ONLY),\n@@ -173,7 +173,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \ttree = parse_tree_indirect(sha1);\n \tif (!tree)\n \t\tdie(\"not a tree object\");\n-\tread_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n+\terr = read_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n \n-\treturn 0;\n+\treturn err;\n }\ndiff --git a/t/t3103-ls-tree-missing-tree.sh b/t/t3103-ls-tree-missing-tree.sh\nindex cd17fa7..365ac07 100755\n--- a/t/t3103-ls-tree-missing-tree.sh\n+++ b/t/t3103-ls-tree-missing-tree.sh\n@@ -11,7 +11,7 @@ test_expect_success 'setup' '\n \tgit commit -m test\n '\n \n-test_expect_failure 'ls-tree fails with non-zero exit code on broken tree' '\n+test_expect_success 'ls-tree fails with non-zero exit code on broken tree' '\n \trm -f .git/objects/5f/cffbd6e4c5c5b8d81f5e9314b20e338e3ffff5 &&\n \ttest_must_fail git ls-tree -r HEAD\n '\n-- \n1.7.6.347.g96e0b\n"},{"id":"171925","messageId":"7vipqsw29v.fsf@alter.siamese.dyndns.org","threadId":"27902","inReplyTo":"1311469670-15392-2-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH 1/2] Add a test to check that git ls-tree sets non-zero exit code on error.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-07-24T07:45:00Z","receivedAt":"2011-07-24T07:45:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jon Seymour writes:\n\n> t/t3103-ls-tree-missing-tree.sh | 19 +++++++++++++++++++\n> 1 files changed, 19 insertions(+), 0 deletions(-)\n> create mode 100755 t/t3103-ls-tree-missing-tree.sh\n\nI'd rather not waste a new test number for something trivial like this.\nDon't we already have a script that tests ls-tree?\n"},{"id":"171926","messageId":"7vei1gw28w.fsf@alter.siamese.dyndns.org","threadId":"27902","inReplyTo":"1311469670-15392-3-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH 2/2] Ensure git ls-tree exits with a non-zero exit code if read_tree_recursive fails.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-07-24T07:45:35Z","receivedAt":"2011-07-24T07:45:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jon Seymour writes:\n\n> In the case of a corrupt repository, git ls-tree may report an error but\n> presently it exits with a code of 0.\n>\n> This change uses the return code of read_tree_recursive instead.\n>\n> Improved-by: Jens Lehmann \n> Signed-off-by: Jon Seymour \n> ---\n> builtin/ls-tree.c | 6 +++---\n> t/t3103-ls-tree-missing-tree.sh | 2 +-\n> 2 files changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> index f08c5b0..6d6c992 100644\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -120,7 +120,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n> {\n> \tunsigned char sha1[20];\n> \tstruct tree *tree;\n> -\tint i, full_tree = 0;\n> +\tint i, full_tree = 0, err;\n> \tconst struct option ls_tree_options[] = {\n> \t\tOPT_BIT('d', NULL, &ls_options, \"only show trees\",\n> \t\t\tLS_TREE_ONLY),\n> @@ -173,7 +173,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n> \ttree = parse_tree_indirect(sha1);\n> \tif (!tree)\n> \t\tdie(\"not a tree object\");\n> -\tread_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n> +\terr = read_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n> \n> -\treturn 0;\n> +\treturn err;\n> }\n\nMakes sense. Thanks.\n"},{"id":"171931","messageId":"CAH3AnrrH3YcvzxPo=gdXavUh-yMsXcZ6UND4j=uvvC7Cbnmqng@mail.gmail.com","threadId":"27902","inReplyTo":"7vipqsw29v.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] Add a test to check that git ls-tree sets non-zero exit code on error.","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2011-07-24T09:12:18Z","receivedAt":"2011-07-24T09:12:18Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Sun, Jul 24, 2011 at 5:45 PM, Junio C Hamano wrote:\n> Jon Seymour writes:\n>\n>>  t/t3103-ls-tree-missing-tree.sh |   19 +++++++++++++++++++\n>>  1 files changed, 19 insertions(+), 0 deletions(-)\n>>  create mode 100755 t/t3103-ls-tree-missing-tree.sh\n>\n> I'd rather not waste a new test number for something trivial like this.\n> Don't we already have a script that tests ls-tree?\n>\n\nHappy to take suggestions about which other test I should modify to\ninclude this one and how that should be renamed (if at all).\n\nThe other tests t3100 -> t3102 are all currently quite coherent and focused.\n\nPerhaps a rename &/or merge of existing tests is required?\n\nFYI: t31xx has quite a lot of head room (e.g. there are still 96 names\navailable in that space)\n\njon.\n"},{"id":"171945","messageId":"4E2C212E.8000404@web.de","threadId":"27902","inReplyTo":"1311469670-15392-3-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH 2/2] Ensure git ls-tree exits with a non-zero exit code if read_tree_recursive fails.","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-07-24T13:42:06Z","receivedAt":"2011-07-24T13:42:06Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.07.2011 03:07, schrieb Jon Seymour:\n> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> index f08c5b0..6d6c992 100644\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -120,7 +120,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n> {\n> \tunsigned char sha1[20];\n> \tstruct tree *tree;\n> -\tint i, full_tree = 0;\n> +\tint i, full_tree = 0, err;\n> \tconst struct option ls_tree_options[] = {\n> \t\tOPT_BIT('d', NULL, &ls_options, \"only show trees\",\n> \t\t\tLS_TREE_ONLY),\n> @@ -173,7 +173,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n> \ttree = parse_tree_indirect(sha1);\n> \tif (!tree)\n> \t\tdie(\"not a tree object\");\n> -\tread_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n> +\terr = read_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n> \n> -\treturn 0;\n> +\treturn err;\n> }\n\nNit: Is it really necessary to introduce a new variable \"err\" for that? Looks\nlike a \"return read_tree_recursive(...)\" would suffice here ...\n"}]}