{"thread":{"id":"30900","subject":"[PATCH/RFC] add: listen to --ignore-errors for submodule-errors","startedAt":"2012-06-25T23:21:59Z","lastAt":"2012-06-26T08:25:23Z","messageCount":4,"participants":["Erik Faye-Lund","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"194238","messageId":"1340666519-41804-1-git-send-email-kusmabite@gmail.com","threadId":"30900","inReplyTo":null,"subject":"[PATCH/RFC] add: listen to --ignore-errors for submodule-errors","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-06-25T23:21:59Z","receivedAt":"2012-06-25T23:21:59Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"\"git add --ignore-errors -- some-submodule/foo\" surprisingly\nthrows an error saying \"fatal: Path 'some-submodule/foo' is in\nsubmodule 'some-submodule/foo'\".\n\nFix this by making sure we consult the flag, and propagate the\nerror code properly.\n\nSigned-off-by: Erik Faye-Lund \n---\n\nI recently tried to do the following in the msysGit-repo:\n\n $ git add --ignore-errors -- */.gitignore\n fatal: Path 'src/git-cheetah/.gitignore' is in submodule 'src/git-cheetah'\n\nI was a bit puzzled by this; I explicitly specified --ignore-errors\nbecause I did not want to be stopped due to src/git-cheetah/.gitignore\nbeing located in a submodule.\n\nThe documentation seems to suggest that this is what is supposed to\nhappen, and this seems like the most likely behavior that the user\nwanted. After all, there's no good reason submodules are special\nin this regard, no?\n\n builtin/add.c | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 87446cf..6e6feb0 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -20,7 +20,7 @@ static const char * const builtin_add_usage[] = {\n \tNULL\n };\n static int patch_interactive, add_interactive, edit_interactive;\n-static int take_worktree_changes;\n+static int take_worktree_changes, ignore_add_errors;\n \n struct update_callback_data {\n \tint flags;\n@@ -153,9 +153,9 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int\n \treturn seen;\n }\n \n-static void treat_gitlinks(const char **pathspec)\n+static int treat_gitlinks(const char **pathspec)\n {\n-\tint i;\n+\tint i, exit_status = 0;\n \n \tif (!pathspec || !*pathspec)\n \t\treturn;\n@@ -172,12 +172,15 @@ static void treat_gitlinks(const char **pathspec)\n \t\t\t\tif (len2 == len + 1)\n \t\t\t\t\t/* strip trailing slash */\n \t\t\t\t\tpathspec[j] = xstrndup(ce->name, len);\n-\t\t\t\telse\n+\t\t\t\telse if (!ignore_add_errors)\n \t\t\t\t\tdie (_(\"Path '%s' is in submodule '%.*s'\"),\n \t\t\t\t\t\tpathspec[j], len, ce->name);\n+\t\t\t\telse\n+\t\t\t\t\texit_status = 1;\n \t\t\t}\n \t\t}\n \t}\n+\treturn exit_status;\n }\n \n static void refresh(int verbose, const char **pathspec)\n@@ -312,7 +315,7 @@ static const char ignore_error[] =\n N_(\"The following paths are ignored by one of your .gitignore files:\\n\");\n \n static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;\n-static int ignore_add_errors, addremove, intent_to_add, ignore_missing = 0;\n+static int addremove, intent_to_add, ignore_missing = 0;\n \n static struct option builtin_add_options[] = {\n \tOPT__DRY_RUN(&show_only, \"dry run\"),\n@@ -418,7 +421,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tif (read_cache() < 0)\n \t\tdie(_(\"index file corrupt\"));\n-\ttreat_gitlinks(pathspec);\n+\texit_status |= treat_gitlinks(pathspec);\n \n \tif (add_new_files) {\n \t\tint baselen;\n-- \n1.7.11.msysgit.0.3.g3006a55\n"},{"id":"194250","messageId":"7vfw9ieqvh.fsf@alter.siamese.dyndns.org","threadId":"30900","inReplyTo":"1340666519-41804-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH/RFC] add: listen to --ignore-errors for submodule-errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-26T02:41:54Z","receivedAt":"2012-06-26T02:41:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund writes:\n\n> I recently tried to do the following in the msysGit-repo:\n>\n> $ git add --ignore-errors -- */.gitignore\n> fatal: Path 'src/git-cheetah/.gitignore' is in submodule 'src/git-cheetah'\n>\n> I was a bit puzzled by this; I explicitly specified --ignore-errors\n> because I did not want to be stopped due to src/git-cheetah/.gitignore\n> being located in a submodule.\n\nIf I recall correctly, originally --ignore-errors was added was by\nthose who (arguably misguidedly) wanted to randomly run \"git add\"\nthat can potentially race with ongoing working tree updates\n(i.e. think of a poor-man's unreliable snapshotting filesystem), to\nwhich \"git add\" will notice that the working tree file it was asked\nto index changed while it was reading and error out. Also on some\nsystems, \"git add\" on files that are currently open may not be able\nto read from them, which would also cause a run-time error. The\nkind of errors the option was meant to ignore were \"these paths are\nperfectly OK to add, but for some reason, adding them fails at this\nmoment, and for the purpose of poor-man's unreliable snapshot, it is\nOK not to pick the exact current state up, as we will pick it up the\nnext round\", not your kind of request that will lead to an error of\nthe \"adding this path will break the structural integrity of the\nrepository and git should error out\" kind.\n\n> The documentation seems to suggest that this is what is supposed to\n> happen, and this seems like the most likely behavior that the user\n> wanted. After all, there's no good reason submodules are special\n> in this regard, no?\n\nHow does \"git add .git\" or \"git add .git/config\" behave with your\npatch applied? It is exactly the same kind of error that breaks the\nstructural integrity of the repository as adding src/cheetah/.gitignore\nto the top-level project repository, and there is no good reason to\nspecial case submodules, either.\n"},{"id":"194258","messageId":"7v395iemt2.fsf@alter.siamese.dyndns.org","threadId":"30900","inReplyTo":"7vfw9ieqvh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] add: listen to --ignore-errors for submodule-errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-26T04:09:45Z","receivedAt":"2012-06-26T04:09:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Erik Faye-Lund writes:\n>\n>> I recently tried to do the following in the msysGit-repo:\n>>\n>> $ git add --ignore-errors -- */.gitignore\n>> fatal: Path 'src/git-cheetah/.gitignore' is in submodule 'src/git-cheetah'\n>>\n>> I was a bit puzzled by this; I explicitly specified --ignore-errors\n>> because I did not want to be stopped due to src/git-cheetah/.gitignore\n>> being located in a submodule.\n>\n> If I recall correctly, originally --ignore-errors was added was by\n> those who (arguably misguidedly) wanted to randomly run \"git add\"\n> that can potentially race with ongoing working tree updates\n> (i.e. think of a poor-man's unreliable snapshotting filesystem), to\n> which \"git add\" will notice that the working tree file it was asked\n> to index changed while it was reading and error out. Also on some\n> systems, \"git add\" on files that are currently open may not be able\n> to read from them, which would also cause a run-time error. The\n> kind of errors the option was meant to ignore were \"these paths are\n> perfectly OK to add, but for some reason, adding them fails at this\n> moment, and for the purpose of poor-man's unreliable snapshot, it is\n> OK not to pick the exact current state up, as we will pick it up the\n> next round\", not your kind of request that will lead to an error of\n> the \"adding this path will break the structural integrity of the\n> repository and git should error out\" kind.\n>\n>> The documentation seems to suggest that this is what is supposed to\n>> happen, and this seems like the most likely behavior that the user\n>> wanted. After all, there's no good reason submodules are special\n>> in this regard, no?\n>\n> How does \"git add .git\" or \"git add .git/config\" behave with your\n> patch applied? It is exactly the same kind of error that breaks the\n> structural integrity of the repository as adding src/cheetah/.gitignore\n> to the top-level project repository, and there is no good reason to\n> special case submodules, either.\n\nFor that matter, running \"git add ../../foo\" from the top-level of\nthe working tree falls into the same category. There are boundaries\nin both upward and downward directions that define the area you\ncould add to the index.\n\nNow, I am not saying that \"--ignore-errors\" should _never_ mean to\nignore errors of this kind. I was merely giving the historical\nbackground for the semantics you are observing. I personally think\nit might even be an improvement if you made the option to instruct\nthe command to consistently ignore requests to add these paths\noutside the working tree boundary in any direction (not just the\ndownwards boundary defined by the presense of a submodule, but the\ndownwards boundary defined by the \".git\" directory, and the upward\nboundary at the root of the working tree), and add only remaining\nvalid paths to the index. I do not think it is the right thing to\nspecial case only the submodules, though.\n"},{"id":"194268","messageId":"CABPQNSZmQOiwOo_dV43BidT3TRp63mOJeJWe4pK-9z0ryB6_QQ@mail.gmail.com","threadId":"30900","inReplyTo":"7vfw9ieqvh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] add: listen to --ignore-errors for submodule-errors","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-06-26T08:25:23Z","receivedAt":"2012-06-26T08:25:23Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Jun 26, 2012 at 4:41 AM, Junio C Hamano wrote:\n> Erik Faye-Lund writes:\n>\n>> I recently tried to do the following in the msysGit-repo:\n>>\n>>  $ git add --ignore-errors -- */.gitignore\n>>  fatal: Path 'src/git-cheetah/.gitignore' is in submodule 'src/git-cheetah'\n>>\n>> I was a bit puzzled by this; I explicitly specified --ignore-errors\n>> because I did not want to be stopped due to src/git-cheetah/.gitignore\n>> being located in a submodule.\n>\n> If I recall correctly, originally --ignore-errors was added was by\n> those who (arguably misguidedly) wanted to randomly run \"git add\"\n> that can potentially race with ongoing working tree updates\n> (i.e. think of a poor-man's unreliable snapshotting filesystem), to\n> which \"git add\" will notice that the working tree file it was asked\n> to index changed while it was reading and error out.  Also on some\n> systems, \"git add\" on files that are currently open may not be able\n> to read from them, which would also cause a run-time error.  The\n> kind of errors the option was meant to ignore were \"these paths are\n> perfectly OK to add, but for some reason, adding them fails at this\n> moment, and for the purpose of poor-man's unreliable snapshot, it is\n> OK not to pick the exact current state up, as we will pick it up the\n> next round\", not your kind of request that will lead to an error of\n> the \"adding this path will break the structural integrity of the\n> repository and git should error out\" kind.\n>\n\nRight. I guess my confusion was due to the documentation wording \"If\nsome files could not be added *because of errors indexing them*, do\nnot abort the operation\". It's not obvious to me that \"because if\nerrors indexing them\" means what you describe above.\n\n>> The documentation seems to suggest that this is what is supposed to\n>> happen, and this seems like the most likely behavior that the user\n>> wanted. After all, there's no good reason submodules are special\n>> in this regard, no?\n>\n> How does \"git add .git\" or \"git add .git/config\" behave with your\n> patch applied?  It is exactly the same kind of error that breaks the\n> structural integrity of the repository as adding src/cheetah/.gitignore\n> to the top-level project repository, and there is no good reason to\n> special case submodules, either.\n\nBoth \"git add .git\" and \"git add --ignore-errors .git\" returned\nsilently without error even before my patch.\n\n\"git add .git/config\" outputs\n\"error: Invalid path '.git/config'\nerror: unable to add .git/config to index\nfatal: adding files failed\"\n\n...and \"git add --ignore-errors .git/config\" outputs\n\"error: Invalid path '.git/config'\nerror: unable to add .git/config to index\"\n\n...this is both before and after my patch. I didn't touch that part of\nthe code-path, so this already treats these errors as non-fatal when\n--ignore-errors are specified.\n\nI simply found a new code-path where errors weren't ignored, but there\nmight of course be more. However, the lack of a \"fatal: adding files\nfailed\" for files inside .git seems to suggest to me that even though\nthe intent might be what you describe above, that is not actually what\nthe code does.\n\nOf course, silently ignoring \"git add .git\" seems odd to me.\n"}]}