{"thread":{"id":"27909","subject":"[PATCH v3 2/2] Ensure git ls-tree exits with a non-zero exit code if read_tree_recursive fails.","startedAt":"2011-07-24T14:59:12Z","lastAt":"2011-07-25T17:38:50Z","messageCount":9,"participants":["Jon Seymour","Nguyen Thai Ngoc Duy","Andreas Schwab","Jens Lehmann","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"171957","messageId":"1311519554-16587-1-git-send-email-jon.seymour@gmail.com","threadId":"27909","inReplyTo":null,"subject":"[PATCH v3 0/2] ls-tree: exit with non-zero status on error","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2011-07-24T14:59:12Z","receivedAt":"2011-07-24T14:59:12Z","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| v3: Generalize name of test script (in response to Junio's comment).\n| Removed unnecessary local variable, per Jens' comment.\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-misc.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":"171958","messageId":"1311519554-16587-2-git-send-email-jon.seymour@gmail.com","threadId":"27909","inReplyTo":"1311519554-16587-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v3 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-24T14:59:13Z","receivedAt":"2011-07-24T14:59:13Z","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\nAdditional tests of adhoc or uncategorised nature should be added to this\nfile.\n\nImproved-by: Jens Lehmann \nImproved-by: Junio C Hamano \nSigned-off-by: Jon Seymour \n---\n t/t3103-ls-tree-misc.sh | 24 ++++++++++++++++++++++++\n 1 files changed, 24 insertions(+), 0 deletions(-)\n create mode 100755 t/t3103-ls-tree-misc.sh\n\ndiff --git a/t/t3103-ls-tree-misc.sh b/t/t3103-ls-tree-misc.sh\nnew file mode 100755\nindex 0000000..c9c20f9\n--- /dev/null\n+++ b/t/t3103-ls-tree-misc.sh\n@@ -0,0 +1,24 @@\n+#!/bin/sh\n+\n+test_description='\n+Miscellaneous tests for git ls-tree.\n+\n+\t 1. git ls-tree fails in presence of tree damage.\n+\n+'\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.gf855\n"},{"id":"171956","messageId":"1311519554-16587-3-git-send-email-jon.seymour@gmail.com","threadId":"27909","inReplyTo":"1311519554-16587-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v3 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-24T14:59:14Z","receivedAt":"2011-07-24T14:59:14Z","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 | 3 +--\n t/t3103-ls-tree-misc.sh | 2 +-\n 2 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex f08c5b0..d96e9c4 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -173,7 +173,6 @@ 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+\treturn read_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n \n-\treturn 0;\n }\ndiff --git a/t/t3103-ls-tree-misc.sh b/t/t3103-ls-tree-misc.sh\nindex c9c20f9..09dcf04 100755\n--- a/t/t3103-ls-tree-misc.sh\n+++ b/t/t3103-ls-tree-misc.sh\n@@ -16,7 +16,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.gf855\n"},{"id":"171975","messageId":"CACsJy8Aa_=BvZOt58sM4dknVzvihH9OtHAS1aRxqPwNLxh0fkg@mail.gmail.com","threadId":"27909","inReplyTo":"1311519554-16587-3-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH v3 2/2] Ensure git ls-tree exits with a non-zero exit code if read_tree_recursive fails.","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-07-25T01:31:18Z","receivedAt":"2011-07-25T01:31:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jul 24, 2011 at 9:59 PM, Jon Seymour wrote:\n> @@ -173,7 +173,6 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n>        tree = parse_tree_indirect(sha1);\n>        if (!tree)\n>                die(\"not a tree object\");\n> -       read_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n> +       return read_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n>\n> -       return 0;\n\nNit picking. Most programs return positive value (usually 1) for error\ncases here. read_tree_recursive may return -1 (which turns out to be\n255). Also removing the last blank line in this function would be\nnice.\n-- \nDuy\n"},{"id":"171980","messageId":"CAH3Anrrbj93briHKpxEjpoVTOnSikQYQyxa4R=C7QKt=punyVw@mail.gmail.com","threadId":"27909","inReplyTo":"CACsJy8Aa_=BvZOt58sM4dknVzvihH9OtHAS1aRxqPwNLxh0fkg@mail.gmail.com","subject":"Re: [PATCH v3 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-25T08:45:35Z","receivedAt":"2011-07-25T08:45:35Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"Fair point.\n\nEveryone ok with this revision? If so, I will re-roll v4.\n\nJens: I added the variable back because the return statement with a\nternary operator needed a line break. Let me know\nif there is a better way to format it.\n\njon.\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex d96e9c4..990e1a3 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\nchar *prefix)\n {\n unsigned char sha1[20];\n struct tree *tree;\n- int i, full_tree = 0;\n+ int i, full_tree = 0, ret;\n const struct option ls_tree_options[] = {\n OPT_BIT('d', NULL, &ls_options, \"only show trees\",\n LS_TREE_ONLY),\n@@ -173,6 +173,7 @@ int cmd_ls_tree(int argc, const char **argv, const\nchar *prefix)\n tree = parse_tree_indirect(sha1);\n if (!tree)\n die(\"not a tree object\");\n- return read_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n+ ret = read_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n\n+ return ret ? 1 : 0;\n }\n\n\nOn Mon, Jul 25, 2011 at 11:31 AM, Nguyen Thai Ngoc Duy\n wrote:\n> On Sun, Jul 24, 2011 at 9:59 PM, Jon Seymour wrote:\n>> @@ -173,7 +173,6 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n>>        tree = parse_tree_indirect(sha1);\n>>        if (!tree)\n>>                die(\"not a tree object\");\n>> -       read_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n>> +       return read_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n>>\n>> -       return 0;\n>\n> Nit picking. Most programs return positive value (usually 1) for error\n> cases here. read_tree_recursive may return -1 (which turns out to be\n> 255). Also removing the last blank line in this function would be\n> nice.\n> --\n> Duy\n>\n"},{"id":"171987","messageId":"CACsJy8DSLK=-rrtvWQbcoS93mMysy=a0GVUCWh=6srYHZW1-mA@mail.gmail.com","threadId":"27909","inReplyTo":"CAH3Anrrbj93briHKpxEjpoVTOnSikQYQyxa4R=C7QKt=punyVw@mail.gmail.com","subject":"Re: [PATCH v3 2/2] Ensure git ls-tree exits with a non-zero exit code if read_tree_recursive fails.","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-07-25T12:58:26Z","receivedAt":"2011-07-25T12:58:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jul 25, 2011 at 3:45 PM, Jon Seymour wrote:\n> Fair point.\n>\n> Everyone ok with this revision? If so, I will re-roll v4.\n\nI'm good.\n-- \nDuy\n"},{"id":"171992","messageId":"m239hugwrx.fsf@igel.home","threadId":"27909","inReplyTo":"CAH3Anrrbj93briHKpxEjpoVTOnSikQYQyxa4R=C7QKt=punyVw@mail.gmail.com","subject":"Re: [PATCH v3 2/2] Ensure git ls-tree exits with a non-zero exit code if read_tree_recursive fails.","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2011-07-25T16:17:22Z","receivedAt":"2011-07-25T16:17:22Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Jon Seymour writes:\n\n> + return ret ? 1 : 0;\n\n return ret != 0;\n return !!ret;\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"171995","messageId":"4E2D9DE5.1050502@web.de","threadId":"27909","inReplyTo":"CAH3Anrrbj93briHKpxEjpoVTOnSikQYQyxa4R=C7QKt=punyVw@mail.gmail.com","subject":"Re: [PATCH v3 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-25T16:46:29Z","receivedAt":"2011-07-25T16:46:29Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 25.07.2011 10:45, schrieb Jon Seymour:\n> Fair point.\n> \n> Everyone ok with this revision? If so, I will re-roll v4.\n> \n> Jens: I added the variable back because the return statement with a\n> ternary operator needed a line break. Let me know\n> if there is a better way to format it.\n\nNow the extra variable makes sense to me, so I'm good too.\n"},{"id":"171997","messageId":"7vmxg2s1jp.fsf@alter.siamese.dyndns.org","threadId":"27909","inReplyTo":"CAH3Anrrbj93briHKpxEjpoVTOnSikQYQyxa4R=C7QKt=punyVw@mail.gmail.com","subject":"Re: [PATCH v3 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-25T17:38:50Z","receivedAt":"2011-07-25T17:38:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jon Seymour writes:\n\n> @@ -173,6 +173,7 @@ int cmd_ls_tree(int argc, const char **argv, const\n> char *prefix)\n> tree = parse_tree_indirect(sha1);\n> if (!tree)\n> die(\"not a tree object\");\n> - return read_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n> + ret = read_tree_recursive(tree, \"\", 0, 0, &pathspec, show_tree, NULL);\n>\n> + return ret ? 1 : 0;\n> }\n\nSurely, or \"return !!read_tree_recursive(...);\" is fine as well.\n"}]}