{"thread":{"id":"23501","subject":"[BUG] Git add <device file> silently fails","startedAt":"2010-04-17T14:24:17Z","lastAt":"2010-04-19T05:15:31Z","messageCount":9,"participants":["Andreas Gruenbacher","Alex Riesen","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"139749","messageId":"201004171624.17797.agruen@suse.de","threadId":"23501","inReplyTo":null,"subject":"[BUG] Git add <device file> silently fails","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-04-17T14:24:17Z","receivedAt":"2010-04-17T14:24:17Z","isPatch":false,"sender":{"key":"agruen@suse.de","avatar":null},"body":"Hello,\n\nthere is code in read-cache.c:add_to_index() which checks if a file to be \nadded is a regular file, directory, or symlink; this function otherwise \nerror()s out.  It looks as if add_to_index() is supposed to be called via:\n\n  builtin/add.c:update_callback() ->\n    read-cache.c:add_file_to_index() ->\n      read-cache.c:add_to_index()\n\nHowever, when trying to add a device special file, update_callback() ends up \nnever getting called, no error message is produced, and git add silently \nfails:\n\n\t$ sudo mknod foobar c 1 3\n\t$ git add foobar\n   $ echo $?\n\t0\n\nMaybe someone familiar with run_diff_files() can have a look?\n\nThanks,\nAndreas\n"},{"id":"139750","messageId":"u2s81b0412b1004170744u4cc3c0e1z6d7019fe405a67ec@mail.gmail.com","threadId":"23501","inReplyTo":"201004171624.17797.agruen@suse.de","subject":"Re: [BUG] Git add <device file> silently fails","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2010-04-17T14:44:05Z","receivedAt":"2010-04-17T14:44:05Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Sat, Apr 17, 2010 at 16:24, Andreas Gruenbacher <agruen@suse.de> wrote:\n> However, when trying to add a device special file, update_callback() ends up\n> never getting called, no error message is produced, and git add silently\n> fails:\n>\n>        $ sudo mknod foobar c 1 3\n>        $ git add foobar\n>   $ echo $?\n>        0\n\nI think something like this should make the accident more\nnoticable:\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 87d2980..9c4a5f2 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -347,6 +347,8 @@ static int add_files(struct dir_struct *dir, int flags)\n \t\tdie(\"no files added\");\n \t}\n\n+\tif (!dir->nr)\n+\t\tdie(\"No files selected for addition\");\n \tfor (i = 0; i < dir->nr; i++)\n \t\tif (add_file_to_cache(dir->entries[i]->name, flags)) {\n \t\t\tif (!ignore_add_errors)\n"},{"id":"139751","messageId":"201004171700.22851.agruen@suse.de","threadId":"23501","inReplyTo":"u2s81b0412b1004170744u4cc3c0e1z6d7019fe405a67ec@mail.gmail.com","subject":"Re: [BUG] Git add <device file> silently fails","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-04-17T15:00:22Z","receivedAt":"2010-04-17T15:00:22Z","isPatch":false,"sender":{"key":"agruen@suse.de","avatar":null},"body":"On Saturday 17 April 2010 16:44:05 Alex Riesen wrote:\n> I think something like this should make the accident more\n> noticable:\n\nDoesn't actually tell what the problem might be, though.\n\nThanks,\nAndreas\n"},{"id":"139752","messageId":"g2h81b0412b1004170827y883c0cc8ofc72712c20e95dc8@mail.gmail.com","threadId":"23501","inReplyTo":"201004171700.22851.agruen@suse.de","subject":"Re: [BUG] Git add <device file> silently fails","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2010-04-17T15:27:44Z","receivedAt":"2010-04-17T15:27:44Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Sat, Apr 17, 2010 at 17:00, Andreas Gruenbacher <agruen@suse.de> wrote:\n> On Saturday 17 April 2010 16:44:05 Alex Riesen wrote:\n>> I think something like this should make the accident more\n>> noticable:\n>\n> Doesn't actually tell what the problem might be, though.\n>\n\nOh, sorry. Somehow I thought, you did found what the problem is\n(Git is not an archiving tool and does not store irregular files), and\njust wanted to point at \"git add\" failing silently adding such a name.\n\nIn this particular case, the name you given to Git, is a device\nentry point, which is filtered. Far too early, in my opinion, so\nno sensible diagnostics can be produced. That's why I suggested\nan exit with generic error message. I hope it is enough to at least\npoint the user to where a problem (like a fifo, device or socket\ngive in the command line) might be.\n"},{"id":"139755","messageId":"7v4oja3uh7.fsf@alter.siamese.dyndns.org","threadId":"23501","inReplyTo":"u2s81b0412b1004170744u4cc3c0e1z6d7019fe405a67ec@mail.gmail.com","subject":"Re: [BUG] Git add <device file> silently fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-17T16:38:12Z","receivedAt":"2010-04-17T16:38:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> I think something like this should make the accident more\n> noticable:\n\nThe early skippage done in dir.c (read-directory-recursive) should treat\nthese as ignored just like paths that are ignored with .gitignore\nmechanism, and if we do so, we shouldn't need this patch to add another\ncodepath to give notification to the user (we would however still need\nto reword \"'add -f' if you really want to add it\", though).\n\n> diff --git a/builtin/add.c b/builtin/add.c\n> index 87d2980..9c4a5f2 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -347,6 +347,8 @@ static int add_files(struct dir_struct *dir, int flags)\n>  \t\tdie(\"no files added\");\n>  \t}\n>\n> +\tif (!dir->nr)\n> +\t\tdie(\"No files selected for addition\");\n>  \tfor (i = 0; i < dir->nr; i++)\n>  \t\tif (add_file_to_cache(dir->entries[i]->name, flags)) {\n>  \t\t\tif (!ignore_add_errors)\n"},{"id":"139762","messageId":"n2i81b0412b1004171032v713f156ase295cbe7bbedf1f6@mail.gmail.com","threadId":"23501","inReplyTo":"7v4oja3uh7.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] Git add <device file> silently fails","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2010-04-17T17:32:22Z","receivedAt":"2010-04-17T17:32:22Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Sat, Apr 17, 2010 at 18:38, Junio C Hamano <gitster@pobox.com> wrote:\n> Alex Riesen <raa.lkml@gmail.com> writes:\n>\n>> I think something like this should make the accident more\n>> noticable:\n>\n> The early skippage done in dir.c (read-directory-recursive) should treat\n> these as ignored just like paths that are ignored with .gitignore\n> mechanism, and if we do so, we shouldn't need this patch to add another\n> codepath to give notification to the user (we would however still need\n> to reword \"'add -f' if you really want to add it\", though).\n>\n\nI see. Special files are not treated as ignored yet (and there will be\nno way to un-ignore them). I have to read the code for a while,\nignored pathnames are sometimes stored for later use too.\n"},{"id":"139766","messageId":"201004171957.00944.agruen@suse.de","threadId":"23501","inReplyTo":"7v4oja3uh7.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] Git add <device file> silently fails","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-04-17T17:57:00Z","receivedAt":"2010-04-17T17:57:00Z","isPatch":false,"sender":{"key":"agruen@suse.de","avatar":null},"body":"On Saturday 17 April 2010 18:38:12 Junio C Hamano wrote:\n> The early skippage done in dir.c (read-directory-recursive) should treat\n> these as ignored just like paths that are ignored with .gitignore\n> mechanism, and if we do so, we shouldn't need this patch to add another\n> codepath to give notification to the user (we would however still need\n> to reword \"'add -f' if you really want to add it\", though).\n\nI see, but dumbing down the error message until it fits both cases doesn'\nseem all that useful, either.  Here is a shot, maybe it's acceptable in\nyour eyes.\n\nAndreas\n\n\nSubject: [PATCH] Complain when trying to \"git add\" decive special files\n\nThis is done by adding a list of files to struct dir_struct which were\nignored because of their file type.\n\nSigned-off-by: Andreas Gruenbacher <agruen@suse.de>\n---\n builtin/add.c |   26 +++++++++++++++++---------\n dir.c         |   19 +++++++++++++------\n dir.h         |   14 ++++++++++++--\n 3 files changed, 42 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 87d2980..a3d7839 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -304,9 +304,6 @@ static int edit_patch(int argc, const char **argv, const char *prefix)\n \n static struct lock_file lock_file;\n \n-static const char ignore_error[] =\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;\n \n@@ -339,14 +336,25 @@ static int add_files(struct dir_struct *dir, int flags)\n {\n \tint i, exit_status = 0;\n \n-\tif (dir->ignored_nr) {\n-\t\tfprintf(stderr, ignore_error);\n-\t\tfor (i = 0; i < dir->ignored_nr; i++)\n-\t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n-\t\tfprintf(stderr, \"Use -f if you really want to add them.\\n\");\n-\t\tdie(\"no files added\");\n+\tif (dir->ignored[DIR_IGNORED_FILETYPE].nr) {\n+\t\tfprintf(stderr, \"The following paths are ignored \"\n+\t\t\t\t\"because their file types are not supported:\\n\");\n+\t\tfor (i = 0; i < dir->ignored[DIR_IGNORED_FILETYPE].nr; i++)\n+\t\t\tfprintf(stderr, \"%s\\n\",\n+\t\t\t\tdir->ignored[DIR_IGNORED_FILETYPE].entries[i]->name);\n \t}\n \n+\tif (dir->ignored[DIR_IGNORED_EXCLUDED].nr) {\n+\t\tfprintf(stderr, \"The following paths are ignored \"\n+\t\t\t\t\"by one of your .gitignore files; \"\n+\t\t\t\t\"use -f if you really want to add them:\\n\");\n+\t\tfor (i = 0; i < dir->ignored[DIR_IGNORED_EXCLUDED].nr; i++)\n+\t\t\tfprintf(stderr, \"%s\\n\",\n+\t\t\t\tdir->ignored[DIR_IGNORED_EXCLUDED].entries[i]->name);\n+\t}\n+\tif (dir->ignored[DIR_IGNORED_FILETYPE].nr || dir->ignored[DIR_IGNORED_EXCLUDED].nr)\n+\t\tdie(\"no files added\");\n+\n \tfor (i = 0; i < dir->nr; i++)\n \t\tif (add_file_to_cache(dir->entries[i]->name, flags)) {\n \t\t\tif (!ignore_add_errors)\ndiff --git a/dir.c b/dir.c\nindex cb83332..87e7fca 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -453,13 +453,16 @@ static struct dir_entry *dir_add_name(struct dir_struct *dir, const char \n*pathna\n \treturn dir->entries[dir->nr++] = dir_entry_new(pathname, len);\n }\n \n-static struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len)\n+static struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len,\n+\t\t\t\t\t enum ignore_reason reason)\n {\n+\tstruct dir_vector *vector = &dir->ignored[reason];\n+\n \tif (!cache_name_is_other(pathname, len))\n \t\treturn NULL;\n \n-\tALLOC_GROW(dir->ignored, dir->ignored_nr+1, dir->ignored_alloc);\n-\treturn dir->ignored[dir->ignored_nr++] = dir_entry_new(pathname, len);\n+\tALLOC_GROW(vector->entries, vector->nr+1, vector->alloc);\n+\treturn vector->entries[vector->nr++] = dir_entry_new(pathname, len);\n }\n \n enum exist_status {\n@@ -695,7 +698,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n \tint exclude = excluded(dir, path, &dtype);\n \tif (exclude && (dir->flags & DIR_COLLECT_IGNORED)\n \t    && exclude_matches_pathspec(path, *len, simplify))\n-\t\tdir_add_ignored(dir, path, *len);\n+\t\tdir_add_ignored(dir, path, *len, DIR_IGNORED_EXCLUDED);\n \n \t/*\n \t * Excluded? If we don't explicitly want to show\n@@ -720,7 +723,8 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n \n \tswitch (dtype) {\n \tdefault:\n-\t\treturn path_ignored;\n+\t\tdir_add_ignored(dir, path, *len, DIR_IGNORED_FILETYPE);\n+\t\tbreak;\n \tcase DT_DIR:\n \t\tmemcpy(path + *len, \"/\", 2);\n \t\t(*len)++;\n@@ -907,6 +911,7 @@ static int treat_leading_path(struct dir_struct *dir,\n int read_directory(struct dir_struct *dir, const char *path, int len, const char **pathspec)\n {\n \tstruct path_simplify *simplify;\n+\tenum ignore_reason reason;\n \n \tif (has_symlink_leading_path(path, len))\n \t\treturn dir->nr;\n@@ -916,7 +921,9 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const \nchar\n \t\tread_directory_recursive(dir, path, len, 0, simplify);\n \tfree_simplify(simplify);\n \tqsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);\n-\tqsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), cmp_name);\n+\tfor (reason = 0; reason < ARRAY_SIZE(dir->ignored); reason++)\n+\t\tqsort(dir->ignored[reason].entries, dir->ignored[reason].nr,\n+\t\t      sizeof(struct dir_entry *), cmp_name);\n \treturn dir->nr;\n }\n \ndiff --git a/dir.h b/dir.h\nindex 3bead5f..b7d1a21 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -31,9 +31,18 @@ struct exclude_stack {\n \tint exclude_ix;\n };\n \n+enum ignore_reason {\n+\tDIR_IGNORED_EXCLUDED,\n+\tDIR_IGNORED_FILETYPE,\n+};\n+\n+struct dir_vector {\n+\tint nr, alloc;\n+\tstruct dir_entry **entries;\n+};\n+\n struct dir_struct {\n \tint nr, alloc;\n-\tint ignored_nr, ignored_alloc;\n \tenum {\n \t\tDIR_SHOW_IGNORED = 1<<0,\n \t\tDIR_SHOW_OTHER_DIRECTORIES = 1<<1,\n@@ -42,7 +51,8 @@ struct dir_struct {\n \t\tDIR_COLLECT_IGNORED = 1<<4\n \t} flags;\n \tstruct dir_entry **entries;\n-\tstruct dir_entry **ignored;\n+\tstruct dir_vector ignored[2];\n+\n \n \t/* Exclude info */\n \tconst char *exclude_per_dir;\n-- \n1.7.1.rc1.12.ga601\n"},{"id":"139769","messageId":"v2o81b0412b1004171123z20c2c042qabb2b76143390a36@mail.gmail.com","threadId":"23501","inReplyTo":"n2i81b0412b1004171032v713f156ase295cbe7bbedf1f6@mail.gmail.com","subject":"Re: [BUG] Git add <device file> silently fails","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2010-04-17T18:23:26Z","receivedAt":"2010-04-17T18:23:26Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Sat, Apr 17, 2010 at 19:32, Alex Riesen <raa.lkml@gmail.com> wrote:\n> On Sat, Apr 17, 2010 at 18:38, Junio C Hamano <gitster@pobox.com> wrote:\n>> Alex Riesen <raa.lkml@gmail.com> writes:\n>>\n>>> I think something like this should make the accident more\n>>> noticable:\n>>\n>> The early skippage done in dir.c (read-directory-recursive) should treat\n>> these as ignored just like paths that are ignored with .gitignore\n>> mechanism, and if we do so, we shouldn't need this patch to add another\n>> codepath to give notification to the user (we would however still need\n>> to reword \"'add -f' if you really want to add it\", though).\n>>\n>\n> I see. Special files are not treated as ignored yet (and there will be\n> no way to un-ignore them). I have to read the code for a while,\n> ignored pathnames are sometimes stored for later use too.\n>\n\nI am tempted to add a mode_t or DT_something to struct dir_entry.\nUsers of read_directory are likely to do an lstat anyway (well,\nbuiltin/add.c does), and sometimes there is something to fill it with\n(get_dtype for DT_UNKNOWN). The only problem is that the structure\nis allocated a lot...\n"},{"id":"139863","messageId":"7vbpdgt43w.fsf@alter.siamese.dyndns.org","threadId":"23501","inReplyTo":"201004171957.00944.agruen@suse.de","subject":"Re: [BUG] Git add <device file> silently fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-19T05:15:31Z","receivedAt":"2010-04-19T05:15:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Gruenbacher <agruen@suse.de> writes:\n\n> @@ -720,7 +723,8 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n>  \n>  \tswitch (dtype) {\n>  \tdefault:\n> -\t\treturn path_ignored;\n> +\t\tdir_add_ignored(dir, path, *len, DIR_IGNORED_FILETYPE);\n> +\t\tbreak;\n\nHmm, do we want to break and return path_handled here, to cause\nthe calling read_directory_recursive() to call dir_add_name()?\n\nAlso I suspect that (dir->flags & DIR_COLLECT_IGNORED) needs to be checked\nbefore making this call.\n\n> +struct dir_vector {\n> +\tint nr, alloc;\n> +\tstruct dir_entry **entries;\n> +};\n\nWe would probably call a structure of this shape \"dir_array\", as I haven't\nseen us calling anything \"vector\" for naming consistency.\n\nInstead of introducing two dir-arrays for different kinds of ignoredness,\nit may be cleaner to add one bit (or more for later expansion) to dir_entry\nand mark the ones in ignored dir-array with the ignore reason.\n"}]}