{"thread":{"id":"29645","subject":"[PATCH] Add an option to require a filter to be successful","startedAt":"2012-02-16T23:18:48Z","lastAt":"2012-02-18T07:27:49Z","messageCount":8,"participants":["Jehan Bing","Junio C Hamano","jehan@orb.com","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"184849","messageId":"1329434328-26621-1-git-send-email-jehan@orb.com","threadId":"29645","inReplyTo":null,"subject":"[PATCH] Add an option to require a filter to be successful","fromName":"Jehan Bing","fromEmail":"jehan@orb.com","sentAt":"2012-02-16T23:18:48Z","receivedAt":"2012-02-16T23:18:48Z","isPatch":true,"sender":{"key":"jehan@orb.com","avatar":null},"body":"By default, if a filter driver fails, the unfiltered content will be\nused. This patch adds a \"filter.<name>.required\" config option. When\nset to true, git will abort if the filter fails.\n\nA typical usage would be for a \"bigfile\" filter, where the smudge\ncommand can fail if the file is not available locally. Without the\n\"required\", the content of repository, i.e. a reference to the real\ncontent, will be checked out. Unless one saves the output logs, it\nthen fairly easy to lose track of what \"bigfile\" wasn't checked out\ncorrectly.\n\nAnother example would be for an encrypted repository if the clean\ncommand (encryption) fails. Without the \"required\", an unencrypted\ncontent could be stored in the repository by mistake.\n\nSigned-off-by: Jehan Bing <jehan@orb.com>\n---\n Documentation/gitattributes.txt |   14 ++++++++++++\n convert.c                       |   28 +++++++++++++++++++++---\n t/t0021-conversion.sh           |   43 +++++++++++++++++++++++++++++++++++++++\n 3 files changed, 81 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex a85b187..4d1af93 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -305,6 +305,10 @@ intent is that if someone unsets the filter driver definition,\n or does not have the appropriate filter program, the project\n should still be usable.\n \n+The exception is if the filter definition has the `required`\n+attribute set to `true`. In that case, the filter must apply\n+successfully or git will abort the current operation.\n+\n For example, in .gitattributes, you would assign the `filter`\n attribute for paths.\n \n@@ -335,6 +339,16 @@ input that is already correctly indented.  In this case, the lack of a\n smudge filter means that the clean filter _must_ accept its own output\n without modifying it.\n \n+If you do not wish git to continue if `clean` or `smudge` fail, you can\n+add a `required` attribute to the filter:\n+\n+------------------------\n+[filter \"crypt\"]\n+\tclean = openssl enc ...\n+\tsmudge = openssl enc -d ...\n+\trequired = true\n+------------------------\n+\n Sequence \"%f\" on the filter command line is replaced with the name of\n the file the filter is working on.  A filter might use this in keyword\n substitution.  For example:\ndiff --git a/convert.c b/convert.c\nindex 12868ed..6c95a90 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -429,6 +429,7 @@ static struct convert_driver {\n \tstruct convert_driver *next;\n \tconst char *smudge;\n \tconst char *clean;\n+\tint required;\n } *user_convert, **user_convert_tail;\n \n static int read_convert_config(const char *var, const char *value, void *cb)\n@@ -472,6 +473,11 @@ static int read_convert_config(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"clean\", ep))\n \t\treturn git_config_string(&drv->clean, var, value);\n \n+\tif (!strcmp(\"required\", ep)) {\n+\t\tdrv->required = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \treturn 0;\n }\n \n@@ -747,13 +753,19 @@ int convert_to_git(const char *path, const char *src, size_t len,\n {\n \tint ret = 0;\n \tconst char *filter = NULL;\n+\tint required = 0;\n \tstruct conv_attrs ca;\n \n \tconvert_attrs(&ca, path);\n-\tif (ca.drv)\n+\tif (ca.drv) {\n \t\tfilter = ca.drv->clean;\n+\t\trequired = ca.drv->required;\n+\t}\n \n \tret |= apply_filter(path, src, len, dst, filter);\n+\tif (!ret && required)\n+\t\tdie(\"required filter '%s' failed\", ca.drv->name);\n+\n \tif (ret) {\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n@@ -771,13 +783,16 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t\t\t\t    size_t len, struct strbuf *dst,\n \t\t\t\t\t    int normalizing)\n {\n-\tint ret = 0;\n+\tint ret = 0, ret_filter = 0;\n \tconst char *filter = NULL;\n+\tint required = 0;\n \tstruct conv_attrs ca;\n \n \tconvert_attrs(&ca, path);\n-\tif (ca.drv)\n+\tif (ca.drv) {\n \t\tfilter = ca.drv->smudge;\n+\t\trequired = ca.drv->required;\n+\t}\n \n \tret |= ident_to_worktree(path, src, len, dst, ca.ident);\n \tif (ret) {\n@@ -796,7 +811,12 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t\tlen = dst->len;\n \t\t}\n \t}\n-\treturn ret | apply_filter(path, src, len, dst, filter);\n+\n+\tret_filter = apply_filter(path, src, len, dst, filter);\n+\tif (!ret_filter && required)\n+\t\tdie(\"required filter %s failed\", ca.drv->name);\n+\n+\treturn ret | ret_filter;\n }\n \n int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst)\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex f19e651..f80a59f 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -153,4 +153,47 @@ test_expect_success 'filter shell-escaped filenames' '\n \t:\n '\n \n+test_expect_success 'required filter success' '\n+\tgit config filter.required.smudge cat &&\n+\tgit config filter.required.clean cat &&\n+\tgit config filter.required.required true &&\n+\n+\t{\n+\t    echo \"*.r filter=required\"\n+\t} >.gitattributes &&\n+\n+\techo test > test.r &&\n+\tgit add test.r &&\n+\trm -f test.r &&\n+\tgit checkout -- test.r\n+'\n+\n+test_expect_success 'required filter smudge failure' '\n+\tgit config filter.failsmudge.smudge false &&\n+\tgit config filter.failsmudge.clean cat &&\n+\tgit config filter.failsmudge.required true &&\n+\n+\t{\n+\t    echo \"*.fs filter=failsmudge\"\n+\t} >.gitattributes &&\n+\n+\techo test > test.fs &&\n+\tgit add test.fs &&\n+\trm -f test.fs &&\n+\t! git checkout -- test.fs\n+'\n+\n+test_expect_success 'required filter clean failure' '\n+\tgit config filter.failclean.smudge cat &&\n+\tgit config filter.failclean.clean false &&\n+\tgit config filter.failclean.required true &&\n+\n+\t{\n+\t    echo \"*.fc filter=failclean\"\n+\t} >.gitattributes &&\n+\n+\techo test > test.fc &&\n+\t! git add test.fc\n+'\n+\n test_done\n-- \n1.7.9\n"},{"id":"184854","messageId":"7vobsywck1.fsf@alter.siamese.dyndns.org","threadId":"29645","inReplyTo":"1329434328-26621-1-git-send-email-jehan@orb.com","subject":"Re: [PATCH] Add an option to require a filter to be successful","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-17T00:03:58Z","receivedAt":"2012-02-17T00:03:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jehan Bing <jehan@orb.com> writes:\n\n> By default, if a filter driver fails, the unfiltered content will be\n> used. This patch adds a \"filter.<name>.required\" config option. When\n> set to true, git will abort if the filter fails.\n>\n> A typical usage would be for a \"bigfile\" filter, where the smudge\n> command can fail if the file is not available locally. Without the\n> \"required\", the content of repository, i.e. a reference to the real\n> content, will be checked out. Unless one saves the output logs, it\n> then fairly easy to lose track of what \"bigfile\" wasn't checked out\n> correctly.\n>\n> Another example would be for an encrypted repository if the clean\n> command (encryption) fails. Without the \"required\", an unencrypted\n> content could be stored in the repository by mistake.\n\nThe above describes sample situations where setting the \"required\" may be\nvery useful, without saying anything about in what situation it might be\nuseful to set it to \"optional\".\n\nWhich makes the reader wonder why this is not done as a bugfix patch that\nunconditionally propagates the failure from the filter up the callchain.\n\nThat is because the first sentence in the message is too weak. It needs to\nbe followed by something like:\n\n    This is because the content filtering is done to massage the content\n    into a shape that is more convenient for the platform, filesystem, and\n    the user to use.  The key phrase here is \"more convenient\" and not\n    \"turning something unusable into usable\".\n\nwhich is what the part of gitattributes documentation shown in the context\nsays.\n\nThat is a statement of principle.  And according to that principle, your\nconfiguration option should never exist.\n\nIf we are changing that principle and making it configurable, I think the\nupdate to the existing documentation should state things a bit stronger.\nWe shouldn't be saying \"Do not use filter to turn unusable contents to\nusable\" and in the next breath \"But you can use it if you set this at the\nsame time\".  That is simply too confusing.\n\nHere is an attempt to rephrase the part that updates the documentation.\n\nNote that filter.<driver>.required is *NOT* an attribute.  An attribute is\nsomething you attach to paths.\n\n\n Documentation/gitattributes.txt |   35 ++++++++++++++++++++++++++---------\n 1 file changed, 26 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 25e46ae..39a2654 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -294,16 +294,23 @@ output is used to update the worktree file.  Similarly, the\n `clean` command is used to convert the contents of worktree file\n upon checkin.\n \n-A missing filter driver definition in the config is not an error\n-but makes the filter a no-op passthru.\n+One use of the content filtering is to massage the content into a shape\n+that is more convenient for the platform, filesystem, and the user to use.\n \n-The content filtering is done to massage the content into a\n-shape that is more convenient for the platform, filesystem, and\n-the user to use.  The key phrase here is \"more convenient\" and not\n-\"turning something unusable into usable\".  In other words, the\n-intent is that if someone unsets the filter driver definition,\n-or does not have the appropriate filter program, the project\n-should still be usable.\n+Another use of the content filtering is to store the content that cannot\n+be directly used in the repository (e.g. an UUID that refers to the true\n+content stored outside git, or an encrypted content) and turn it into a\n+usable form upon checkout (e.g. download the external content, decrypt the\n+encrypted content).\n+\n+These two filters behave differently, and by default, a filter is taken as\n+the former, massaging the contents into more convenient shape.  A missing\n+filter driver definition in the config, or a filter driver that exits with\n+a non-zero status, is not an error but makes the filter a no-op passthru.\n+\n+You can declare that a filter turns a content that by itself is unusable\n+into usable by setting filter.<drivername>.required configuration variable\n+to `true`.\n \n For example, in .gitattributes, you would assign the `filter`\n attribute for paths.\n@@ -335,6 +342,16 @@ input that is already correctly indented.  In this case, the lack of a\n smudge filter means that the clean filter _must_ accept its own output\n without modifying it.\n \n+If a filter _must_ succeed in order to make the stored contents usable,\n+you can declare that the filter is `required`, in the configuration:\n+\n+------------------------\n+[filter \"crypt\"]\n+\tclean = openssl enc ...\n+\tsmudge = openssl enc -d ...\n+\trequired\n+------------------------\n+\n Sequence \"%f\" on the filter command line is replaced with the name of\n the file the filter is working on.  A filter might use this in keyword\n substitution.  For example:\n"},{"id":"184857","messageId":"4f3daaf7.e302440a.02ba.fffff463@mx.google.com","threadId":"29645","inReplyTo":"7vobsywck1.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] Add a setting to require a filter to be successful","fromName":"","fromEmail":"jehan@orb.com","sentAt":"2012-02-17T01:19:03Z","receivedAt":"2012-02-17T01:19:03Z","isPatch":true,"sender":{"key":"jehan@orb.com","avatar":null},"body":"From: Jehan Bing <jehan@orb.com>\n\nBy default, if a filter driver fails, the unfiltered content will be\nused. This is because the content filtering is done to massage the\ncontent into a shape that is more convenient for the platform,\nfilesystem, and the user to use. The key phrase here is \"more\nconvenient\" and not \"turning something unusable into usable\".\n\nHowever, another use of the content filtering is to store the content\nthat cannot be directly used in the repository (e.g. an UUID that\nrefers to the true content stored outside git, or an encrypted\ncontent) and turn it into a usable form upon checkout (e.g. download\nthe external content, decrypt the encrypted content).\nIn this situation, it is preferable to have git fail instead of using\nthe unfiltered content.\n\nThis patch adds an optional \"filter.<filtername>.required\"\nconfiguration variable. When missing or set to false, git will use\nthe unfiltered content if the filter driver fails (old behavior).\nWhen set to true, git will instead abort the current operation.\n\nSigned-off-by: Jehan Bing <jehan@orb.com>\n---\nThanks Junio for your comment. This version use your version of\ngitattributes.txt and I rewrote the commit message to be\nstronger.\n\n-Jehan\n\n Documentation/gitattributes.txt |   35 +++++++++++++++++++++++--------\n convert.c                       |   28 +++++++++++++++++++++---\n t/t0021-conversion.sh           |   43 +++++++++++++++++++++++++++++++++++++++\n 3 files changed, 93 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex a85b187..6abaf9a 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -294,16 +294,23 @@ output is used to update the worktree file.  Similarly, the\n `clean` command is used to convert the contents of worktree file\n upon checkin.\n \n-A missing filter driver definition in the config is not an error\n-but makes the filter a no-op passthru.\n+One use of the content filtering is to massage the content into a shape\n+that is more convenient for the platform, filesystem, and the user to use.\n \n-The content filtering is done to massage the content into a\n-shape that is more convenient for the platform, filesystem, and\n-the user to use.  The key phrase here is \"more convenient\" and not\n-\"turning something unusable into usable\".  In other words, the\n-intent is that if someone unsets the filter driver definition,\n-or does not have the appropriate filter program, the project\n-should still be usable.\n+Another use of the content filtering is to store the content that cannot\n+be directly used in the repository (e.g. an UUID that refers to the true\n+content stored outside git, or an encrypted content) and turn it into a\n+usable form upon checkout (e.g. download the external content, decrypt the\n+encrypted content).\n+\n+These two filters behave differently, and by default, a filter is taken as\n+the former, massaging the contents into more convenient shape.  A missing\n+filter driver definition in the config, or a filter driver that exits with\n+a non-zero status, is not an error but makes the filter a no-op passthru.\n+\n+You can declare that a filter turns a content that by itself is unusable\n+into usable by setting filter.<drivername>.required configuration variable\n+to `true`.\n \n For example, in .gitattributes, you would assign the `filter`\n attribute for paths.\n@@ -335,6 +342,16 @@ input that is already correctly indented.  In this case, the lack of a\n smudge filter means that the clean filter _must_ accept its own output\n without modifying it.\n \n+If a filter _must_ succeed in order to make the stored contents usable,\n+you can declare that the filter is `required`, in the configuration:\n+\n+------------------------\n+[filter \"crypt\"]\n+\tclean = openssl enc ...\n+\tsmudge = openssl enc -d ...\n+\trequired\n+------------------------\n+\n Sequence \"%f\" on the filter command line is replaced with the name of\n the file the filter is working on.  A filter might use this in keyword\n substitution.  For example:\ndiff --git a/convert.c b/convert.c\nindex 12868ed..6c95a90 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -429,6 +429,7 @@ static struct convert_driver {\n \tstruct convert_driver *next;\n \tconst char *smudge;\n \tconst char *clean;\n+\tint required;\n } *user_convert, **user_convert_tail;\n \n static int read_convert_config(const char *var, const char *value, void *cb)\n@@ -472,6 +473,11 @@ static int read_convert_config(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"clean\", ep))\n \t\treturn git_config_string(&drv->clean, var, value);\n \n+\tif (!strcmp(\"required\", ep)) {\n+\t\tdrv->required = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \treturn 0;\n }\n \n@@ -747,13 +753,19 @@ int convert_to_git(const char *path, const char *src, size_t len,\n {\n \tint ret = 0;\n \tconst char *filter = NULL;\n+\tint required = 0;\n \tstruct conv_attrs ca;\n \n \tconvert_attrs(&ca, path);\n-\tif (ca.drv)\n+\tif (ca.drv) {\n \t\tfilter = ca.drv->clean;\n+\t\trequired = ca.drv->required;\n+\t}\n \n \tret |= apply_filter(path, src, len, dst, filter);\n+\tif (!ret && required)\n+\t\tdie(\"required filter '%s' failed\", ca.drv->name);\n+\n \tif (ret) {\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n@@ -771,13 +783,16 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t\t\t\t    size_t len, struct strbuf *dst,\n \t\t\t\t\t    int normalizing)\n {\n-\tint ret = 0;\n+\tint ret = 0, ret_filter = 0;\n \tconst char *filter = NULL;\n+\tint required = 0;\n \tstruct conv_attrs ca;\n \n \tconvert_attrs(&ca, path);\n-\tif (ca.drv)\n+\tif (ca.drv) {\n \t\tfilter = ca.drv->smudge;\n+\t\trequired = ca.drv->required;\n+\t}\n \n \tret |= ident_to_worktree(path, src, len, dst, ca.ident);\n \tif (ret) {\n@@ -796,7 +811,12 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t\tlen = dst->len;\n \t\t}\n \t}\n-\treturn ret | apply_filter(path, src, len, dst, filter);\n+\n+\tret_filter = apply_filter(path, src, len, dst, filter);\n+\tif (!ret_filter && required)\n+\t\tdie(\"required filter %s failed\", ca.drv->name);\n+\n+\treturn ret | ret_filter;\n }\n \n int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst)\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex f19e651..f80a59f 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -153,4 +153,47 @@ test_expect_success 'filter shell-escaped filenames' '\n \t:\n '\n \n+test_expect_success 'required filter success' '\n+\tgit config filter.required.smudge cat &&\n+\tgit config filter.required.clean cat &&\n+\tgit config filter.required.required true &&\n+\n+\t{\n+\t    echo \"*.r filter=required\"\n+\t} >.gitattributes &&\n+\n+\techo test > test.r &&\n+\tgit add test.r &&\n+\trm -f test.r &&\n+\tgit checkout -- test.r\n+'\n+\n+test_expect_success 'required filter smudge failure' '\n+\tgit config filter.failsmudge.smudge false &&\n+\tgit config filter.failsmudge.clean cat &&\n+\tgit config filter.failsmudge.required true &&\n+\n+\t{\n+\t    echo \"*.fs filter=failsmudge\"\n+\t} >.gitattributes &&\n+\n+\techo test > test.fs &&\n+\tgit add test.fs &&\n+\trm -f test.fs &&\n+\t! git checkout -- test.fs\n+'\n+\n+test_expect_success 'required filter clean failure' '\n+\tgit config filter.failclean.smudge cat &&\n+\tgit config filter.failclean.clean false &&\n+\tgit config filter.failclean.required true &&\n+\n+\t{\n+\t    echo \"*.fc filter=failclean\"\n+\t} >.gitattributes &&\n+\n+\techo test > test.fc &&\n+\t! git add test.fc\n+'\n+\n test_done\n-- \n1.7.9\n"},{"id":"184864","messageId":"4F3DFCD0.6070002@viscovery.net","threadId":"29645","inReplyTo":"4f3daaf7.e302440a.02ba.fffff463@mx.google.com","subject":"Re: [PATCH v2] Add a setting to require a filter to be successful","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-02-17T07:08:00Z","receivedAt":"2012-02-17T07:08:00Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 2/17/2012 2:19, schrieb jehan@orb.com:\n> @@ -747,13 +753,19 @@ int convert_to_git(const char *path, const char *src, size_t len,\n...\n>  \tret |= apply_filter(path, src, len, dst, filter);\n> +\tif (!ret && required)\n> +\t\tdie(\"required filter '%s' failed\", ca.drv->name);\n\nWouldn't it be much more helpful if this were:\n\n\tdie(\"%s: clean filter '%s' failed\", path, ca.drv->name);\n\nLikewise (with s/clean/smudge/) in convert_to_working_tree_internal().\n\n> +\t! git checkout -- test.fs\n\n\ttest_must_fail git checkout -- test.fs\n\n> +\t! git add test.fc\n\n\ttest_must_fail git add test.fc\n\n-- Hannes\n"},{"id":"184887","messageId":"7vd39dv5g5.fsf@alter.siamese.dyndns.org","threadId":"29645","inReplyTo":"4F3DFCD0.6070002@viscovery.net","subject":"Re: [PATCH v2] Add a setting to require a filter to be successful","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-17T15:35:06Z","receivedAt":"2012-02-17T15:35:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Am 2/17/2012 2:19, schrieb jehan@orb.com:\n>> @@ -747,13 +753,19 @@ int convert_to_git(const char *path, const char *src, size_t len,\n> ...\n>>  \tret |= apply_filter(path, src, len, dst, filter);\n>> +\tif (!ret && required)\n>> +\t\tdie(\"required filter '%s' failed\", ca.drv->name);\n>\n> Wouldn't it be much more helpful if this were:\n>\n> \tdie(\"%s: clean filter '%s' failed\", path, ca.drv->name);\n>\n> Likewise (with s/clean/smudge/) in convert_to_working_tree_internal().\n>\n>> +\t! git checkout -- test.fs\n>\n> \ttest_must_fail git checkout -- test.fs\n>\n>> +\t! git add test.fc\n>\n> \ttest_must_fail git add test.fc\n>\n> -- Hannes\n\nThanks; I'll just squash these in in-place.\n"},{"id":"184913","messageId":"7vd39dqa1i.fsf@alter.siamese.dyndns.org","threadId":"29645","inReplyTo":"4F3DFCD0.6070002@viscovery.net","subject":"Re: [PATCH v2] Add a setting to require a filter to be successful","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-18T00:07:05Z","receivedAt":"2012-02-18T00:07:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A few test in t0021 use 'false' as the filter, which can exit without\nreading any byte from us, before we start writing and causes us to die\nwith SIGPIPE, leading to intermittent test failure.  I think treating this\nas a failure of running the filter (the end user's filter should read what\nis fed in full, produce its output and write the result back to us) is the\nright thing to do, and this patch needs more work to handle such a\nsituation better, probably by using sigchain_push(SIGPIPE) or something.\n"},{"id":"184915","messageId":"4F3EF43D.2040102@orb.com","threadId":"29645","inReplyTo":"7vd39dqa1i.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] Add a setting to require a filter to be successful","fromName":"Jehan Bing","fromEmail":"jehan@orb.com","sentAt":"2012-02-18T00:43:41Z","receivedAt":"2012-02-18T00:43:41Z","isPatch":true,"sender":{"key":"jehan@orb.com","avatar":null},"body":"On 2012-02-17 16:07, Junio C Hamano wrote:\n> A few test in t0021 use 'false' as the filter, which can exit without\n> reading any byte from us, before we start writing and causes us to die\n> with SIGPIPE, leading to intermittent test failure.  I think treating this\n> as a failure of running the filter (the end user's filter should read what\n> is fed in full, produce its output and write the result back to us) is the\n> right thing to do, and this patch needs more work to handle such a\n> situation better, probably by using sigchain_push(SIGPIPE) or something.\n\nIf I understand what you're saying, current version of git already have \nthe problem: if a filter fails without reading anything, git will die \ninstead of using the unfiltered content. My patch has only made the \nissue apparent by testing with a failing filter.\nAm I understanding correctly?\n"},{"id":"184921","messageId":"7v4nuor47e.fsf@alter.siamese.dyndns.org","threadId":"29645","inReplyTo":"4F3EF43D.2040102@orb.com","subject":"Re: [PATCH v2] Add a setting to require a filter to be successful","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-18T07:27:49Z","receivedAt":"2012-02-18T07:27:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jehan Bing <jehan@orb.com> writes:\n\n> If I understand what you're saying, current version of git already\n> have the problem: if a filter fails without reading anything, git will\n> die instead of using the unfiltered content. My patch has only made\n> the issue apparent by testing with a failing filter.\n> Am I understanding correctly?\n\nYes.\n"}]}