{"thread":{"id":"28755","subject":"[PATCH/WIP 00/11] read_directory() rewrite to support struct pathspec","startedAt":"2011-10-24T06:36:05Z","lastAt":"2012-03-15T08:12:42Z","messageCount":50,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Nguyen Thai Ngoc Duy","Johan Herland","Johannes Sixt","David Bremner"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"178214","messageId":"1319438176-7304-1-git-send-email-pclouds@gmail.com","threadId":"28755","inReplyTo":null,"subject":"[PATCH/WIP 00/11] read_directory() rewrite to support struct pathspec","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T06:36:05Z","receivedAt":"2011-10-24T06:36:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is the first time \"make test\" fully passes (*) for me, so it's\nprobably good enough for human eyes. Just heads up where this might\ngo.\n\nA few points:\n\n - \"git add --ignore-missing\" is killed because I could not find an\n easy way to incorporate it to the new read_directory(). It looks\n like a hack to me, to expose .gitignore matching. Luckily no one\n except submodule seems to use it.\n\n - I chose to use tree_entry_interesting() instead of\n match_pathspec(). The former has more optimizations but requires a\n tree-based structure. So I have to read the whole directory in,\n re-construct a temporary tree object to make t_e_i() happy. I\n _think_ it does not impact performance with reasonable dir size.\n\n - there'll be more work to get rid of match_pathspec() calls after\n read_directory()/fill_directory(). I haven't got finished this part\n yet.\n\n - I really like to kill match_pathspec() so we only have one pathspec\n implementation instead of two now, but that may be real hard\n because of staged entries in index.\n\n(*) t7012.7 fails but I think that's the test's fault.\n\nNguyễn Thái Ngọc Duy (11):\n Introduce \"check-attr --excluded\" as a replacement for \"add --ignore-missing\"\n notes-merge: use opendir/readdir instead of using read_directory()\n t5403: avoid doing \"git add foo/bar\" where foo/.git exists\n tree-walk.c: do not leak internal structure in tree_entry_len()\n symbolize return values of tree_entry_interesting()\n read_directory_recursive: reduce one indentation level\n tree_entry_interesting: make use of local pointer \"item\"\n tree-walk: mark useful pathspecs\n tree_entry_interesting: differentiate partial vs full match\n read-dir: stop using path_simplify code in favor of tree_entry_interesting()\n dir.c: remove dead code after read_directory() rewrite\n\n Documentation/git-check-attr.txt | 4 +\n builtin/add.c | 36 ++--\n builtin/check-attr.c | 26 +++\n builtin/grep.c | 11 +-\n builtin/pack-objects.c | 2 +-\n cache.h | 1 +\n dir.c | 428 +++++++++++++++++++-------------------\n dir.h | 8 +-\n git-submodule.sh | 2 +-\n list-objects.c | 9 +-\n notes-merge.c | 45 +++--\n t/t3700-add.sh | 19 --\n t/t5403-post-checkout-hook.sh | 17 +-\n tree-diff.c | 19 +-\n tree-walk.c | 85 ++++----\n tree-walk.h | 19 ++-\n tree.c | 11 +-\n unpack-trees.c | 6 +-\n 18 files changed, 394 insertions(+), 354 deletions(-)\n\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178215","messageId":"1319438176-7304-2-git-send-email-pclouds@gmail.com","threadId":"28755","inReplyTo":"1319438176-7304-1-git-send-email-pclouds@gmail.com","subject":"[PATCH/WIP 01/11] Introduce \"check-attr --excluded\" as a replacement for \"add --ignore-missing\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T06:36:06Z","receivedAt":"2011-10-24T06:36:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"--ignore-missing is used by submodule to check if a path may be\nignored by .gitignore files. It does not really fit in git-add (git\nadd takes pathspec, but --ignore-missing takes only paths)\n\nGoogle reckons that --ignore-missing is not used anywhere but\ngit-submodule.sh. Remove --ignore-missing and introduce \"check-attr\n--excluded\" as a replacement.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/git-check-attr.txt | 4 ++++\n builtin/add.c | 14 +++-----------\n builtin/check-attr.c | 26 ++++++++++++++++++++++++++\n git-submodule.sh | 2 +-\n t/t3700-add.sh | 19 -------------------\n 5 files changed, 34 insertions(+), 31 deletions(-)\n\ndiff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt\nindex 5abdbaa..94d2068 100644\n--- a/Documentation/git-check-attr.txt\n+++ b/Documentation/git-check-attr.txt\n@@ -11,6 +11,7 @@ SYNOPSIS\n [verse]\n 'git check-attr' [-a | --all | attr...] [--] pathname...\n 'git check-attr' --stdin [-z] [-a | --all | attr...] < \n+'git check-attr' --excluded pathname...\n \n DESCRIPTION\n -----------\n@@ -34,6 +35,9 @@ OPTIONS\n \tOnly meaningful with `--stdin`; paths are separated with a\n \tNUL character instead of a linefeed character.\n \n+--excluded::\n+\tCheck if given paths are excluded by standard .gitignore rules.\n+\n \\--::\n \tInterpret all preceding arguments as attributes and all following\n \targuments as path names.\ndiff --git a/builtin/add.c b/builtin/add.c\nindex c59b0c9..23ad4b8 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -310,7 +310,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 ignore_add_errors, addremove, intent_to_add;\n \n static struct option builtin_add_options[] = {\n \tOPT__DRY_RUN(&show_only, \"dry run\"),\n@@ -325,7 +325,6 @@ static struct option builtin_add_options[] = {\n \tOPT_BOOLEAN('A', \"all\", &addremove, \"add changes from all tracked and untracked files\"),\n \tOPT_BOOLEAN( 0 , \"refresh\", &refresh_only, \"don't add, only refresh the index\"),\n \tOPT_BOOLEAN( 0 , \"ignore-errors\", &ignore_add_errors, \"just skip files which cannot be added because of errors\"),\n-\tOPT_BOOLEAN( 0 , \"ignore-missing\", &ignore_missing, \"check if - even missing - files are ignored in dry run\"),\n \tOPT_END(),\n };\n \n@@ -387,8 +386,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tif (addremove && take_worktree_changes)\n \t\tdie(_(\"-A and -u are mutually incompatible\"));\n-\tif (!show_only && ignore_missing)\n-\t\tdie(_(\"Option --ignore-missing can only be used together with --dry-run\"));\n \tif ((addremove || take_worktree_changes) && !argc) {\n \t\tstatic const char *here[2] = { \".\", NULL };\n \t\targc = 1;\n@@ -446,13 +443,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tfor (i = 0; pathspec[i]; i++) {\n \t\t\tif (!seen[i] && pathspec[i][0]\n \t\t\t && !file_exists(pathspec[i])) {\n-\t\t\t\tif (ignore_missing) {\n-\t\t\t\t\tint dtype = DT_UNKNOWN;\n-\t\t\t\t\tif (excluded(&dir, pathspec[i], &dtype))\n-\t\t\t\t\t\tdir_add_ignored(&dir, pathspec[i], strlen(pathspec[i]));\n-\t\t\t\t} else\n-\t\t\t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n-\t\t\t\t\t pathspec[i]);\n+\t\t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n+\t\t\t\t pathspec[i]);\n \t\t\t}\n \t\t}\n \t\tfree(seen);\ndiff --git a/builtin/check-attr.c b/builtin/check-attr.c\nindex 44c421e..4c17ccc 100644\n--- a/builtin/check-attr.c\n+++ b/builtin/check-attr.c\n@@ -2,11 +2,13 @@\n #include \"cache.h\"\n #include \"attr.h\"\n #include \"quote.h\"\n+#include \"dir.h\"\n #include \"parse-options.h\"\n \n static int all_attrs;\n static int cached_attrs;\n static int stdin_paths;\n+static int exclude;\n static const char * const check_attr_usage[] = {\n \"git check-attr [-a | --all | attr...] [--] pathname...\",\n \"git check-attr --stdin [-a | --all | attr...] < \",\n@@ -21,6 +23,7 @@ static const struct option check_attr_options[] = {\n \tOPT_BOOLEAN(0 , \"stdin\", &stdin_paths, \"read file names from stdin\"),\n \tOPT_BOOLEAN('z', NULL, &null_term_line,\n \t\t\"input paths are terminated by a null character\"),\n+\tOPT_BOOLEAN(0, \"excluded\", &exclude, \"check exclude patterns\"),\n \tOPT_END()\n };\n \n@@ -43,6 +46,16 @@ static void output_attr(int cnt, struct git_attr_check *check,\n \t}\n }\n \n+static void check_exclude(struct dir_struct *dir, const char *prefix, const char *file)\n+{\n+\tchar *full_path =\n+\t\tprefix_path(prefix, prefix ? strlen(prefix) : 0, file);\n+\tint dtype = DT_UNKNOWN;\n+\tif (excluded(dir, full_path, &dtype))\n+\t\tdie(\"%s is ignored by one of your .gitignore files\", full_path);\n+\tfree(full_path);\n+}\n+\n static void check_attr(const char *prefix, int cnt,\n \tstruct git_attr_check *check, const char *file)\n {\n@@ -103,6 +116,19 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)\n \t\tdie(\"invalid cache\");\n \t}\n \n+\tif (exclude) {\n+\t\tstruct dir_struct dir;\n+\n+\t\tif (stdin_paths)\n+\t\t\tdie(\"--excluded cannot be used with --stdin (yet)\");\n+\n+\t\tmemset(&dir, 0, sizeof(dir));\n+\t\tsetup_standard_excludes(&dir);\n+\t\tfor (i = 0; i < argc; i++)\n+\t\t\tcheck_exclude(&dir, prefix, argv[i]);\n+\t\treturn 0;\n+\t}\n+\n \tif (cached_attrs)\n \t\tgit_attr_set_direction(GIT_ATTR_INDEX, NULL);\n \ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 928a62f..0bc3762 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -262,7 +262,7 @@ cmd_add()\n \tgit ls-files --error-unmatch \"$path\" > /dev/null 2>&1 &&\n \tdie \"$(eval_gettext \"'\\$path' already exists in the index\")\"\n \n-\tif test -z \"$force\" && ! git add --dry-run --ignore-missing \"$path\" > /dev/null 2>&1\n+\tif test -z \"$force\" && ! git check-attr --excluded \"$path\" > /dev/null 2>&1\n \tthen\n \t\teval_gettextln \"The following path is ignored by one of your .gitignore files:\n \\$path\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex 575d950..23ff998 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -276,23 +276,4 @@ test_expect_success 'git add --dry-run of an existing file output' \"\n \ttest_i18ncmp expect actual\n \"\n \n-cat >expect.err <<\\EOF\n-The following paths are ignored by one of your .gitignore files:\n-ignored-file\n-Use -f if you really want to add them.\n-fatal: no files added\n-EOF\n-cat >expect.out <<\\EOF\n-add 'track-this'\n-EOF\n-\n-test_expect_success 'git add --dry-run --ignore-missing of non-existing file' '\n-\ttest_must_fail git add --dry-run --ignore-missing track-this ignored-file >actual.out 2>actual.err\n-'\n-\n-test_expect_success 'git add --dry-run --ignore-missing of non-existing file output' '\n-\ttest_i18ncmp expect.out actual.out &&\n-\ttest_i18ncmp expect.err actual.err\n-'\n-\n test_done\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178216","messageId":"1319438176-7304-3-git-send-email-pclouds@gmail.com","threadId":"28755","inReplyTo":"1319438176-7304-1-git-send-email-pclouds@gmail.com","subject":"[PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T06:36:07Z","receivedAt":"2011-10-24T06:36:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"notes_merge_commit() only needs to list all entries (non-recursively)\nunder a directory, which can be easily accomplished with\nopendir/readdir and would be more lightweight than read_directory().\n\nread_directory() is designed to list paths inside a working\ndirectory. Using it outside of its scope may lead to undesired effects.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n notes-merge.c | 45 +++++++++++++++++++++++++++------------------\n 1 files changed, 27 insertions(+), 18 deletions(-)\n\ndiff --git a/notes-merge.c b/notes-merge.c\nindex e9e4199..80d64a2 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -680,48 +680,57 @@ int notes_merge_commit(struct notes_merge_options *o,\n \t * commit message and parents from 'partial_commit'.\n \t * Finally store the new commit object SHA1 into 'result_sha1'.\n \t */\n-\tstruct dir_struct dir;\n-\tchar *path = xstrdup(git_path(NOTES_MERGE_WORKTREE \"/\"));\n-\tint path_len = strlen(path), i;\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tstruct strbuf path = STRBUF_INIT;\n \tconst char *msg = strstr(partial_commit->buffer, \"\\n\\n\");\n+\tint baselen;\n \n-\tOUTPUT(o, 3, \"Committing notes in notes merge worktree at %.*s\",\n-\t path_len - 1, path);\n+\tstrbuf_addstr(&path, git_path(NOTES_MERGE_WORKTREE));\n+\tOUTPUT(o, 3, \"Committing notes in notes merge worktree at %s\", path.buf);\n \n \tif (!msg || msg[2] == '\\0')\n \t\tdie(\"partial notes commit has empty message\");\n \tmsg += 2;\n \n-\tmemset(&dir, 0, sizeof(dir));\n-\tread_directory(&dir, path, path_len, NULL);\n-\tfor (i = 0; i < dir.nr; i++) {\n-\t\tstruct dir_entry *ent = dir.entries[i];\n+\tdir = opendir(path.buf);\n+\tif (!dir)\n+\t\tdie_errno(\"could not open %s\", path.buf);\n+\n+\tstrbuf_addch(&path, '/');\n+\tbaselen = path.len;\n+\twhile ((e = readdir(dir)) != NULL) {\n \t\tstruct stat st;\n-\t\tconst char *relpath = ent->name + path_len;\n \t\tunsigned char obj_sha1[20], blob_sha1[20];\n \n-\t\tif (ent->len - path_len != 40 || get_sha1_hex(relpath, obj_sha1)) {\n-\t\t\tOUTPUT(o, 3, \"Skipping non-SHA1 entry '%s'\", ent->name);\n+\t\tif (is_dot_or_dotdot(e->d_name))\n+\t\t\tcontinue;\n+\n+\t\tif (strlen(e->d_name) != 40 || get_sha1_hex(e->d_name, obj_sha1)) {\n+\t\t\tOUTPUT(o, 3, \"Skipping non-SHA1 entry '%s%s'\", path.buf, e->d_name);\n \t\t\tcontinue;\n \t\t}\n \n+\t\tstrbuf_addstr(&path, e->d_name);\n \t\t/* write file as blob, and add to partial_tree */\n-\t\tif (stat(ent->name, &st))\n-\t\t\tdie_errno(\"Failed to stat '%s'\", ent->name);\n-\t\tif (index_path(blob_sha1, ent->name, &st, HASH_WRITE_OBJECT))\n-\t\t\tdie(\"Failed to write blob object from '%s'\", ent->name);\n+\t\tif (stat(path.buf, &st))\n+\t\t\tdie_errno(\"Failed to stat '%s'\", path.buf);\n+\t\tif (index_path(blob_sha1, path.buf, &st, HASH_WRITE_OBJECT))\n+\t\t\tdie(\"Failed to write blob object from '%s'\", path.buf);\n \t\tif (add_note(partial_tree, obj_sha1, blob_sha1, NULL))\n \t\t\tdie(\"Failed to add resolved note '%s' to notes tree\",\n-\t\t\t ent->name);\n+\t\t\t path.buf);\n \t\tOUTPUT(o, 4, \"Added resolved note for object %s: %s\",\n \t\t sha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1));\n+\t\tstrbuf_setlen(&path, baselen);\n \t}\n \n \tcreate_notes_commit(partial_tree, partial_commit->parents, msg,\n \t\t\t result_sha1);\n \tOUTPUT(o, 4, \"Finalized notes merge commit: %s\",\n \t sha1_to_hex(result_sha1));\n-\tfree(path);\n+\tstrbuf_release(&path);\n+\tclosedir(dir);\n \treturn 0;\n }\n \n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178217","messageId":"1319438176-7304-4-git-send-email-pclouds@gmail.com","threadId":"28755","inReplyTo":"1319438176-7304-1-git-send-email-pclouds@gmail.com","subject":"[PATCH/WIP 03/11] t5403: avoid doing \"git add foo/bar\" where foo/.git exists","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T06:36:08Z","receivedAt":"2011-10-24T06:36:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"In this case, \"foo\" is considered a submodule and bar, if added,\nbelongs to foo/.git. \"git add\" should only allow \"git add foo\" in this\ncase, but it passes somehow.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n t/t5403-post-checkout-hook.sh | 17 ++++++++++-------\n 1 files changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh\nindex 1753ef2..3b3e2c1 100755\n--- a/t/t5403-post-checkout-hook.sh\n+++ b/t/t5403-post-checkout-hook.sh\n@@ -16,10 +16,13 @@ test_expect_success setup '\n \tgit update-ref refs/heads/master $commit0 &&\n \tgit clone ./. clone1 &&\n \tgit clone ./. clone2 &&\n-\tGIT_DIR=clone2/.git git branch new2 &&\n-\techo Data for commit1. >clone2/b &&\n-\tGIT_DIR=clone2/.git git add clone2/b &&\n-\tGIT_DIR=clone2/.git git commit -m new2\n+\t(\n+\t\tcd clone2 &&\n+\t\tgit branch new2 &&\n+\t\techo Data for commit1. >b &&\n+\t\tgit add b &&\n+\t\tgit commit -m new2\n+\t)\n '\n \n for clone in 1 2; do\n@@ -48,7 +51,7 @@ test_expect_success 'post-checkout runs as expected ' '\n '\n \n test_expect_success 'post-checkout args are correct with git checkout -b ' '\n-\tGIT_DIR=clone1/.git git checkout -b new1 &&\n+\t( cd clone1 && git checkout -b new1 ) &&\n \told=$(awk \"{print \\$1}\" clone1/.git/post-checkout.args) &&\n \tnew=$(awk \"{print \\$2}\" clone1/.git/post-checkout.args) &&\n \tflag=$(awk \"{print \\$3}\" clone1/.git/post-checkout.args) &&\n@@ -56,7 +59,7 @@ test_expect_success 'post-checkout args are correct with git checkout -b ' '\n '\n \n test_expect_success 'post-checkout receives the right args with HEAD changed ' '\n-\tGIT_DIR=clone2/.git git checkout new2 &&\n+\t( cd clone2 && git checkout new2 ) &&\n \told=$(awk \"{print \\$1}\" clone2/.git/post-checkout.args) &&\n \tnew=$(awk \"{print \\$2}\" clone2/.git/post-checkout.args) &&\n \tflag=$(awk \"{print \\$3}\" clone2/.git/post-checkout.args) &&\n@@ -64,7 +67,7 @@ test_expect_success 'post-checkout receives the right args with HEAD changed ' '\n '\n \n test_expect_success 'post-checkout receives the right args when not switching branches ' '\n-\tGIT_DIR=clone2/.git git checkout master b &&\n+\t( cd clone2 && git checkout master b ) &&\n \told=$(awk \"{print \\$1}\" clone2/.git/post-checkout.args) &&\n \tnew=$(awk \"{print \\$2}\" clone2/.git/post-checkout.args) &&\n \tflag=$(awk \"{print \\$3}\" clone2/.git/post-checkout.args) &&\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178218","messageId":"1319438176-7304-5-git-send-email-pclouds@gmail.com","threadId":"28755","inReplyTo":"1319438176-7304-1-git-send-email-pclouds@gmail.com","subject":"[PATCH/WIP 04/11] tree-walk.c: do not leak internal structure in tree_entry_len()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T06:36:09Z","receivedAt":"2011-10-24T06:36:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"tree_entry_len() does not simply take two random arguments and return\na tree length. The two pointers must point to a tree item structure,\nor struct name_entry. Passing random pointers will return incorrect\nvalue.\n\nForce callers to pass struct name_entry instead of two pointers (with\nhope that they don't manually construct struct name_entry themselves)\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/grep.c | 2 +-\n builtin/pack-objects.c | 2 +-\n tree-diff.c | 6 +++---\n tree-walk.c | 16 ++++++++--------\n tree-walk.h | 6 +++---\n tree.c | 2 +-\n unpack-trees.c | 6 +++---\n 7 files changed, 20 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 7d0779f..2cd0612 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -547,7 +547,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,\n \tint old_baselen = base->len;\n \n \twhile (tree_entry(tree, &entry)) {\n-\t\tint te_len = tree_entry_len(entry.path, entry.sha1);\n+\t\tint te_len = tree_entry_len(&entry);\n \n \t\tif (match != 2) {\n \t\t\tmatch = tree_entry_interesting(&entry, base, tn_len, pathspec);\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 2b18de5..864154b 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -975,7 +975,7 @@ static void add_pbase_object(struct tree_desc *tree,\n \twhile (tree_entry(tree,&entry)) {\n \t\tif (S_ISGITLINK(entry.mode))\n \t\t\tcontinue;\n-\t\tcmp = tree_entry_len(entry.path, entry.sha1) != cmplen ? 1 :\n+\t\tcmp = tree_entry_len(&entry) != cmplen ? 1 :\n \t\t memcmp(name, entry.path, cmplen);\n \t\tif (cmp > 0)\n \t\t\tcontinue;\ndiff --git a/tree-diff.c b/tree-diff.c\nindex b3cc2e4..6782484 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -21,8 +21,8 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,\n \tsha1 = tree_entry_extract(t1, &path1, &mode1);\n \tsha2 = tree_entry_extract(t2, &path2, &mode2);\n \n-\tpathlen1 = tree_entry_len(path1, sha1);\n-\tpathlen2 = tree_entry_len(path2, sha2);\n+\tpathlen1 = tree_entry_len(&t1->entry);\n+\tpathlen2 = tree_entry_len(&t2->entry);\n \tcmp = base_name_compare(path1, pathlen1, mode1, path2, pathlen2, mode2);\n \tif (cmp < 0) {\n \t\tshow_entry(opt, \"-\", t1, base);\n@@ -85,7 +85,7 @@ static void show_entry(struct diff_options *opt, const char *prefix,\n \tunsigned mode;\n \tconst char *path;\n \tconst unsigned char *sha1 = tree_entry_extract(desc, &path, &mode);\n-\tint pathlen = tree_entry_len(path, sha1);\n+\tint pathlen = tree_entry_len(&desc->entry);\n \tint old_baselen = base->len;\n \n \tstrbuf_add(base, path, pathlen);\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 418107e..f5d19f9 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -116,7 +116,7 @@ void setup_traverse_info(struct traverse_info *info, const char *base)\n \n char *make_traverse_path(char *path, const struct traverse_info *info, const struct name_entry *n)\n {\n-\tint len = tree_entry_len(n->path, n->sha1);\n+\tint len = tree_entry_len(n);\n \tint pathlen = info->pathlen;\n \n \tpath[pathlen + len] = 0;\n@@ -126,7 +126,7 @@ char *make_traverse_path(char *path, const struct traverse_info *info, const str\n \t\t\tbreak;\n \t\tpath[--pathlen] = '/';\n \t\tn = &info->name;\n-\t\tlen = tree_entry_len(n->path, n->sha1);\n+\t\tlen = tree_entry_len(n);\n \t\tinfo = info->prev;\n \t\tpathlen -= len;\n \t}\n@@ -253,7 +253,7 @@ static void extended_entry_extract(struct tree_desc_x *t,\n \t * The caller wants \"first\" from this tree, or nothing.\n \t */\n \tpath = a->path;\n-\tlen = tree_entry_len(a->path, a->sha1);\n+\tlen = tree_entry_len(a);\n \tswitch (check_entry_match(first, first_len, path, len)) {\n \tcase -1:\n \t\tentry_clear(a);\n@@ -271,7 +271,7 @@ static void extended_entry_extract(struct tree_desc_x *t,\n \twhile (probe.size) {\n \t\tentry_extract(&probe, a);\n \t\tpath = a->path;\n-\t\tlen = tree_entry_len(a->path, a->sha1);\n+\t\tlen = tree_entry_len(a);\n \t\tswitch (check_entry_match(first, first_len, path, len)) {\n \t\tcase -1:\n \t\t\tentry_clear(a);\n@@ -362,7 +362,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)\n \t\t\te = entry + i;\n \t\t\tif (!e->path)\n \t\t\t\tcontinue;\n-\t\t\tlen = tree_entry_len(e->path, e->sha1);\n+\t\t\tlen = tree_entry_len(e);\n \t\t\tif (!first) {\n \t\t\t\tfirst = e->path;\n \t\t\t\tfirst_len = len;\n@@ -381,7 +381,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)\n \t\t\t\t/* Cull the ones that are not the earliest */\n \t\t\t\tif (!e->path)\n \t\t\t\t\tcontinue;\n-\t\t\t\tlen = tree_entry_len(e->path, e->sha1);\n+\t\t\t\tlen = tree_entry_len(e);\n \t\t\t\tif (name_compare(e->path, len, first, first_len))\n \t\t\t\t\tentry_clear(e);\n \t\t\t}\n@@ -434,8 +434,8 @@ static int find_tree_entry(struct tree_desc *t, const char *name, unsigned char\n \t\tint entrylen, cmp;\n \n \t\tsha1 = tree_entry_extract(t, &entry, mode);\n+\t\tentrylen = tree_entry_len(&t->entry);\n \t\tupdate_tree_entry(t);\n-\t\tentrylen = tree_entry_len(entry, sha1);\n \t\tif (entrylen > namelen)\n \t\t\tcontinue;\n \t\tcmp = memcmp(name, entry, entrylen);\n@@ -596,7 +596,7 @@ int tree_entry_interesting(const struct name_entry *entry,\n \t\t\t\t ps->max_depth);\n \t}\n \n-\tpathlen = tree_entry_len(entry->path, entry->sha1);\n+\tpathlen = tree_entry_len(entry);\n \n \tfor (i = ps->nr - 1; i >= 0; i--) {\n \t\tconst struct pathspec_item *item = ps->items+i;\ndiff --git a/tree-walk.h b/tree-walk.h\nindex 0089581..884d01a 100644\n--- a/tree-walk.h\n+++ b/tree-walk.h\n@@ -20,9 +20,9 @@ static inline const unsigned char *tree_entry_extract(struct tree_desc *desc, co\n \treturn desc->entry.sha1;\n }\n \n-static inline int tree_entry_len(const char *name, const unsigned char *sha1)\n+static inline int tree_entry_len(const struct name_entry *ne)\n {\n-\treturn (const char *)sha1 - name - 1;\n+\treturn (const char *)ne->sha1 - ne->path - 1;\n }\n \n void update_tree_entry(struct tree_desc *);\n@@ -58,7 +58,7 @@ extern void setup_traverse_info(struct traverse_info *info, const char *base);\n \n static inline int traverse_path_len(const struct traverse_info *info, const struct name_entry *n)\n {\n-\treturn info->pathlen + tree_entry_len(n->path, n->sha1);\n+\treturn info->pathlen + tree_entry_len(n);\n }\n \n extern int tree_entry_interesting(const struct name_entry *, struct strbuf *, int, const struct pathspec *ps);\ndiff --git a/tree.c b/tree.c\nindex 698ecf7..e622198 100644\n--- a/tree.c\n+++ b/tree.c\n@@ -99,7 +99,7 @@ static int read_tree_1(struct tree *tree, struct strbuf *base,\n \t\telse\n \t\t\tcontinue;\n \n-\t\tlen = tree_entry_len(entry.path, entry.sha1);\n+\t\tlen = tree_entry_len(&entry);\n \t\tstrbuf_add(base, entry.path, len);\n \t\tstrbuf_addch(base, '/');\n \t\tretval = read_tree_1(lookup_tree(sha1),\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 8282f5e..7c9ecf6 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -446,7 +446,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,\n \tnewinfo.prev = info;\n \tnewinfo.pathspec = info->pathspec;\n \tnewinfo.name = *p;\n-\tnewinfo.pathlen += tree_entry_len(p->path, p->sha1) + 1;\n+\tnewinfo.pathlen += tree_entry_len(p) + 1;\n \tnewinfo.conflicts |= df_conflicts;\n \n \tfor (i = 0; i < n; i++, dirmask >>= 1) {\n@@ -495,7 +495,7 @@ static int do_compare_entry(const struct cache_entry *ce, const struct traverse_\n \tce_len -= pathlen;\n \tce_name = ce->name + pathlen;\n \n-\tlen = tree_entry_len(n->path, n->sha1);\n+\tlen = tree_entry_len(n);\n \treturn df_name_compare(ce_name, ce_len, S_IFREG, n->path, len, n->mode);\n }\n \n@@ -626,7 +626,7 @@ static int find_cache_pos(struct traverse_info *info,\n \tstruct unpack_trees_options *o = info->data;\n \tstruct index_state *index = o->src_index;\n \tint pfxlen = info->pathlen;\n-\tint p_len = tree_entry_len(p->path, p->sha1);\n+\tint p_len = tree_entry_len(p);\n \n \tfor (pos = o->cache_bottom; pos < index->cache_nr; pos++) {\n \t\tstruct cache_entry *ce = index->cache[pos];\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178219","messageId":"1319438176-7304-6-git-send-email-pclouds@gmail.com","threadId":"28755","inReplyTo":"1319438176-7304-1-git-send-email-pclouds@gmail.com","subject":"[PATCH/WIP 05/11] symbolize return values of tree_entry_interesting()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T06:36:10Z","receivedAt":"2011-10-24T06:36:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This helps extending the value later on for \"interesting, but cannot\ndecide if the entry truely matches yet\" (ie. prefix matches)\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/grep.c | 9 +++++----\n list-objects.c | 9 +++++----\n tree-diff.c | 13 +++++++------\n tree-walk.c | 45 +++++++++++++++++++++------------------------\n tree-walk.h | 12 +++++++++++-\n tree.c | 9 +++++----\n 6 files changed, 54 insertions(+), 43 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 2cd0612..2fc51fa 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -542,18 +542,19 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int\n static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,\n \t\t struct tree_desc *tree, struct strbuf *base, int tn_len)\n {\n-\tint hit = 0, match = 0;\n+\tint hit = 0;\n+\tenum interesting match = entry_not_interesting;\n \tstruct name_entry entry;\n \tint old_baselen = base->len;\n \n \twhile (tree_entry(tree, &entry)) {\n \t\tint te_len = tree_entry_len(&entry);\n \n-\t\tif (match != 2) {\n+\t\tif (match != all_entries_interesting) {\n \t\t\tmatch = tree_entry_interesting(&entry, base, tn_len, pathspec);\n-\t\t\tif (match < 0)\n+\t\t\tif (match == all_entries_not_interesting)\n \t\t\t\tbreak;\n-\t\t\tif (match == 0)\n+\t\t\tif (match == entry_not_interesting)\n \t\t\t\tcontinue;\n \t\t}\n \ndiff --git a/list-objects.c b/list-objects.c\nindex 39d80c0..3dd4a96 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -71,7 +71,8 @@ static void process_tree(struct rev_info *revs,\n \tstruct tree_desc desc;\n \tstruct name_entry entry;\n \tstruct name_path me;\n-\tint match = revs->diffopt.pathspec.nr == 0 ? 2 : 0;\n+\tenum interesting match = revs->diffopt.pathspec.nr == 0 ?\n+\t\tall_entries_interesting: entry_not_interesting;\n \tint baselen = base->len;\n \n \tif (!revs->tree_objects)\n@@ -97,12 +98,12 @@ static void process_tree(struct rev_info *revs,\n \tinit_tree_desc(&desc, tree->buffer, tree->size);\n \n \twhile (tree_entry(&desc, &entry)) {\n-\t\tif (match != 2) {\n+\t\tif (match != all_entries_interesting) {\n \t\t\tmatch = tree_entry_interesting(&entry, base, 0,\n \t\t\t\t\t\t &revs->diffopt.pathspec);\n-\t\t\tif (match < 0)\n+\t\t\tif (match == all_entries_not_interesting)\n \t\t\t\tbreak;\n-\t\t\tif (match == 0)\n+\t\t\tif (match == entry_not_interesting)\n \t\t\t\tcontinue;\n \t\t}\n \ndiff --git a/tree-diff.c b/tree-diff.c\nindex 6782484..25cc981 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -64,14 +64,14 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,\n static void show_tree(struct diff_options *opt, const char *prefix,\n \t\t struct tree_desc *desc, struct strbuf *base)\n {\n-\tint match = 0;\n+\tenum interesting match = entry_not_interesting;\n \tfor (; desc->size; update_tree_entry(desc)) {\n-\t\tif (match != 2) {\n+\t\tif (match != all_entries_interesting) {\n \t\t\tmatch = tree_entry_interesting(&desc->entry, base, 0,\n \t\t\t\t\t\t &opt->pathspec);\n-\t\t\tif (match < 0)\n+\t\t\tif (match == all_entries_not_interesting)\n \t\t\t\tbreak;\n-\t\t\tif (match == 0)\n+\t\t\tif (match == entry_not_interesting)\n \t\t\t\tcontinue;\n \t\t}\n \t\tshow_entry(opt, prefix, desc, base);\n@@ -114,12 +114,13 @@ static void show_entry(struct diff_options *opt, const char *prefix,\n }\n \n static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,\n-\t\t\t struct diff_options *opt, int *match)\n+\t\t\t struct diff_options *opt,\n+\t\t\t enum interesting *match)\n {\n \twhile (t->size) {\n \t\t*match = tree_entry_interesting(&t->entry, base, 0, &opt->pathspec);\n \t\tif (*match) {\n-\t\t\tif (*match < 0)\n+\t\t\tif (*match == all_entries_not_interesting)\n \t\t\t\tt->size = 0;\n \t\t\tbreak;\n \t\t}\ndiff --git a/tree-walk.c b/tree-walk.c\nindex f5d19f9..fc03262 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -573,27 +573,23 @@ static int match_dir_prefix(const char *base,\n *\n * Pre-condition: either baselen == base_offset (i.e. empty path)\n * or base[baselen-1] == '/' (i.e. with trailing slash).\n- *\n- * Return:\n- * - 2 for \"yes, and all subsequent entries will be\"\n- * - 1 for yes\n- * - zero for no\n- * - negative for \"no, and no subsequent entries will be either\"\n */\n-int tree_entry_interesting(const struct name_entry *entry,\n-\t\t\t struct strbuf *base, int base_offset,\n-\t\t\t const struct pathspec *ps)\n+enum interesting tree_entry_interesting(const struct name_entry *entry,\n+\t\t\t\t\tstruct strbuf *base, int base_offset,\n+\t\t\t\t\tconst struct pathspec *ps)\n {\n \tint i;\n \tint pathlen, baselen = base->len - base_offset;\n-\tint never_interesting = ps->has_wildcard ? 0 : -1;\n+\tint never_interesting = ps->has_wildcard ?\n+\t\tentry_not_interesting : all_entries_not_interesting;\n \n \tif (!ps->nr) {\n \t\tif (!ps->recursive || ps->max_depth == -1)\n-\t\t\treturn 2;\n-\t\treturn !!within_depth(base->buf + base_offset, baselen,\n-\t\t\t\t !!S_ISDIR(entry->mode),\n-\t\t\t\t ps->max_depth);\n+\t\t\treturn all_entries_interesting;\n+\t\treturn within_depth(base->buf + base_offset, baselen,\n+\t\t\t\t !!S_ISDIR(entry->mode),\n+\t\t\t\t ps->max_depth) ?\n+\t\t\tentry_interesting : entry_not_interesting;\n \t}\n \n \tpathlen = tree_entry_len(entry);\n@@ -610,12 +606,13 @@ int tree_entry_interesting(const struct name_entry *entry,\n \t\t\t\tgoto match_wildcards;\n \n \t\t\tif (!ps->recursive || ps->max_depth == -1)\n-\t\t\t\treturn 2;\n+\t\t\t\treturn all_entries_interesting;\n \n-\t\t\treturn !!within_depth(base_str + matchlen + 1,\n-\t\t\t\t\t baselen - matchlen - 1,\n-\t\t\t\t\t !!S_ISDIR(entry->mode),\n-\t\t\t\t\t ps->max_depth);\n+\t\t\treturn within_depth(base_str + matchlen + 1,\n+\t\t\t\t\t baselen - matchlen - 1,\n+\t\t\t\t\t !!S_ISDIR(entry->mode),\n+\t\t\t\t\t ps->max_depth) ?\n+\t\t\t\tentry_interesting : entry_not_interesting;\n \t\t}\n \n \t\t/* Either there must be no base, or the base must match. */\n@@ -623,18 +620,18 @@ int tree_entry_interesting(const struct name_entry *entry,\n \t\t\tif (match_entry(entry, pathlen,\n \t\t\t\t\tmatch + baselen, matchlen - baselen,\n \t\t\t\t\t&never_interesting))\n-\t\t\t\treturn 1;\n+\t\t\t\treturn entry_interesting;\n \n \t\t\tif (ps->items[i].use_wildcard) {\n \t\t\t\tif (!fnmatch(match + baselen, entry->path, 0))\n-\t\t\t\t\treturn 1;\n+\t\t\t\t\treturn entry_interesting;\n \n \t\t\t\t/*\n \t\t\t\t * Match all directories. We'll try to\n \t\t\t\t * match files later on.\n \t\t\t\t */\n \t\t\t\tif (ps->recursive && S_ISDIR(entry->mode))\n-\t\t\t\t\treturn 1;\n+\t\t\t\t\treturn entry_interesting;\n \t\t\t}\n \n \t\t\tcontinue;\n@@ -653,7 +650,7 @@ match_wildcards:\n \n \t\tif (!fnmatch(match, base->buf + base_offset, 0)) {\n \t\t\tstrbuf_setlen(base, base_offset + baselen);\n-\t\t\treturn 1;\n+\t\t\treturn entry_interesting;\n \t\t}\n \t\tstrbuf_setlen(base, base_offset + baselen);\n \n@@ -662,7 +659,7 @@ match_wildcards:\n \t\t * later on.\n \t\t */\n \t\tif (ps->recursive && S_ISDIR(entry->mode))\n-\t\t\treturn 1;\n+\t\t\treturn entry_interesting;\n \t}\n \treturn never_interesting; /* No matches */\n }\ndiff --git a/tree-walk.h b/tree-walk.h\nindex 884d01a..2bf0db9 100644\n--- a/tree-walk.h\n+++ b/tree-walk.h\n@@ -61,6 +61,16 @@ static inline int traverse_path_len(const struct traverse_info *info, const stru\n \treturn info->pathlen + tree_entry_len(n);\n }\n \n-extern int tree_entry_interesting(const struct name_entry *, struct strbuf *, int, const struct pathspec *ps);\n+/* in general, positive means \"kind of interesting\" */\n+enum interesting {\n+\tall_entries_not_interesting = -1, /* no, and no subsequent entries will be either */\n+\tentry_not_interesting = 0,\n+\tentry_interesting = 1,\n+\tall_entries_interesting = 2 /* yes, and all subsequent entries will be */\n+};\n+\n+extern enum interesting tree_entry_interesting(const struct name_entry *,\n+\t\t\t\t\t struct strbuf *, int,\n+\t\t\t\t\t const struct pathspec *ps);\n \n #endif\ndiff --git a/tree.c b/tree.c\nindex e622198..676e9f7 100644\n--- a/tree.c\n+++ b/tree.c\n@@ -52,7 +52,8 @@ static int read_tree_1(struct tree *tree, struct strbuf *base,\n \tstruct tree_desc desc;\n \tstruct name_entry entry;\n \tunsigned char sha1[20];\n-\tint len, retval = 0, oldlen = base->len;\n+\tint len, oldlen = base->len;\n+\tenum interesting retval = entry_not_interesting;\n \n \tif (parse_tree(tree))\n \t\treturn -1;\n@@ -60,11 +61,11 @@ static int read_tree_1(struct tree *tree, struct strbuf *base,\n \tinit_tree_desc(&desc, tree->buffer, tree->size);\n \n \twhile (tree_entry(&desc, &entry)) {\n-\t\tif (retval != 2) {\n+\t\tif (retval != all_entries_interesting) {\n \t\t\tretval = tree_entry_interesting(&entry, base, 0, pathspec);\n-\t\t\tif (retval < 0)\n+\t\t\tif (retval == all_entries_not_interesting)\n \t\t\t\tbreak;\n-\t\t\tif (retval == 0)\n+\t\t\tif (retval == entry_not_interesting)\n \t\t\t\tcontinue;\n \t\t}\n \n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178220","messageId":"1319438176-7304-7-git-send-email-pclouds@gmail.com","threadId":"28755","inReplyTo":"1319438176-7304-1-git-send-email-pclouds@gmail.com","subject":"[PATCH/WIP 06/11] read_directory_recursive: reduce one indentation level","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T06:36:11Z","receivedAt":"2011-10-24T06:36:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n dir.c | 50 +++++++++++++++++++++++++-------------------------\n 1 files changed, 25 insertions(+), 25 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 6c0d782..0a78d00 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -968,34 +968,34 @@ static int read_directory_recursive(struct dir_struct *dir,\n {\n \tDIR *fdir = opendir(*base ? base : \".\");\n \tint contents = 0;\n+\tstruct dirent *de;\n+\tchar path[PATH_MAX + 1];\n \n-\tif (fdir) {\n-\t\tstruct dirent *de;\n-\t\tchar path[PATH_MAX + 1];\n-\t\tmemcpy(path, base, baselen);\n-\n-\t\twhile ((de = readdir(fdir)) != NULL) {\n-\t\t\tint len;\n-\t\t\tswitch (treat_path(dir, de, path, sizeof(path),\n-\t\t\t\t\t baselen, simplify, &len)) {\n-\t\t\tcase path_recurse:\n-\t\t\t\tcontents += read_directory_recursive\n-\t\t\t\t\t(dir, path, len, 0, simplify);\n-\t\t\t\tcontinue;\n-\t\t\tcase path_ignored:\n-\t\t\t\tcontinue;\n-\t\t\tcase path_handled:\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t\tcontents++;\n-\t\t\tif (check_only)\n-\t\t\t\tgoto exit_early;\n-\t\t\telse\n-\t\t\t\tdir_add_name(dir, path, len);\n+\tif (!fdir)\n+\t\treturn 0;\n+\n+\tmemcpy(path, base, baselen);\n+\n+\twhile ((de = readdir(fdir)) != NULL) {\n+\t\tint len;\n+\t\tswitch (treat_path(dir, de, path, sizeof(path),\n+\t\t\t\t baselen, simplify, &len)) {\n+\t\tcase path_recurse:\n+\t\t\tcontents += read_directory_recursive(dir, path, len, 0, simplify);\n+\t\t\tcontinue;\n+\t\tcase path_ignored:\n+\t\t\tcontinue;\n+\t\tcase path_handled:\n+\t\t\tbreak;\n \t\t}\n-exit_early:\n-\t\tclosedir(fdir);\n+\t\tcontents++;\n+\t\tif (check_only)\n+\t\t\tgoto exit_early;\n+\t\telse\n+\t\t\tdir_add_name(dir, path, len);\n \t}\n+exit_early:\n+\tclosedir(fdir);\n \n \treturn contents;\n }\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178221","messageId":"1319438176-7304-8-git-send-email-pclouds@gmail.com","threadId":"28755","inReplyTo":"1319438176-7304-1-git-send-email-pclouds@gmail.com","subject":"[PATCH/WIP 07/11] tree_entry_interesting: make use of local pointer \"item\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T06:36:12Z","receivedAt":"2011-10-24T06:36:12Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n tree-walk.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/tree-walk.c b/tree-walk.c\nindex fc03262..2d9d17a 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -622,7 +622,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,\n \t\t\t\t\t&never_interesting))\n \t\t\t\treturn entry_interesting;\n \n-\t\t\tif (ps->items[i].use_wildcard) {\n+\t\t\tif (item->use_wildcard) {\n \t\t\t\tif (!fnmatch(match + baselen, entry->path, 0))\n \t\t\t\t\treturn entry_interesting;\n \n@@ -638,7 +638,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,\n \t\t}\n \n match_wildcards:\n-\t\tif (!ps->items[i].use_wildcard)\n+\t\tif (!item->use_wildcard)\n \t\t\tcontinue;\n \n \t\t/*\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178222","messageId":"1319438176-7304-9-git-send-email-pclouds@gmail.com","threadId":"28755","inReplyTo":"1319438176-7304-1-git-send-email-pclouds@gmail.com","subject":"[PATCH/WIP 08/11] tree-walk: mark useful pathspecs","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T06:36:13Z","receivedAt":"2011-10-24T06:36:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Useful pathspecs are those that help decide whether an item is in or\nout, as opposed to useless ones whose existence does not change the\nresults.\n\nCallers are responsible for cleaning before use, or doing anything\nafter.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n cache.h | 1 +\n tree-walk.c | 13 ++++++++++---\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex be07ec7..946d910 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -532,6 +532,7 @@ struct pathspec {\n \t\tconst char *match;\n \t\tint len;\n \t\tunsigned int use_wildcard:1;\n+\t\tunsigned int useful:1;\n \t} *items;\n };\n \ndiff --git a/tree-walk.c b/tree-walk.c\nindex 2d9d17a..5e9c522 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -595,11 +595,15 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,\n \tpathlen = tree_entry_len(entry);\n \n \tfor (i = ps->nr - 1; i >= 0; i--) {\n-\t\tconst struct pathspec_item *item = ps->items+i;\n+\t\tstruct pathspec_item *item = ps->items+i;\n \t\tconst char *match = item->match;\n \t\tconst char *base_str = base->buf + base_offset;\n \t\tint matchlen = item->len;\n \n+\t\t/* assume it will be used (which usually means break\n+\t\t the loop and return), reset it otherwise */\n+\t\titem->useful = 1;\n+\n \t\tif (baselen >= matchlen) {\n \t\t\t/* If it doesn't match, move along... */\n \t\t\tif (!match_dir_prefix(base_str, match, matchlen))\n@@ -634,12 +638,12 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,\n \t\t\t\t\treturn entry_interesting;\n \t\t\t}\n \n-\t\t\tcontinue;\n+\t\t\tgoto nouse;\n \t\t}\n \n match_wildcards:\n \t\tif (!item->use_wildcard)\n-\t\t\tcontinue;\n+\t\t\tgoto nouse;\n \n \t\t/*\n \t\t * Concatenate base and entry->path into one and do\n@@ -660,6 +664,9 @@ match_wildcards:\n \t\t */\n \t\tif (ps->recursive && S_ISDIR(entry->mode))\n \t\t\treturn entry_interesting;\n+\n+nouse:\n+\t\titem->useful = 0;\n \t}\n \treturn never_interesting; /* No matches */\n }\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178223","messageId":"1319438176-7304-10-git-send-email-pclouds@gmail.com","threadId":"28755","inReplyTo":"1319438176-7304-1-git-send-email-pclouds@gmail.com","subject":"[PATCH/WIP 09/11] tree_entry_interesting: differentiate partial vs full match","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T06:36:14Z","receivedAt":"2011-10-24T06:36:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Up until now, for a/b pathspec, both paths a and a/b would return\nentry_interesting. Make it return entry_matched for the latter.\n\nThis way if the caller follows up to \"a\", but decide to stop for some\nreason, then it knows that \"a\" has not really matched the given\npathspec yet.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n tree-walk.c | 13 ++++++++-----\n tree-walk.h | 5 +++--\n 2 files changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 5e9c522..6e12f0f 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -616,19 +616,22 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,\n \t\t\t\t\t baselen - matchlen - 1,\n \t\t\t\t\t !!S_ISDIR(entry->mode),\n \t\t\t\t\t ps->max_depth) ?\n-\t\t\t\tentry_interesting : entry_not_interesting;\n+\t\t\t\tentry_matched : entry_not_interesting;\n \t\t}\n \n \t\t/* Either there must be no base, or the base must match. */\n \t\tif (baselen == 0 || !strncmp(base_str, match, baselen)) {\n \t\t\tif (match_entry(entry, pathlen,\n \t\t\t\t\tmatch + baselen, matchlen - baselen,\n-\t\t\t\t\t&never_interesting))\n-\t\t\t\treturn entry_interesting;\n+\t\t\t\t\t&never_interesting)) {\n+\t\t\t\tif (match[baselen + pathlen] == '/')\n+\t\t\t\t\treturn entry_interesting;\n+\t\t\t\treturn entry_matched;\n+\t\t\t}\n \n \t\t\tif (item->use_wildcard) {\n \t\t\t\tif (!fnmatch(match + baselen, entry->path, 0))\n-\t\t\t\t\treturn entry_interesting;\n+\t\t\t\t\treturn entry_matched;\n \n \t\t\t\t/*\n \t\t\t\t * Match all directories. We'll try to\n@@ -654,7 +657,7 @@ match_wildcards:\n \n \t\tif (!fnmatch(match, base->buf + base_offset, 0)) {\n \t\t\tstrbuf_setlen(base, base_offset + baselen);\n-\t\t\treturn entry_interesting;\n+\t\t\treturn entry_matched;\n \t\t}\n \t\tstrbuf_setlen(base, base_offset + baselen);\n \ndiff --git a/tree-walk.h b/tree-walk.h\nindex 2bf0db9..a5f92fa 100644\n--- a/tree-walk.h\n+++ b/tree-walk.h\n@@ -65,8 +65,9 @@ static inline int traverse_path_len(const struct traverse_info *info, const stru\n enum interesting {\n \tall_entries_not_interesting = -1, /* no, and no subsequent entries will be either */\n \tentry_not_interesting = 0,\n-\tentry_interesting = 1,\n-\tall_entries_interesting = 2 /* yes, and all subsequent entries will be */\n+\tentry_interesting = 1, /* a potential match, not not there yet */\n+\tentry_matched = 2,\n+\tall_entries_interesting = 3 /* yes, and all subsequent entries will be */\n };\n \n extern enum interesting tree_entry_interesting(const struct name_entry *,\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178224","messageId":"1319438176-7304-11-git-send-email-pclouds@gmail.com","threadId":"28755","inReplyTo":"1319438176-7304-1-git-send-email-pclouds@gmail.com","subject":"[PATCH/WIP 10/11] read-dir: stop using path_simplify code in favor of tree_entry_interesting()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T06:36:15Z","receivedAt":"2011-10-24T06:36:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Current code tries to find a prefix set of given pathspecs and filter\non the set. Call sites are supposed to do exact pathspec matching\nagain to remove unmatched entries (but matches the prefix set).\n\nThis patch makes read_directory() use tree_entry_interesting()\ndirectly, thus remove the need to filter again by call sites (although\ncall sites are untouched in this patch).\n\nA less intrusive way would be to use match_pathspec_depth(), but I'd\nrather reduce the use of that function and eventually remove it, so we\nonly have to maintain pathspec matching at one place:\ntree_entry_interesting().\n\nIn order to make use of tree_entry_interesting(), directory content\nfrom readdir() must be converted to tree object format, which means we\nhave to read all items of a directory at once and sort it. If the\ndirectory is large, it may become expensive operation. But again,\ncurrent code does nothing to stop reading directory early, so nothing\nis lost.\n\nignored_nr and ignored[] are not longer filled. read_directory() users\nare supposed to use useful[] instead.\n\nMany functions are left unused in this patch to avoid clutter up the\npatch. They will be removed later.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/add.c | 22 +++--\n dir.c | 317 ++++++++++++++++++++++++++++++++++++++-------------------\n dir.h | 5 +\n tree-walk.c | 2 +\n 4 files changed, 236 insertions(+), 110 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 23ad4b8..92ba3d4 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -307,7 +307,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)\n static struct lock_file lock_file;\n \n static const char ignore_error[] =\n-N_(\"The following paths are ignored by one of your .gitignore files:\\n\");\n+N_(\"The following pathspecs 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@@ -342,12 +342,20 @@ 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->useful) {\n+\t\tint show_header = 0;\n+\t\tfor (i = 0; i < dir->ps2.nr; i++)\n+\t\t\tif (!dir->useful[i]) {\n+\t\t\t\tif (!show_header) {\n+\t\t\t\t\tfprintf(stderr, _(ignore_error));\n+\t\t\t\t\tshow_header = 1;\n+\t\t\t\t}\n+\t\t\t\tfprintf(stderr, \"%s\\n\", dir->ps2.items[i].match);\n+\t\t\t}\n+\t\tif (show_header) {\n+\t\t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n+\t\t\tdie(_(\"no files added\"));\n+\t\t}\n \t}\n \n \tfor (i = 0; i < dir->nr; i++)\ndiff --git a/dir.c b/dir.c\nindex 0a78d00..2946b2d 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -8,14 +8,18 @@\n #include \"cache.h\"\n #include \"dir.h\"\n #include \"refs.h\"\n+#include \"tree-walk.h\"\n+#include \"string-list.h\"\n \n struct path_simplify {\n \tint len;\n \tconst char *path;\n };\n \n-static int read_directory_recursive(struct dir_struct *dir, const char *path, int len,\n-\tint check_only, const struct path_simplify *simplify);\n+static int read_directory_recursive(struct dir_struct *dir,\n+\t\t\t\t struct strbuf *base,\n+\t\t\t\t int check_only,\n+\t\t\t\t enum interesting match);\n static int get_dtype(struct dirent *de, const char *path, int len);\n \n /* helper string functions with support for the ignore_case flag */\n@@ -609,6 +613,93 @@ struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname,\n \treturn dir->ignored[dir->ignored_nr++] = dir_entry_new(pathname, len);\n }\n \n+/* Read and convert directory to tree object (with invalid SHA-1) */\n+static void* dir_to_tree(struct strbuf *path, unsigned long *size)\n+{\n+\tint pathlen = path->len;\n+\tDIR *fdir = opendir(pathlen ? path->buf : \".\");\n+\tstruct string_list paths = STRING_LIST_INIT_DUP;\n+\tstruct dirent *de;\n+\tchar *tree, *p;\n+\tint i,dtype;\n+\n+\tif (!fdir)\n+\t\treturn NULL;\n+\n+\t*size = 0;\n+\twhile ((de = readdir(fdir)) != NULL) {\n+\t\tint namelen = strlen(de->d_name);\n+\t\tstruct string_list_item *item;\n+\t\tconst char *mode = NULL;\n+\n+\t\tif (is_dot_or_dotdot(de->d_name) ||\n+\t\t !strcmp(de->d_name, \".git\") ||\n+\t\t /* Ignore overly long pathnames! */\n+\t\t namelen + pathlen + 8 > PATH_MAX)\n+\t\t\tcontinue;\n+\n+\t\tstrbuf_add(path, de->d_name, namelen);\n+\t\tdtype = get_dtype(de, path->buf, path->len);\n+\t\tstrbuf_setlen(path, pathlen);\n+\n+\t\tswitch (dtype) {\n+\t\tcase DT_DIR: mode = \"040000 \"; break;\n+\t\tcase DT_REG: mode = \"100644 \"; break;\n+\t\tcase DT_LNK: mode = \"120000 \"; break;\n+\t\tdefault: continue;\n+\t\t}\n+\t\titem = string_list_insert(&paths, de->d_name);\n+\t\titem->util = (void*)mode;\n+\t\t/* 100644 SPC path NUL SHA-1 */\n+\t\t*size += 6 + 1 + namelen + 1 + 20;\n+\t}\n+\tclosedir(fdir);\n+\n+\ttree = xmalloc(*size);\n+\tfor (i = 0, p = tree;i < paths.nr; i++) {\n+\t\tint len = strlen(paths.items[i].string) + 1;\n+\t\tif (!paths.items[i].util ||\n+\t\t strlen(paths.items[i].util) != 7)\n+\t\t\tdie(\"BUG: util should contain a mode\");\n+\t\tmemcpy(p, paths.items[i].util, 7);\n+\t\tp += 7;\n+\t\tmemcpy(p, paths.items[i].string, len);\n+\t\tp += len;\n+\t\t/* we don't need valid SHA-1 for tree_entry_interesting() */\n+\t\tmemcpy(p, \"\\xbb\\xaa\\xdd\\xbb\\xaa\\xdd\\xbb\\xaa\\xdd\", 9);\n+\t\tp += 20;\n+\t}\n+\tstring_list_clear(&paths, 0);\n+\treturn tree;\n+}\n+\n+static enum interesting match_both_pathspecs(struct dir_struct *dir,\n+\t\t\t\t\t struct strbuf *base,\n+\t\t\t\t\t const struct name_entry *ne)\n+{\n+\tint i;\n+\tenum interesting ret1, ret2;\n+\n+\t/* ps1 contains the base path, no need to care about it */\n+\tfor (i = 0; i < dir->ps2.nr; i++)\n+\t\tdir->ps2.items[i].useful = 0;\n+\n+\tret1 = tree_entry_interesting(ne, base, 0, &dir->ps1);\n+\tif (ret1 <= 0)\n+\t\treturn ret1;\n+\tret2 = tree_entry_interesting(ne, base, 0, &dir->ps2);\n+\tif (ret2 <= 0)\n+\t\treturn ret2;\n+\n+\tif (ret1 == all_entries_interesting && ret2 == all_entries_interesting)\n+\t\treturn all_entries_interesting;\n+\telse if ((ret1 == entry_matched || ret1 == all_entries_interesting) &&\n+\t\t (ret2 == entry_matched || ret2 == all_entries_interesting))\n+\t\treturn entry_matched;\n+\telse\n+\t\treturn entry_interesting;\n+}\n+\n enum exist_status {\n \tindex_nonexistent = 0,\n \tindex_directory,\n@@ -722,11 +813,10 @@ enum directory_treatment {\n };\n \n static enum directory_treatment treat_directory(struct dir_struct *dir,\n-\tconst char *dirname, int len,\n-\tconst struct path_simplify *simplify)\n+\t\t\t\t\t\tstruct strbuf *dirname)\n {\n \t/* The \"len-1\" is to strip the final '/' */\n-\tswitch (directory_exists_in_index(dirname, len-1)) {\n+\tswitch (directory_exists_in_index(dirname->buf, dirname->len-1)) {\n \tcase index_directory:\n \t\treturn recurse_into_directory;\n \n@@ -740,7 +830,7 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,\n \t\t\tbreak;\n \t\tif (!(dir->flags & DIR_NO_GITLINKS)) {\n \t\t\tunsigned char sha1[20];\n-\t\t\tif (resolve_gitlink_ref(dirname, \"HEAD\", sha1) == 0)\n+\t\t\tif (resolve_gitlink_ref(dirname->buf, \"HEAD\", sha1) == 0)\n \t\t\t\treturn show_directory;\n \t\t}\n \t\treturn recurse_into_directory;\n@@ -749,7 +839,7 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,\n \t/* This is the \"show_other_directories\" case */\n \tif (!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))\n \t\treturn show_directory;\n-\tif (!read_directory_recursive(dir, dirname, len, 1, simplify))\n+\tif (!read_directory_recursive(dir, dirname, 1, entry_not_interesting))\n \t\treturn ignore_directory;\n \treturn show_directory;\n }\n@@ -780,31 +870,35 @@ static int simplify_away(const char *path, int pathlen, const struct path_simpli\n }\n \n /*\n- * This function tells us whether an excluded path matches a\n- * list of \"interesting\" pathspecs. That is, whether a path matched\n- * by any of the pathspecs could possibly be ignored by excluding\n- * the specified path. This can happen if:\n+ * This function flags pathspecs that are completely excluded, which\n+ * usually means an input mistake. In other words, if all matched\n+ * _files_ of a pathspec are excluded, flag the pathspec.\n *\n- * 1. the path is mentioned explicitly in the pathspec\n+ * The negated version would be: if any of matched files (by pathspec\n+ * X) are not excluded, pathspec X is clear, which is exactly what\n+ * this function does.\n *\n- * 2. the path is a directory prefix of some element in the\n- * pathspec\n+ * This function ignores dir->ps1 because that contains exactly one\n+ * pathspec item: the path base. No need to worry about that.\n */\n-static int exclude_matches_pathspec(const char *path, int len,\n-\t\tconst struct path_simplify *simplify)\n+static void mark_useful(struct dir_struct *dir,\n+\t\t\tconst char *path, int len,\n+\t\t\tint dtype,\n+\t\t\tstruct pathspec *ps, int exclude,\n+\t\t\tenum interesting match)\n {\n-\tif (simplify) {\n-\t\tfor (; simplify->path; simplify++) {\n-\t\t\tif (len == simplify->len\n-\t\t\t && !memcmp(path, simplify->path, len))\n-\t\t\t\treturn 1;\n-\t\t\tif (len < simplify->len\n-\t\t\t && simplify->path[len] == '/'\n-\t\t\t && !memcmp(path, simplify->path, len))\n-\t\t\t\treturn 1;\n-\t\t}\n-\t}\n-\treturn 0;\n+\tint i;\n+\tif (!(dir->flags & DIR_COLLECT_IGNORED))\n+\t\treturn;\n+\t/* half-matches (eg. prefix matches) do not count as useful */\n+\tif (match != all_entries_interesting && match != entry_matched)\n+\t\treturn;\n+\tif (exclude && cache_name_is_other(path, len))\n+\t\treturn;\n+\n+\tfor (i = 0; i < ps->nr; i++)\n+\t\tif (ps->items[i].useful)\n+\t\t\tdir->useful[i] = 1;\n }\n \n static int get_index_dtype(const char *path, int len)\n@@ -872,15 +966,24 @@ enum path_treatment {\n \tpath_recurse\n };\n \n-static enum path_treatment treat_one_path(struct dir_struct *dir,\n-\t\t\t\t\t char *path, int *len,\n-\t\t\t\t\t const struct path_simplify *simplify,\n-\t\t\t\t\t int dtype, struct dirent *de)\n+/* base is modified to contain ne */\n+static int treat_path(struct dir_struct *dir,\n+\t\t struct strbuf *base, const struct name_entry *ne,\n+\t\t enum interesting match)\n {\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+\tint exclude, dtype;\n+\n+\tstrbuf_add(base, ne->path, tree_entry_len(ne));\n+\n+\t/* It does not matter DT_REG or something else, excluded()\n+\t * only cares if it's DT_DIR or not */\n+\tdtype = S_ISDIR(ne->mode) ? DT_DIR : DT_REG;\n+\texclude = excluded(dir, base->buf, &dtype);\n+\n+\t/* intermediate directory match does not count */\n+\tif (dtype == DT_REG)\n+\t\tmark_useful(dir, base->buf, base->len, dtype,\n+\t\t\t\t &dir->ps2, exclude, match);\n \n \t/*\n \t * Excluded? If we don't explicitly want to show\n@@ -889,9 +992,6 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n \tif (exclude && !(dir->flags & DIR_SHOW_IGNORED))\n \t\treturn path_ignored;\n \n-\tif (dtype == DT_UNKNOWN)\n-\t\tdtype = get_dtype(de, path, *len);\n-\n \t/*\n \t * Do we want to see just the ignored files?\n \t * We still need to recurse into directories,\n@@ -899,17 +999,13 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n \t * directory may contain files that we do..\n \t */\n \tif (!exclude && (dir->flags & DIR_SHOW_IGNORED)) {\n-\t\tif (dtype != DT_DIR)\n+\t\tif (!S_ISDIR(ne->mode))\n \t\t\treturn path_ignored;\n \t}\n \n-\tswitch (dtype) {\n-\tdefault:\n-\t\treturn path_ignored;\n-\tcase DT_DIR:\n-\t\tmemcpy(path + *len, \"/\", 2);\n-\t\t(*len)++;\n-\t\tswitch (treat_directory(dir, path, *len, simplify)) {\n+\tif (S_ISDIR(ne->mode)) {\n+\t\tstrbuf_addch(base, '/');\n+\t\tswitch (treat_directory(dir, base)) {\n \t\tcase show_directory:\n \t\t\tif (exclude != !!(dir->flags\n \t\t\t\t\t & DIR_SHOW_IGNORED))\n@@ -920,38 +1016,14 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n \t\tcase ignore_directory:\n \t\t\treturn path_ignored;\n \t\t}\n-\t\tbreak;\n-\tcase DT_REG:\n-\tcase DT_LNK:\n-\t\tbreak;\n+\n+\t\t/* path_handled for dirs, must be gitlinks */\n+\t\tmark_useful(dir, base->buf, base->len, dtype,\n+\t\t\t\t &dir->ps2, exclude, match);\n \t}\n \treturn path_handled;\n }\n \n-static enum path_treatment treat_path(struct dir_struct *dir,\n-\t\t\t\t struct dirent *de,\n-\t\t\t\t char *path, int path_max,\n-\t\t\t\t int baselen,\n-\t\t\t\t const struct path_simplify *simplify,\n-\t\t\t\t int *len)\n-{\n-\tint dtype;\n-\n-\tif (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, \".git\"))\n-\t\treturn path_ignored;\n-\t*len = strlen(de->d_name);\n-\t/* Ignore overly long pathnames! */\n-\tif (*len + baselen + 8 > path_max)\n-\t\treturn path_ignored;\n-\tmemcpy(path + baselen, de->d_name, *len + 1);\n-\t*len += baselen;\n-\tif (simplify_away(path, *len, simplify))\n-\t\treturn path_ignored;\n-\n-\tdtype = DTYPE(de);\n-\treturn treat_one_path(dir, path, len, simplify, dtype, de);\n-}\n-\n /*\n * Read a directory tree. We currently ignore anything but\n * directories, regular files and symlinks. That's because git\n@@ -962,40 +1034,46 @@ static enum path_treatment treat_path(struct dir_struct *dir,\n * That likely will not change.\n */\n static int read_directory_recursive(struct dir_struct *dir,\n-\t\t\t\t const char *base, int baselen,\n+\t\t\t\t struct strbuf *base,\n \t\t\t\t int check_only,\n-\t\t\t\t const struct path_simplify *simplify)\n+\t\t\t\t enum interesting match)\n {\n-\tDIR *fdir = opendir(*base ? base : \".\");\n-\tint contents = 0;\n-\tstruct dirent *de;\n-\tchar path[PATH_MAX + 1];\n+\tunsigned long size;\n+\tvoid *tree_buf = dir_to_tree(base, &size);\n+\tint contents = 0, baselen = base->len;\n+\tstruct tree_desc desc;\n+\tstruct name_entry ne;\n \n-\tif (!fdir)\n+\tif (!tree_buf)\n \t\treturn 0;\n \n-\tmemcpy(path, base, baselen);\n+\tinit_tree_desc(&desc, tree_buf, size);\n \n-\twhile ((de = readdir(fdir)) != NULL) {\n-\t\tint len;\n-\t\tswitch (treat_path(dir, de, path, sizeof(path),\n-\t\t\t\t baselen, simplify, &len)) {\n+\twhile (tree_entry(&desc, &ne)) {\n+\t\tstrbuf_setlen(base, baselen);\n+\t\tif (match != all_entries_interesting) {\n+\t\t\tmatch = match_both_pathspecs(dir, base, &ne);\n+\t\t\tif (match == all_entries_not_interesting)\n+\t\t\t\tbreak;\n+\t\t\tif (match == entry_not_interesting)\n+\t\t\t\tcontinue;\n+\t\t}\n+\t\tswitch (treat_path(dir, base, &ne, match)) {\n \t\tcase path_recurse:\n-\t\t\tcontents += read_directory_recursive(dir, path, len, 0, simplify);\n-\t\t\tcontinue;\n-\t\tcase path_ignored:\n+\t\t\tcontents += read_directory_recursive(dir, base, 0, match);\n \t\t\tcontinue;\n \t\tcase path_handled:\n+\t\t\tcontents++;\n+\t\t\tif (check_only)\n+\t\t\t\tgoto exit_early;\n+\n+\t\t\tdir_add_name(dir, base->buf, base->len);\n \t\t\tbreak;\n \t\t}\n-\t\tcontents++;\n-\t\tif (check_only)\n-\t\t\tgoto exit_early;\n-\t\telse\n-\t\t\tdir_add_name(dir, path, len);\n \t}\n exit_early:\n-\tclosedir(fdir);\n+\tfree(tree_buf);\n+\tstrbuf_setlen(base, baselen);\n \n \treturn contents;\n }\n@@ -1054,6 +1132,7 @@ static void free_simplify(struct path_simplify *simplify)\n \tfree(simplify);\n }\n \n+#if 0\n static int treat_leading_path(struct dir_struct *dir,\n \t\t\t const char *path, int len,\n \t\t\t const struct path_simplify *simplify)\n@@ -1088,20 +1167,52 @@ static int treat_leading_path(struct dir_struct *dir,\n \t\t\treturn 1; /* finished checking */\n \t}\n }\n+#endif\n \n-int read_directory(struct dir_struct *dir, const char *path, int len, const char **pathspec)\n+int read_directory(struct dir_struct *dir, const char *path, int len,\n+\t\t const char **pathspec)\n {\n-\tstruct path_simplify *simplify;\n+\tchar *newpath = NULL;\n+\tstruct strbuf base = STRBUF_INIT;\n \n \tif (has_symlink_leading_path(path, len))\n \t\treturn dir->nr;\n \n-\tsimplify = create_simplify(pathspec);\n-\tif (!len || treat_leading_path(dir, path, len, simplify))\n-\t\tread_directory_recursive(dir, path, len, 0, simplify);\n-\tfree_simplify(simplify);\n+\t/*\n+\t * tree_entry_interesting() does not implement AND operator on\n+\t * pathspecs so we call tree_entry_interesting() twice and\n+\t * join the results ourselves in match_both_pathspecs()\n+\t */\n+\tif (path && *path) {\n+\t\tconst char *pathspec1[2];\n+\t\tnewpath = xmalloc(len + 1);\n+\t\tmemcpy(newpath, path, len);\n+\t\tnewpath[len] = 0;\n+\t\tpathspec1[0] = newpath;\n+\t\tpathspec1[1] = NULL;\n+\t\tinit_pathspec(&dir->ps1, pathspec1);\n+\t}\n+\telse\n+\t\tinit_pathspec(&dir->ps1, NULL);\n+\tinit_pathspec(&dir->ps2, pathspec);\n+\n+\tif (dir->flags & DIR_COLLECT_IGNORED) {\n+\t\tint size = sizeof(*dir->useful) * dir->ps2.nr;\n+\t\tdir->useful = xmalloc(size);\n+\t\t/* guilty until proven useful */\n+\t\tmemset(dir->useful, 0, size);\n+\t}\n+\n+\tread_directory_recursive(dir, &base, 0, entry_not_interesting);\n+\n+\tstrbuf_release(&base);\n+\tfree_pathspec(&dir->ps1);\n+\tif (!(dir->flags & DIR_COLLECT_IGNORED))\n+\t\tfree_pathspec(&dir->ps2);\n+\tfree(newpath);\n+\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+\n \treturn dir->nr;\n }\n \ndiff --git a/dir.h b/dir.h\nindex dd6947e..362d7b1 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -43,6 +43,11 @@ struct dir_struct {\n \t} flags;\n \tstruct dir_entry **entries;\n \tstruct dir_entry **ignored;\n+\tint *useful;\n+\n+\t/* Include info (a joint of ps1 and ps2) */\n+\tstruct pathspec ps1;\n+\tstruct pathspec ps2;\n \n \t/* Exclude info */\n \tconst char *exclude_per_dir;\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 6e12f0f..b56fec1 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -600,6 +600,8 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,\n \t\tconst char *base_str = base->buf + base_offset;\n \t\tint matchlen = item->len;\n \n+\t\t/* TODO: 07ccbff (runstatus: do not recurse into subdirectories if not needed - 2006-09-28) */\n+\n \t\t/* assume it will be used (which usually means break\n \t\t the loop and return), reset it otherwise */\n \t\titem->useful = 1;\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178225","messageId":"1319438176-7304-12-git-send-email-pclouds@gmail.com","threadId":"28755","inReplyTo":"1319438176-7304-1-git-send-email-pclouds@gmail.com","subject":"[PATCH/WIP 11/11] dir.c: remove dead code after read_directory() rewrite","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T06:36:16Z","receivedAt":"2011-10-24T06:36:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n dir.c | 121 -----------------------------------------------------------------\n dir.h | 3 --\n 2 files changed, 0 insertions(+), 124 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 2946b2d..4094962 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -11,11 +11,6 @@\n #include \"tree-walk.h\"\n #include \"string-list.h\"\n \n-struct path_simplify {\n-\tint len;\n-\tconst char *path;\n-};\n-\n static int read_directory_recursive(struct dir_struct *dir,\n \t\t\t\t struct strbuf *base,\n \t\t\t\t int check_only,\n@@ -604,15 +599,6 @@ static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathna\n \treturn dir->entries[dir->nr++] = dir_entry_new(pathname, len);\n }\n \n-struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len)\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-}\n-\n /* Read and convert directory to tree object (with invalid SHA-1) */\n static void* dir_to_tree(struct strbuf *path, unsigned long *size)\n {\n@@ -845,31 +831,6 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,\n }\n \n /*\n- * This is an inexact early pruning of any recursive directory\n- * reading - if the path cannot possibly be in the pathspec,\n- * return true, and we'll skip it early.\n- */\n-static int simplify_away(const char *path, int pathlen, const struct path_simplify *simplify)\n-{\n-\tif (simplify) {\n-\t\tfor (;;) {\n-\t\t\tconst char *match = simplify->path;\n-\t\t\tint len = simplify->len;\n-\n-\t\t\tif (!match)\n-\t\t\t\tbreak;\n-\t\t\tif (len > pathlen)\n-\t\t\t\tlen = pathlen;\n-\t\t\tif (!memcmp(path, match, len))\n-\t\t\t\treturn 0;\n-\t\t\tsimplify++;\n-\t\t}\n-\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n-\n-/*\n * This function flags pathspecs that are completely excluded, which\n * usually means an input mistake. In other words, if all matched\n * _files_ of a pathspec are excluded, flag the pathspec.\n@@ -1087,88 +1048,6 @@ static int cmp_name(const void *p1, const void *p2)\n \t\t\t\t e2->name, e2->len);\n }\n \n-/*\n- * Return the length of the \"simple\" part of a path match limiter.\n- */\n-static int simple_length(const char *match)\n-{\n-\tint len = -1;\n-\n-\tfor (;;) {\n-\t\tunsigned char c = *match++;\n-\t\tlen++;\n-\t\tif (c == '\\0' || is_glob_special(c))\n-\t\t\treturn len;\n-\t}\n-}\n-\n-static struct path_simplify *create_simplify(const char **pathspec)\n-{\n-\tint nr, alloc = 0;\n-\tstruct path_simplify *simplify = NULL;\n-\n-\tif (!pathspec)\n-\t\treturn NULL;\n-\n-\tfor (nr = 0 ; ; nr++) {\n-\t\tconst char *match;\n-\t\tif (nr >= alloc) {\n-\t\t\talloc = alloc_nr(alloc);\n-\t\t\tsimplify = xrealloc(simplify, alloc * sizeof(*simplify));\n-\t\t}\n-\t\tmatch = *pathspec++;\n-\t\tif (!match)\n-\t\t\tbreak;\n-\t\tsimplify[nr].path = match;\n-\t\tsimplify[nr].len = simple_length(match);\n-\t}\n-\tsimplify[nr].path = NULL;\n-\tsimplify[nr].len = 0;\n-\treturn simplify;\n-}\n-\n-static void free_simplify(struct path_simplify *simplify)\n-{\n-\tfree(simplify);\n-}\n-\n-#if 0\n-static int treat_leading_path(struct dir_struct *dir,\n-\t\t\t const char *path, int len,\n-\t\t\t const struct path_simplify *simplify)\n-{\n-\tchar pathbuf[PATH_MAX];\n-\tint baselen, blen;\n-\tconst char *cp;\n-\n-\twhile (len && path[len - 1] == '/')\n-\t\tlen--;\n-\tif (!len)\n-\t\treturn 1;\n-\tbaselen = 0;\n-\twhile (1) {\n-\t\tcp = path + baselen + !!baselen;\n-\t\tcp = memchr(cp, '/', path + len - cp);\n-\t\tif (!cp)\n-\t\t\tbaselen = len;\n-\t\telse\n-\t\t\tbaselen = cp - path;\n-\t\tmemcpy(pathbuf, path, baselen);\n-\t\tpathbuf[baselen] = '\\0';\n-\t\tif (!is_directory(pathbuf))\n-\t\t\treturn 0;\n-\t\tif (simplify_away(pathbuf, baselen, simplify))\n-\t\t\treturn 0;\n-\t\tblen = baselen;\n-\t\tif (treat_one_path(dir, pathbuf, &blen, simplify,\n-\t\t\t\t DT_DIR, NULL) == path_ignored)\n-\t\t\treturn 0; /* do not recurse into it */\n-\t\tif (len <= baselen)\n-\t\t\treturn 1; /* finished checking */\n-\t}\n-}\n-#endif\n-\n int read_directory(struct dir_struct *dir, const char *path, int len,\n \t\t const char **pathspec)\n {\ndiff --git a/dir.h b/dir.h\nindex 362d7b1..7a7d818 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -33,7 +33,6 @@ struct exclude_stack {\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 +41,6 @@ struct dir_struct {\n \t\tDIR_COLLECT_IGNORED = 1<<4\n \t} flags;\n \tstruct dir_entry **entries;\n-\tstruct dir_entry **ignored;\n \tint *useful;\n \n \t/* Include info (a joint of ps1 and ps2) */\n@@ -82,7 +80,6 @@ extern int read_directory(struct dir_struct *, const char *path, int len, const\n extern int excluded_from_list(const char *pathname, int pathlen, const char *basename,\n \t\t\t int *dtype, struct exclude_list *el);\n extern int excluded(struct dir_struct *, const char *, int *);\n-struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len);\n extern int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,\n \t\t\t\t\t char **buf_p, struct exclude_list *which, int check_index);\n extern void add_excludes_from_file(struct dir_struct *, const char *fname);\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178243","messageId":"7vty6y2u4s.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"1319438176-7304-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH/WIP 00/11] read_directory() rewrite to support struct pathspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-24T17:10:11Z","receivedAt":"2011-10-24T17:10:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> This is the first time \"make test\" fully passes (*) for me, so it's\n> probably good enough for human eyes.\n\nNice way to describe the done-ness of the series. Looking forward to read\nit through ;-)\n\nThanks.\n"},{"id":"178290","messageId":"7vd3dk516p.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"1319438176-7304-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH/WIP 03/11] t5403: avoid doing \"git add foo/bar\" where foo/.git exists","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-25T19:19:26Z","receivedAt":"2011-10-25T19:19:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> In this case, \"foo\" is considered a submodule and bar, if added,\n> belongs to foo/.git. \"git add\" should only allow \"git add foo\" in this\n> case, but it passes somehow.\n\nI do not think the above description is correct.\n\nThe test:\n\n - populates the current directory;\n - makes a clone in ./clone2;\n - creates a file clone2/b;\n - runs \"git add clone2/b\" with GIT_DIR set to clone2/.git, without\n setting GIT_WORK_TREE nor having core.worktree in clone2/.git/config.\n\nThe last step should add a path \"clone2/b\" to $GIT_DIR/index (which is\nclone2/.git/index in this case). The clone2 is not a submodule to the top\nlevel repository in this case, but even if it were, that would not change\nthe definition of what the command should do when GIT_DIR is set without\nGIT_WORK_TREE nor core.worktree in $GIT_DIR/config.\n\nRunning (cd clone2 && git add b) is a _more natural_ thing to do, and it\nwill result in a path \"b\" added to the clone2 repository, so that the\nresult is more useful if you are going to chdir to the repository and keep\nworking on it. But that does not mean the existing test is incorrect. It\ndoes not just pass somehow but the test passes by design.\n\nI did not check if later tests look at the contents of commit \"new2\" to\nmake sure it contains \"clone2/b\", but if they do this change should break\nsuch tests.\n\nSo I am puzzled by this change; what is this trying to achieve?\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> t/t5403-post-checkout-hook.sh | 17 ++++++++++-------\n> 1 files changed, 10 insertions(+), 7 deletions(-)\n>\n> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh\n> index 1753ef2..3b3e2c1 100755\n> --- a/t/t5403-post-checkout-hook.sh\n> +++ b/t/t5403-post-checkout-hook.sh\n> @@ -16,10 +16,13 @@ test_expect_success setup '\n> \tgit update-ref refs/heads/master $commit0 &&\n> \tgit clone ./. clone1 &&\n> \tgit clone ./. clone2 &&\n> -\tGIT_DIR=clone2/.git git branch new2 &&\n> -\techo Data for commit1. >clone2/b &&\n> -\tGIT_DIR=clone2/.git git add clone2/b &&\n> -\tGIT_DIR=clone2/.git git commit -m new2\n> +\t(\n> +\t\tcd clone2 &&\n> +\t\tgit branch new2 &&\n> +\t\techo Data for commit1. >b &&\n> +\t\tgit add b &&\n> +\t\tgit commit -m new2\n> +\t)\n> '\n> \n> for clone in 1 2; do\n> @@ -48,7 +51,7 @@ test_expect_success 'post-checkout runs as expected ' '\n> '\n> \n> test_expect_success 'post-checkout args are correct with git checkout -b ' '\n> -\tGIT_DIR=clone1/.git git checkout -b new1 &&\n> +\t( cd clone1 && git checkout -b new1 ) &&\n> \told=$(awk \"{print \\$1}\" clone1/.git/post-checkout.args) &&\n> \tnew=$(awk \"{print \\$2}\" clone1/.git/post-checkout.args) &&\n> \tflag=$(awk \"{print \\$3}\" clone1/.git/post-checkout.args) &&\n> @@ -56,7 +59,7 @@ test_expect_success 'post-checkout args are correct with git checkout -b ' '\n> '\n> \n> test_expect_success 'post-checkout receives the right args with HEAD changed ' '\n> -\tGIT_DIR=clone2/.git git checkout new2 &&\n> +\t( cd clone2 && git checkout new2 ) &&\n> \told=$(awk \"{print \\$1}\" clone2/.git/post-checkout.args) &&\n> \tnew=$(awk \"{print \\$2}\" clone2/.git/post-checkout.args) &&\n> \tflag=$(awk \"{print \\$3}\" clone2/.git/post-checkout.args) &&\n> @@ -64,7 +67,7 @@ test_expect_success 'post-checkout receives the right args with HEAD changed ' '\n> '\n> \n> test_expect_success 'post-checkout receives the right args when not switching branches ' '\n> -\tGIT_DIR=clone2/.git git checkout master b &&\n> +\t( cd clone2 && git checkout master b ) &&\n> \told=$(awk \"{print \\$1}\" clone2/.git/post-checkout.args) &&\n> \tnew=$(awk \"{print \\$2}\" clone2/.git/post-checkout.args) &&\n> \tflag=$(awk \"{print \\$3}\" clone2/.git/post-checkout.args) &&\n"},{"id":"178291","messageId":"7v8vo85156.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"1319438176-7304-5-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH/WIP 04/11] tree-walk.c: do not leak internal structure in tree_entry_len()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-25T19:20:21Z","receivedAt":"2011-10-25T19:20:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> tree_entry_len() does not simply take two random arguments and return\n> a tree length. The two pointers must point to a tree item structure,\n> or struct name_entry. Passing random pointers will return incorrect\n> value.\n>\n> Force callers to pass struct name_entry instead of two pointers (with\n> hope that they don't manually construct struct name_entry themselves)\n\nMakes quite a lot of sense.\n"},{"id":"178292","messageId":"7v4nyw50y1.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"1319438176-7304-6-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH/WIP 05/11] symbolize return values of tree_entry_interesting()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-25T19:24:38Z","receivedAt":"2011-10-25T19:24:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Makes it a lot easier to read for first-time readers. Nice.\n\nJust one minor formatting nit of lacking SP near \":\", though.\n"},{"id":"178293","messageId":"7vzkgo3m9b.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"1319438176-7304-3-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-25T19:27:12Z","receivedAt":"2011-10-25T19:27:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> notes_merge_commit() only needs to list all entries (non-recursively)\n> under a directory, which can be easily accomplished with\n> opendir/readdir and would be more lightweight than read_directory().\n>\n> read_directory() is designed to list paths inside a working\n> directory. Using it outside of its scope may lead to undesired effects.\n\nTechnically isn't the directory structure this codepath looks at a working\ntree that has extract of a notes tree commit?\n\nLooking at the result of the patch I do not have strong opinions either\nway, though. It isn't like we care about gitignore or attributes rules in\nthe notes tree, so using read_directory() does feel like an overkill.\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> notes-merge.c | 45 +++++++++++++++++++++++++++------------------\n> 1 files changed, 27 insertions(+), 18 deletions(-)\n>\n> diff --git a/notes-merge.c b/notes-merge.c\n> index e9e4199..80d64a2 100644\n> --- a/notes-merge.c\n> +++ b/notes-merge.c\n> @@ -680,48 +680,57 @@ int notes_merge_commit(struct notes_merge_options *o,\n> \t * commit message and parents from 'partial_commit'.\n> \t * Finally store the new commit object SHA1 into 'result_sha1'.\n> \t */\n> -\tstruct dir_struct dir;\n> -\tchar *path = xstrdup(git_path(NOTES_MERGE_WORKTREE \"/\"));\n> -\tint path_len = strlen(path), i;\n> +\tDIR *dir;\n> +\tstruct dirent *e;\n> +\tstruct strbuf path = STRBUF_INIT;\n> \tconst char *msg = strstr(partial_commit->buffer, \"\\n\\n\");\n> +\tint baselen;\n> \n> -\tOUTPUT(o, 3, \"Committing notes in notes merge worktree at %.*s\",\n> -\t path_len - 1, path);\n> +\tstrbuf_addstr(&path, git_path(NOTES_MERGE_WORKTREE));\n> +\tOUTPUT(o, 3, \"Committing notes in notes merge worktree at %s\", path.buf);\n> \n> \tif (!msg || msg[2] == '\\0')\n> \t\tdie(\"partial notes commit has empty message\");\n> \tmsg += 2;\n> \n> -\tmemset(&dir, 0, sizeof(dir));\n> -\tread_directory(&dir, path, path_len, NULL);\n> -\tfor (i = 0; i < dir.nr; i++) {\n> -\t\tstruct dir_entry *ent = dir.entries[i];\n> +\tdir = opendir(path.buf);\n> +\tif (!dir)\n> +\t\tdie_errno(\"could not open %s\", path.buf);\n> +\n> +\tstrbuf_addch(&path, '/');\n> +\tbaselen = path.len;\n> +\twhile ((e = readdir(dir)) != NULL) {\n> \t\tstruct stat st;\n> -\t\tconst char *relpath = ent->name + path_len;\n> \t\tunsigned char obj_sha1[20], blob_sha1[20];\n> \n> -\t\tif (ent->len - path_len != 40 || get_sha1_hex(relpath, obj_sha1)) {\n> -\t\t\tOUTPUT(o, 3, \"Skipping non-SHA1 entry '%s'\", ent->name);\n> +\t\tif (is_dot_or_dotdot(e->d_name))\n> +\t\t\tcontinue;\n> +\n> +\t\tif (strlen(e->d_name) != 40 || get_sha1_hex(e->d_name, obj_sha1)) {\n> +\t\t\tOUTPUT(o, 3, \"Skipping non-SHA1 entry '%s%s'\", path.buf, e->d_name);\n> \t\t\tcontinue;\n> \t\t}\n> \n> +\t\tstrbuf_addstr(&path, e->d_name);\n> \t\t/* write file as blob, and add to partial_tree */\n> -\t\tif (stat(ent->name, &st))\n> -\t\t\tdie_errno(\"Failed to stat '%s'\", ent->name);\n> -\t\tif (index_path(blob_sha1, ent->name, &st, HASH_WRITE_OBJECT))\n> -\t\t\tdie(\"Failed to write blob object from '%s'\", ent->name);\n> +\t\tif (stat(path.buf, &st))\n> +\t\t\tdie_errno(\"Failed to stat '%s'\", path.buf);\n> +\t\tif (index_path(blob_sha1, path.buf, &st, HASH_WRITE_OBJECT))\n> +\t\t\tdie(\"Failed to write blob object from '%s'\", path.buf);\n> \t\tif (add_note(partial_tree, obj_sha1, blob_sha1, NULL))\n> \t\t\tdie(\"Failed to add resolved note '%s' to notes tree\",\n> -\t\t\t ent->name);\n> +\t\t\t path.buf);\n> \t\tOUTPUT(o, 4, \"Added resolved note for object %s: %s\",\n> \t\t sha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1));\n> +\t\tstrbuf_setlen(&path, baselen);\n> \t}\n> \n> \tcreate_notes_commit(partial_tree, partial_commit->parents, msg,\n> \t\t\t result_sha1);\n> \tOUTPUT(o, 4, \"Finalized notes merge commit: %s\",\n> \t sha1_to_hex(result_sha1));\n> -\tfree(path);\n> +\tstrbuf_release(&path);\n> +\tclosedir(dir);\n> \treturn 0;\n> }\n"},{"id":"178301","messageId":"CACsJy8CocoAiVx_PeaaX1oRZvmzfj9-z9JLJkE5unSRVtpGkNA@mail.gmail.com","threadId":"28755","inReplyTo":"7vzkgo3m9b.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-26T00:08:55Z","receivedAt":"2011-10-26T00:08:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/10/26 Junio C Hamano :\n> Nguyễn Thái Ngọc Duy   writes:\n>\n>> notes_merge_commit() only needs to list all entries (non-recursively)\n>> under a directory, which can be easily accomplished with\n>> opendir/readdir and would be more lightweight than read_directory().\n>>\n>> read_directory() is designed to list paths inside a working\n>> directory. Using it outside of its scope may lead to undesired effects.\n>\n> Technically isn't the directory structure this codepath looks at a working\n> tree that has extract of a notes tree commit?\n\nYes it's like a secondary working tree, only for notes, if I read the\ncode correctly. The thing is this space is inside \".git\".\n\nCurrent read_directory() treats given path separately from contents\ninside the path. If the given path has \".git\", it's ok (but it'll stop\nat .git if during tree recursion). The new read_directory() does not\nmake this exception, so when note-merge call\nread_directory(\".git/NOTES_MERGE_WORKTREE\"), read_directory() sees\n\".git\" and stops immediately, assuming it's a gitlink.\n\nOne could say we should keep current behavior, but I don't really see\nit's worth the effort.\n-- \nDuy\n"},{"id":"178303","messageId":"CACsJy8CjJnO6rDVTV1tC9rWXP51LHBtUvNsgVWNfwC+HuNQ-6Q@mail.gmail.com","threadId":"28755","inReplyTo":"7vd3dk516p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/WIP 03/11] t5403: avoid doing \"git add foo/bar\" where foo/.git exists","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-26T00:18:51Z","receivedAt":"2011-10-26T00:18:51Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/10/26 Junio C Hamano :\n> Nguyễn Thái Ngọc Duy   writes:\n>\n>> In this case, \"foo\" is considered a submodule and bar, if added,\n>> belongs to foo/.git. \"git add\" should only allow \"git add foo\" in this\n>> case, but it passes somehow.\n>\n> I do not think the above description is correct.\n>\n> The test:\n>\n>  - populates the current directory;\n>  - makes a clone in ./clone2;\n>  - creates a file clone2/b;\n>  - runs \"git add clone2/b\" with GIT_DIR set to clone2/.git, without\n>   setting GIT_WORK_TREE nor having core.worktree in clone2/.git/config.\n>\n> The last step should add a path \"clone2/b\" to $GIT_DIR/index (which is\n> clone2/.git/index in this case).  The clone2 is not a submodule to the top\n> level repository in this case, but even if it were, that would not change\n> the definition of what the command should do when GIT_DIR is set without\n> GIT_WORK_TREE nor core.worktree in $GIT_DIR/config.\n\nNow look from \"git add\" perspective, it does not really care where\n$GIT_DIR is. It assumes that $(pwd) is working directory's top. So it\n\n - reads content of current directory, it sees \"clone2\" as a directory\n - it descends in and see \".git\" so \"clone2\" must be a git link\n - because clone2 is a separate repository (again $GIT_DIR is not\nconsulted), \"b\" should be managed by \"clone2\"\n - so it stops.\n\nThis is the only place I see a submodule (from the first glance) is\nactually top level repository. Yes I guess we can support this, but\nit's just too weird to be widely used in pratice..\n\n> Running (cd clone2 && git add b) is a _more natural_ thing to do, and it\n> will result in a path \"b\" added to the clone2 repository, so that the\n> result is more useful if you are going to chdir to the repository and keep\n> working on it.  But that does not mean the existing test is incorrect. It\n> does not just pass somehow but the test passes by design.\n>\n> I did not check if later tests look at the contents of commit \"new2\" to\n> make sure it contains \"clone2/b\", but if they do this change should break\n> such tests.\n>\n> So I am puzzled by this change; what is this trying to achieve?\n-- \nDuy\n"},{"id":"178327","messageId":"7vr51z3bqx.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"CACsJy8CjJnO6rDVTV1tC9rWXP51LHBtUvNsgVWNfwC+HuNQ-6Q@mail.gmail.com","subject":"Re: [PATCH/WIP 03/11] t5403: avoid doing \"git add foo/bar\" where foo/.git exists","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-26T17:26:30Z","receivedAt":"2011-10-26T17:26:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> Now look from \"git add\" perspective, it does not really care where\n> $GIT_DIR is.\n> It assumes that $(pwd) is working directory's top. So it\n\nNow you confused me.\n\nDoesn't it use $GIT_DIR to find the index? And it decides that it is at\nthe top level because it is given GIT_DIR but not GIT_WORKING_TREE which\nis how working tree discovery is defined.\n\n> - reads content of current directory, it sees \"clone2\" as a directory\n> - it descends in and see \".git\" so \"clone2\" must be a git link\n> - because clone2 is a separate repository (again $GIT_DIR is not\n> consulted), \"b\" should be managed by \"clone2\"\n> - so it stops.\n>\n> This is the only place I see a submodule (from the first glance) is\n> actually top level repository. Yes I guess we can support this, but\n> it's just too weird to be widely used in pratice..\n\nWhere did you get this idea that submodule is somehow involved in this test?\n\nI do not see there is any submodules involved; the test uses two\nrepositories 1 and 2 that appear in the working tree of the main\nrepository test infrastructure created, but otherwise there is no\nsub/super relation among these three, and there are many other tests with\n\"clone\" or \"mkdir+init\" or \"init \" in the main test repository.\n\nThe clone2 repository tracks a path without having a corresponding file in\nits working tree (i.e. it has \"b\" but tracks \"clone2/b\") which I already\nis said unusual, but unusual does not mean it is bad or we want to remove\na test that covers the unusual case to let a change that regresses the\ncase go unnoticed.\n\n>> Running (cd clone2 && git add b) is a _more natural_ thing to do, and it\n>> will result in a path \"b\" added to the clone2 repository, so that the\n>> result is more useful if you are going to chdir to the repository and keep\n>> working on it. But that does not mean the existing test is incorrect. It\n>> does not just pass somehow but the test passes by design.\n>>\n>> I did not check if later tests look at the contents of commit \"new2\" to\n>> make sure it contains \"clone2/b\", but if they do this change should break\n>> such tests.\n>>\n>> So I am puzzled by this change; what is this trying to achieve?\n\nSo again, what is this change trying to achieve?\n"},{"id":"178328","messageId":"7vmxcn3b8w.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"CACsJy8CocoAiVx_PeaaX1oRZvmzfj9-z9JLJkE5unSRVtpGkNA@mail.gmail.com","subject":"Re: [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-26T17:37:19Z","receivedAt":"2011-10-26T17:37:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> Current read_directory() treats given path separately from contents\n> inside the path. If the given path has \".git\", it's ok (but it'll stop\n> at .git if during tree recursion). The new read_directory() does not\n> make this exception, so when note-merge call\n> read_directory(\".git/NOTES_MERGE_WORKTREE\"), read_directory() sees\n> \".git\" and stops immediately, assuming it's a gitlink.\n\nWhen read_directory(\"where/ever\") is called, what kind of paths does it\ncollect? Do the paths the function collects share \"where/ever\" as their\ncommon prefix? I thought it collects the paths relative to whatever\ntop-level directory given to the function, so that \"where/ever\" could be\nanything.\n\nWhy does it even have to look at the given path in the first place and\nmake a decision heavier than \"can I opendir() and read from it\"? In other\nwords, if you see read_directory(\"some/thing/.git/more/stuff\") and find a\nsubstring \".git/\" in there, what \"magic\" gitlink handling does the code\nhave to do?\n\nI do not think it matters for _this_ particular case, but I can for\nexample imagine an alternative implementation of a merge that uses\ntemporary working tree somewhere other than the main working tree, and one\nof the natural \"temporary\" places such a feature in the future may want to\nuse is inside .git/ somewhere. If you are planning to close the door by\nbreaking the behaviour of read_directory(\".git/some_where\") for such\ncallers with this series, we need to be aware of it, and that is why I am\nnot satisfied by your explanation.\n"},{"id":"178346","messageId":"CACsJy8C4iHffr4UYP9SvCU0OPC-LocUORwAQ492LqaV_tyvFQA@mail.gmail.com","threadId":"28755","inReplyTo":"7vmxcn3b8w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-27T07:51:38Z","receivedAt":"2011-10-27T07:51:38Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Oct 27, 2011 at 4:37 AM, Junio C Hamano wrote:\n> Nguyen Thai Ngoc Duy writes:\n>\n>> Current read_directory() treats given path separately from contents\n>> inside the path. If the given path has \".git\", it's ok (but it'll stop\n>> at .git if during tree recursion). The new read_directory() does not\n>> make this exception, so when note-merge call\n>> read_directory(\".git/NOTES_MERGE_WORKTREE\"), read_directory() sees\n>> \".git\" and stops immediately, assuming it's a gitlink.\n>\n> When read_directory(\"where/ever\") is called, what kind of paths does it\n> collect? Do the paths the function collects share \"where/ever\" as their\n> common prefix? I thought it collects the paths relative to whatever\n> top-level directory given to the function, so that \"where/ever\" could be\n> anything.\n\nCorrect. But read_directory() takes pathspec now so naturally it does\nnot treat \"where/ever\" a common prefix anymore. So it has to open(\".\")\nand starts from there. Even current code does not trust \"where/ever\"\ncompletely. treat_leading_path() may dismiss \"where/ever\" if it's\nexcluded by .gitignore.\n\n> Why does it even have to look at the given path in the first place and\n> make a decision heavier than \"can I opendir() and read from it\"?\n\nBecause opendir(\"wh*/*r\") may fail.\n\n> In other\n> words, if you see read_directory(\"some/thing/.git/more/stuff\") and find a\n> substring \".git/\" in there, what \"magic\" gitlink handling does the code\n> have to do?\n\n\"some/thing/.git\" can be considered a new entry in index, so it should\nstop traversing at \".git\". But because \"some/thing/.git\" does not\nexacly match \"some/thing/.git/more/stuff\", it is filtered out.\n\ngit-add deals with this case especially in order to avoid accidentally\nreplace \"some/thing/.git\" in index with \"some/thing/.git/more/stuff\".\nBut I feel it should be handled by read_directory(), not git-add.\n\n> I do not think it matters for _this_ particular case, but I can for\n> example imagine an alternative implementation of a merge that uses\n> temporary working tree somewhere other than the main working tree, and one\n> of the natural \"temporary\" places such a feature in the future may want to\n> use is inside .git/ somewhere. If you are planning to close the door by\n> breaking the behaviour of read_directory(\".git/some_where\") for such\n> callers with this series, we need to be aware of it, and that is why I am\n> not satisfied by your explanation.\n\nMaybe I should step back a bit. Instead of treating any input to\nread_directory() as pathspec, callers may provide two sets: a prefix\nset and a pathspec set. read_directory() starts from the prefix set\nwithout any checks, then descends in using pathspec.\n\nBut then what about the \"if (treat_one_path(..) == path_ignored)\" in\ntreat_leading_path()? Remove it?\n-- \nDuy\n"},{"id":"178347","messageId":"CACsJy8C2nUJkN5=E7p2u_wjHqWy7EC_BS3Sr4+_QgunWHDdtKg@mail.gmail.com","threadId":"28755","inReplyTo":"7vr51z3bqx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/WIP 03/11] t5403: avoid doing \"git add foo/bar\" where foo/.git exists","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-27T08:06:34Z","receivedAt":"2011-10-27T08:06:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Oct 27, 2011 at 4:26 AM, Junio C Hamano wrote:\n>>  - reads content of current directory, it sees \"clone2\" as a directory\n>>  - it descends in and see \".git\" so \"clone2\" must be a git link\n>>  - because clone2 is a separate repository (again $GIT_DIR is not\n>> consulted), \"b\" should be managed by \"clone2\"\n>>  - so it stops.\n>>\n>> This is the only place I see a submodule (from the first glance) is\n>> actually top level repository. Yes I guess we can support this, but\n>> it's just too weird to be widely used in pratice..\n>\n> Where did you get this idea that submodule is somehow involved in this test?\n\nBecause \"clone2\" looks like a submodule (it has \".git\" inside with valid HEAD)\n\n> I do not see there is any submodules involved; the test uses two\n> repositories 1 and 2 that appear in the working tree of the main\n> repository test infrastructure created, but otherwise there is no\n> sub/super relation among these three, and there are many other tests with\n> \"clone\" or \"mkdir+init\" or \"init \" in the main test repository.\n\nIf I tweak the test a bit\n\ngit clone ./. clone2 &&\nGIT_DIR=clone2/.git git add clone2 &&\nGIT_DIR=clone2/.git git add clone2/b\n\nthen the last command fails with \"Path 'clone2/b' is in submodule\n'clone2'\". So clone2 could be a submodule from that perspective. Doing\nthe the other way around\n\ngit clone ./. clone2 &&\nGIT_DIR=clone2/.git git add clone2/b &&\nGIT_DIR=clone2/.git git add clone2\n\n\"clone2\" is not just a normal path. Should we stick with one way only?\n-- \nDuy\n"},{"id":"178358","messageId":"7vzkgmz6v0.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"CACsJy8C4iHffr4UYP9SvCU0OPC-LocUORwAQ492LqaV_tyvFQA@mail.gmail.com","subject":"Re: [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-27T17:23:15Z","receivedAt":"2011-10-27T17:23:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n>> When read_directory(\"where/ever\") is called, what kind of paths does it\n>> collect? Do the paths the function collects share \"where/ever\" as their\n>> common prefix? I thought it collects the paths relative to whatever\n>> top-level directory given to the function, so that \"where/ever\" could be\n>> anything.\n>\n> Correct. But read_directory() takes pathspec now so naturally it does\n> not treat \"where/ever\" a common prefix anymore. So it has to open(\".\")\n> and starts from there.\n\nThat is a puzzling statement. The read_directory() function takes:\n\n - dir: use this struct to pass traversal status and collected paths;\n\n - path, len: this is the directory (not a pathspec) we start traversal\n from; and\n\n - pathspec: these are the patterns that specify which parts of the\n directory hierarchy under are traversed.\n\nI do not see any good reason for to become a match pattern. Are\nyou trying to get it prepended to elements in pathspec[] and match the path\ncollected including the part?\n\nWhy?\n\nI could see that \"open . and start from there, treating as if \nis also pathspec\" could be made to work, but I do not see why that is\ndesirable.\n\nIn other words, are there existing callers that abuse read_directory()\nto feed a pattern in ? Maybe they should be the one that needs\nfixing instead?\n"},{"id":"178363","messageId":"7vobx2z60w.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"CACsJy8C2nUJkN5=E7p2u_wjHqWy7EC_BS3Sr4+_QgunWHDdtKg@mail.gmail.com","subject":"Re: [PATCH/WIP 03/11] t5403: avoid doing \"git add foo/bar\" where foo/.git exists","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-27T17:41:19Z","receivedAt":"2011-10-27T17:41:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> On Thu, Oct 27, 2011 at 4:26 AM, Junio C Hamano wrote:\n> ...\n>> Where did you get this idea that submodule is somehow involved in this test?\n>\n> Because \"clone2\" looks like a submodule (it has \".git\" inside with valid HEAD)\n\nBut there is a crucial difference between \"it looks like\" and \"it is\". If\nit is not a submodule, and the established behaviour is not to treat it\nlike a submodule, then we shouldn't suddenly change our behaviour to start\ntreating as one.\n\n> ... Should we stick with one way only?\n\nWhatever we have been doing should not change, especially in corner cases.\n"},{"id":"178369","messageId":"7vfwiez4s5.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"1319438176-7304-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH/WIP 01/11] Introduce \"check-attr --excluded\" as a replacement for \"add --ignore-missing\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-27T18:08:10Z","receivedAt":"2011-10-27T18:08:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> --ignore-missing is used by submodule to check if a path may be\n> ignored by .gitignore files. It does not really fit in git-add (git\n> add takes pathspec, but --ignore-missing takes only paths)\n>\n> Google reckons that --ignore-missing is not used anywhere but\n> git-submodule.sh. Remove --ignore-missing and introduce \"check-attr\n> --excluded\" as a replacement.\n\nHmm. \"add --ignore-missing\" somehow does not fit very well with other uses\nof the option of the same name. In all other contexts, \"ignore-missing\"\nmeans just that: ignore the fact that whatever _thing_ we were made to\nexpect to exist by the instruction from the user does not exist, which\nusually results in an error or a report. \"add --ignore-missing\" does not\nseem to be that (for one thing, it requires --dry-run).\n\nIt is unclear to me what is supposed to do to after reading the three-line\ndocumentation in the manpage X-<.\n\nSo I am perfectly OK with the removal in the current form.\n\nBut I do not think \"is this path ignored with the .gitignore rules\" check\nbelongs to check-attr, either.\n\nThe pattern of having .scmignore files to list ignored paths were forced\nupon us by historical version control systems, in the name of \"the users\nexpect it\". If there weren't such constraints, it would have been far\nnicer---we could have just said \"if you want to ignore paths, just use the\nattributes mechanism and give them the 'ignored' attribute\" without having\nto have the exclude mechanism.\n\nBut we do not live in that ideal world.\n\nPerhaps ls-files is a more suitable home for the feature?\n"},{"id":"178372","messageId":"7vbot2z3gf.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"1319438176-7304-6-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH/WIP 05/11] symbolize return values of tree_entry_interesting()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-27T18:36:48Z","receivedAt":"2011-10-27T18:36:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> This helps extending the value later on for \"interesting, but cannot\n> decide if the entry truely matches yet\" (ie. prefix matches)\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n\nGood change; it is a basic code hygiene to avoid magic constants anyway.\n\n> diff --git a/tree-diff.c b/tree-diff.c\n> index 6782484..25cc981 100644\n> --- a/tree-diff.c\n> +++ b/tree-diff.c\n> @@ -114,12 +114,13 @@ static void show_entry(struct diff_options *opt, const char *prefix,\n> }\n> \n> static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,\n> -\t\t\t struct diff_options *opt, int *match)\n> +\t\t\t struct diff_options *opt,\n> +\t\t\t enum interesting *match)\n> {\n> \twhile (t->size) {\n> \t\t*match = tree_entry_interesting(&t->entry, base, 0, &opt->pathspec);\n> \t\tif (*match) {\n> -\t\t\tif (*match < 0)\n> +\t\t\tif (*match == all_entries_not_interesting)\n> \t\t\t\tt->size = 0;\n> \t\t\tbreak;\n> \t\t}\n\nThe caller of this function needs to be updated as well.\n\nBut I have to wonder why this skip_uninteresting() does not peek the\noriginal value of *match and skip, which is the loop structure the other\ncaller of tree_entry_interesting() in this file has.\n\ndiff --git a/tree-diff.c b/tree-diff.c\nindex 25cc981..de4ba28 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -133,7 +133,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,\n {\n \tstruct strbuf base;\n \tint baselen = strlen(base_str);\n-\tint t1_match = 0, t2_match = 0;\n+\tenum interesting t1_match, t2_match;\n \n \t/* Enable recursion indefinitely */\n \topt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);\n@@ -142,6 +142,9 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,\n \tstrbuf_init(&base, PATH_MAX);\n \tstrbuf_add(&base, base_str, baselen);\n \n+\t/* Initialize to something other than all_entries_not_interesting */\n+\tt1_match = t2_match = entry_not_interesting;\n+\n \tfor (;;) {\n \t\tif (diff_can_quit_early(opt))\n \t\t\tbreak;\n"},{"id":"178490","messageId":"CACsJy8B8Zd092JsBqtuVA-zR-o_uPH2m73wJa0uqgkokE5qnNw@mail.gmail.com","threadId":"28755","inReplyTo":"7vzkgmz6v0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-28T20:47:44Z","receivedAt":"2011-10-28T20:47:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Oct 28, 2011 at 4:23 AM, Junio C Hamano wrote:\n> Nguyen Thai Ngoc Duy writes:\n>\n>>> When read_directory(\"where/ever\") is called, what kind of paths does it\n>>> collect? Do the paths the function collects share \"where/ever\" as their\n>>> common prefix? I thought it collects the paths relative to whatever\n>>> top-level directory given to the function, so that \"where/ever\" could be\n>>> anything.\n>>\n>> Correct. But read_directory() takes pathspec now so naturally it does\n>> not treat \"where/ever\" a common prefix anymore.  So it has to open(\".\")\n>> and starts from there.\n>\n> That is a puzzling statement. The read_directory() function takes:\n>\n>  - dir: use this struct to pass traversal status and collected paths;\n>\n>  - path, len: this is the directory (not a pathspec) we start traversal\n>   from; and\n>\n>  - pathspec: these are the patterns that specify which parts of the\n>   directory hierarchy under are traversed.\n>\n> I do not see any good reason for to become a match pattern. Are\n> you trying to get it prepended to elements in pathspec[] and match the path\n> collected including the part?\n>\n> Why?\n>\n> I could see that \"open . and start from there, treating as if \n> is also pathspec\" could be made to work, but I do not see why that is\n> desirable.\n>\n> In other words, are there existing callers that abuse read_directory()\n> to feed a pattern in ? Maybe they should be the one that needs\n> fixing instead?\n\nfill_directory() tries to calculate a common prefix (i.e. \nto read_directory()) from pathspec and that may or may not work when\npathspec magic comes into play. But yes, I could just make\nfill_directory() pass <\"\",0> to read_directory() and keep \nin read_directory() for notes-merge and future users.\n-- \nDuy\n"},{"id":"178491","messageId":"CACsJy8BS_XgNWhWG+dsnMYLGUYTd_MODdhonbLAXtJw2Fe--Zw@mail.gmail.com","threadId":"28755","inReplyTo":"7vfwiez4s5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/WIP 01/11] Introduce \"check-attr --excluded\" as a replacement for \"add --ignore-missing\"","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-28T20:51:15Z","receivedAt":"2011-10-28T20:51:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/10/28 Junio C Hamano :\n> Perhaps ls-files is a more suitable home for the feature?\n\nls-files sounds good. It does all kinds of file selection already.\nI'll see if I can add -I (aka \"show ignored files only) to it.\n-- \nDuy\n"},{"id":"178518","messageId":"CACsJy8DdQXXoYT2gB2L5z6pdCNU_vL2w7c8eJvKRGX2T9iAC3Q@mail.gmail.com","threadId":"28755","inReplyTo":"7vobx2z60w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/WIP 03/11] t5403: avoid doing \"git add foo/bar\" where foo/.git exists","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-30T05:55:22Z","receivedAt":"2011-10-30T05:55:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Oct 28, 2011 at 12:41 AM, Junio C Hamano wrote:\n>> ... Should we stick with one way only?\n>\n> Whatever we have been doing should not change, especially in corner cases.\n\nI disagree. If it's not right, then we should change it even though it\nmay face unpleasant consequences from misusing it. And I don't think\nit's sane to behave like what we're doing now:\n\n$ GIT_DIR=clone2/.git git ls-files --stage\n100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 1\n\n$ ls -l clone2/2 3\n-rw-r--r-- 1 pclouds users 0 Th10 30 12:40 3\n-rw-r--r-- 1 pclouds users 0 Th10 30 12:40 clone2/2\n\n$ GIT_DIR=clone2/.git git add clone2/2 3\n\n$ GIT_DIR=clone2/.git git ls-files --stage\n100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 1\n100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 3\n\n$ GIT_DIR=clone2/.git git add clone2/2\n\n$ GIT_DIR=clone2/.git git ls-files --stage\n100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 1\n100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 3\n100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 clone2/2\n\n\"git add\" behaves inconsistently when \"clone2/2\" and \"3\" are given and\nwhen clone2/2 is given alone. This is just bad to me.\n\nNote that this has nothing to do with read_directory() discussion we\nhad in the notes-merge patch, I agree we should keep the prefix. This\nis about the calculating common prefix automatically from pathspec.\nBut prefix and pathspec are treated differently by read_directory().\nIn \"git add clone2/2 3\", common prefix is \"\" while in \"git add\nclone2/2\" common prefix is \"clone2\".\n-- \nDuy\n"},{"id":"178521","messageId":"7vaa8jrm6a.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"CACsJy8DdQXXoYT2gB2L5z6pdCNU_vL2w7c8eJvKRGX2T9iAC3Q@mail.gmail.com","subject":"Re: [PATCH/WIP 03/11] t5403: avoid doing \"git add foo/bar\" where foo/.git exists","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-30T07:08:45Z","receivedAt":"2011-10-30T07:08:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> Note that this has nothing to do with read_directory() discussion we\n> had in the notes-merge patch...\n\nI think we are in agreement on that point.\n\nGoing back to your example...\n\n> $ GIT_DIR=clone2/.git git add clone2/2 3\n>\n> $ GIT_DIR=clone2/.git git ls-files --stage\n> 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 1\n> 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 3\n\nYou probably found a bug here. It is simply wrong to choose not to add\nclone2/2, especially without telling the caller anything.\n\n\tSide note. I just did this and I am not getting what you saw above.\n\n $ mkdir -p /var/tmp/j/y && cd /var/tmp/j/y\n $ git init; git init clone2\n $ : >3; : >clone2/2\n $ GIT_DIR=clone2/.git git add clone2/2 3\n $ GIT_DIR=clone2/.git git ls-files\n 3\n\tclone2/2\n\n\tThe behavour is different when clone2/.git already has commit, and\n whatever codepath that gives these two different behaviour needs\n to be fixed.\n\nBy the way, I think I know where you are coming from.\n\nIf we think clone2/ and everything underneath belongs to a repository that\nis _not_ governed by our GIT_DIR (which usually is .git), it may be nicer\nwhen the user attempts to add clone2/2 (which would normally belong to\nclone2/.git) to at least warn about it, or even error out. I would not be\nentirely opposed to a change in the behaviour if the above example were\ndone without GIT_DIR and produced an error, like this:\n\n $ git add clone2/2 3; echo $?\n error: clone2/2 is outside our repository, possibly governed by clone2/.git\n 1\n $ git ls-files\n 1\n\nAfter all, if clone2 were a submodule of our repository, we do notice and\nerror out an attempt to add clone2/2 to our repository, so if we changed\nthe way how \"git add\" behaves to do the above, I can buy an argument that\ncalls it a bugfix.\n\nWhen GIT_DIR=clone2/.git is given, however, the caller explicitly declines\nthe repository discovery. We do not know how the repository we are dealing\nwith (which we were explicitly told with $GIT_DIR) and a directory whose\nname is \".git\" under \"clone2\" we happened to find in read_directory()\nrelates to each other, especially when our index does not have clone2 as\nour submodule.\n\nWe however *do* know that our working tree is our current directory, so\nit would be wrong to do this:\n\n $ GIT_DIR=clone2/.git git add clone2/2 3; echo $?\n error: 3 is outside our repository, possibly goverened by .git\n 1\n\nThe command should just add clone2/2 and 3 as it was told to.\n"},{"id":"178526","messageId":"CACsJy8BshQT=iZRHXLaaVaohYRX5tYvCvVZSPPVbVuMX2gSW9w@mail.gmail.com","threadId":"28755","inReplyTo":"7vbot2z3gf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/WIP 05/11] symbolize return values of tree_entry_interesting()","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-30T09:17:43Z","receivedAt":"2011-10-30T09:17:43Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/10/28 Junio C Hamano :\n>>  static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,\n>> -                            struct diff_options *opt, int *match)\n>> +                            struct diff_options *opt,\n>> +                            enum interesting *match)\n>>  {\n>>       while (t->size) {\n>>               *match = tree_entry_interesting(&t->entry, base, 0, &opt->pathspec);\n>>               if (*match) {\n>> -                     if (*match < 0)\n>> +                     if (*match == all_entries_not_interesting)\n>>                               t->size = 0;\n>>                       break;\n>>               }\n>\n> The caller of this function needs to be updated as well.\n\nYeah, thanks.\n\n> But I have to wonder why this skip_uninteresting() does not peek the\n> original value of *match and skip, which is the loop structure the other\n> caller of tree_entry_interesting() in this file has.\n\nProbably because no one asked that question before. I think it makes\nsense for skip_uninteresting() to skip t_e_i() when *match == -1 or 2.\nThanks.\n-- \nDuy\n"},{"id":"178528","messageId":"CACsJy8Ae1MPYzjoouZoFCU6Ltr9UznukfuTrJb=OUJYr9VTYSg@mail.gmail.com","threadId":"28755","inReplyTo":"7vaa8jrm6a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/WIP 03/11] t5403: avoid doing \"git add foo/bar\" where foo/.git exists","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-30T09:55:54Z","receivedAt":"2011-10-30T09:55:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Oct 30, 2011 at 2:08 PM, Junio C Hamano wrote:\n>        Side note. I just did this and I am not getting what you saw above.\n>\n>        $ mkdir -p /var/tmp/j/y && cd /var/tmp/j/y\n>        $ git init; git init clone2\n>        $ : >3; : >clone2/2\n>        $ GIT_DIR=clone2/.git git add clone2/2 3\n>        $ GIT_DIR=clone2/.git git ls-files\n>        3\n>        clone2/2\n>\n>        The behavour is different when clone2/.git already has commit, and\n>        whatever codepath that gives these two different behaviour needs\n>        to be fixed.\n\nIt's resolve_gitlink_ref() in treat_directory(). I think replacing\nthat call() with is_git_directory() would fix this problem. We may\nwant to do the same with remove_dir_recursively().\n\n> When GIT_DIR=clone2/.git is given, however, the caller explicitly declines\n> the repository discovery. We do not know how the repository we are dealing\n> with (which we were explicitly told with $GIT_DIR) and a directory whose\n> name is \".git\" under \"clone2\" we happened to find in read_directory()\n> relates to each other, especially when our index does not have clone2 as\n> our submodule.\n>\n> We however *do* know that our working tree is our current directory, so\n> it would be wrong to do this:\n>\n>    $ GIT_DIR=clone2/.git git add clone2/2 3; echo $?\n>    error: 3 is outside our repository, possibly goverened by .git\n>    1\n>\n> The command should just add clone2/2 and 3 as it was told to.\n\nI am concerned about clone2/2 in this case, not 3. I guess we can\ncheck if clone2/.git is the repo we are using. If it is, skip it.\n-- \nDuy\n"},{"id":"178545","messageId":"7vvcr6qbye.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"CACsJy8Ae1MPYzjoouZoFCU6Ltr9UznukfuTrJb=OUJYr9VTYSg@mail.gmail.com","subject":"Re: [PATCH/WIP 03/11] t5403: avoid doing \"git add foo/bar\" where foo/.git exists","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-30T23:47:05Z","receivedAt":"2011-10-30T23:47:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n>> We however *do* know that our working tree is our current directory, so\n>> it would be wrong to do this:\n>>\n>> $ GIT_DIR=clone2/.git git add clone2/2 3; echo $?\n>> error: 3 is outside our repository, possibly goverened by .git\n>> 1\n>>\n>> The command should just add clone2/2 and 3 as it was told to.\n>\n> I am concerned about clone2/2 in this case, not 3. ...\n\nHmm... If that is the case, I am afraid that I failed to convey my point\nin the previous message.\n\nYou are concerned about clone2/2 because you think GIT_DIR=clone2/.git\nsomehow implies that clone2/2 is a file at the toplevel of some repository\nthat should appear at \"2\" not at \"clone2/2\" in the index, no?\n\nIf that is the case, it means you are somehow getting the notion that\nGIT_WORK_TREE is set to clone2 even though in the example we are\ndiscussing it is _not_. Which in turn would mean \"3\" that is outside of\nthat directory should not even get into the picture.\n\nIn other words, the wish to register clone2/2 at \"2\" in the index by\nstripping clone2/ and the wish to reject \"3\" as outside because it is\nimpossible to strip clone2/ from it are the same thing. You either should\nbe worrying about _both_ paths, or neither of them.\n\nAnd I am saying that you should be worried about neither of them in this\ncase. GIT_DIR= without GIT_WORK_TREE set has treated the\ncurrent directory as the top of the working tree from the beginning of\ntime, and both clone2/2 and 3 _should_ appear in the index in this\nexample, which is $GIT_DIR/index, which happens to be at a confusing\nlocation that is clone2/.git/index.\n\n> ... I guess we can\n> check if clone2/.git is the repo we are using. If it is, skip it.\n"},{"id":"186706","messageId":"1331563647-1909-1-git-send-email-johan@herland.net","threadId":"28755","inReplyTo":"1319438176-7304-3-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/2] t3310: Add testcase demonstrating failure to --commit from within another dir","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2012-03-12T14:47:26Z","receivedAt":"2012-03-12T14:47:26Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"Found-by: David Bremner \nSigned-off-by: Johan Herland \n---\n\nThis is a transcription of David's test script into a git test case.\n\nThanks to David for finding this issue.\n\n\nHave fun! :)\n\n...Johan\n\n t/t3310-notes-merge-manual-resolve.sh | 19 +++++++++++++++++++\n 1 file changed, 19 insertions(+)\n\ndiff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh\nindex 4367197..0c531c3 100755\n--- a/t/t3310-notes-merge-manual-resolve.sh\n+++ b/t/t3310-notes-merge-manual-resolve.sh\n@@ -553,4 +553,23 @@ test_expect_success 'resolve situation by aborting the notes merge' '\n \tverify_notes z\n '\n\n+cat >expect_notes < $(git rev-parse HEAD) &&\n+\t\techo \"bar\" >> $(git rev-parse HEAD) &&\n+\t\tgit notes merge --commit\n+\t) &&\n+\tgit notes show HEAD > actual_notes &&\n+\ttest_cmp expect_notes actual_notes\n+'\n+\n test_done\n--\n1.7.9.2\n"},{"id":"186707","messageId":"1331563647-1909-2-git-send-email-johan@herland.net","threadId":"28755","inReplyTo":"1331563647-1909-1-git-send-email-johan@herland.net","subject":"[PATCH 2/2] notes-merge: use opendir/readdir instead of using read_directory()","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2012-03-12T14:47:27Z","receivedAt":"2012-03-12T14:47:27Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"notes_merge_commit() only needs to list all entries (non-recursively)\nunder a directory, which can be easily accomplished with\nopendir/readdir and would be more lightweight than read_directory().\n\nread_directory() is designed to list paths inside a working\ndirectory. Using it outside of its scope may lead to undesired effects.\n\nApparently, one of the undesired effects of read_directory() is that it\ndoesn't deal with being given absolute paths. This creates problems for\nnotes_merge_commit() when git_path() returns an absolute path, which\nhappens when the current working directory is in a subdirectory of the\n.git directory.\n\nOriginally-by: Nguyễn Thái Ngọc Duy \nUpdated-by: Johan Herland \nSigned-off-by: Johan Herland \n---\n\nThis is a resurrection of pclouds' patch 2/11 in a patch series sent\nlast October for rewriting read_directory(). This patch doesn't\nactually touch read_directory(), but instead rewrites\nnotes_merge_commit() to use opendir()/readdir() instead of\nread_directory(). Since the usage of read_directory() is what caused\nthe bug that David found (in the previous patch), this rewrite happens\nto fix that bug as well.\n\n\nHave fun! :)\n\n...Johan\n\n notes-merge.c | 50 ++++++++++++++++++++-------------\n t/t3310-notes-merge-manual-resolve.sh | 2 +-\n 2 files changed, 31 insertions(+), 21 deletions(-)\n\ndiff --git a/notes-merge.c b/notes-merge.c\nindex fb0832f..3a16af2 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -687,51 +687,60 @@ int notes_merge_commit(struct notes_merge_options *o,\n {\n \t/*\n \t * Iterate through files in .git/NOTES_MERGE_WORKTREE and add all\n-\t * found notes to 'partial_tree'. Write the updates notes tree to\n+\t * found notes to 'partial_tree'. Write the updated notes tree to\n \t * the DB, and commit the resulting tree object while reusing the\n \t * commit message and parents from 'partial_commit'.\n \t * Finally store the new commit object SHA1 into 'result_sha1'.\n \t */\n-\tstruct dir_struct dir;\n-\tchar *path = xstrdup(git_path(NOTES_MERGE_WORKTREE \"/\"));\n-\tint path_len = strlen(path), i;\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tstruct strbuf path = STRBUF_INIT;\n \tchar *msg = strstr(partial_commit->buffer, \"\\n\\n\");\n \tstruct strbuf sb_msg = STRBUF_INIT;\n+\tint baselen;\n\n+\tstrbuf_addstr(&path, git_path(NOTES_MERGE_WORKTREE));\n \tif (o->verbosity >= 3)\n-\t\tprintf(\"Committing notes in notes merge worktree at %.*s\\n\",\n-\t\t\tpath_len - 1, path);\n+\t\tprintf(\"Committing notes in notes merge worktree at %s\\n\",\n+\t\t\tpath.buf);\n\n \tif (!msg || msg[2] == '\\0')\n \t\tdie(\"partial notes commit has empty message\");\n \tmsg += 2;\n\n-\tmemset(&dir, 0, sizeof(dir));\n-\tread_directory(&dir, path, path_len, NULL);\n-\tfor (i = 0; i < dir.nr; i++) {\n-\t\tstruct dir_entry *ent = dir.entries[i];\n+\tdir = opendir(path.buf);\n+\tif (!dir)\n+\t\tdie_errno(\"could not open %s\", path.buf);\n+\n+\tstrbuf_addch(&path, '/');\n+\tbaselen = path.len;\n+\twhile ((e = readdir(dir)) != NULL) {\n \t\tstruct stat st;\n-\t\tconst char *relpath = ent->name + path_len;\n \t\tunsigned char obj_sha1[20], blob_sha1[20];\n\n-\t\tif (ent->len - path_len != 40 || get_sha1_hex(relpath, obj_sha1)) {\n+\t\tif (is_dot_or_dotdot(e->d_name))\n+\t\t\tcontinue;\n+\n+\t\tif (strlen(e->d_name) != 40 || get_sha1_hex(e->d_name, obj_sha1)) {\n \t\t\tif (o->verbosity >= 3)\n-\t\t\t\tprintf(\"Skipping non-SHA1 entry '%s'\\n\",\n-\t\t\t\t\t\t\t\tent->name);\n+\t\t\t\tprintf(\"Skipping non-SHA1 entry '%s%s'\\n\",\n+\t\t\t\t\tpath.buf, e->d_name);\n \t\t\tcontinue;\n \t\t}\n\n+\t\tstrbuf_addstr(&path, e->d_name);\n \t\t/* write file as blob, and add to partial_tree */\n-\t\tif (stat(ent->name, &st))\n-\t\t\tdie_errno(\"Failed to stat '%s'\", ent->name);\n-\t\tif (index_path(blob_sha1, ent->name, &st, HASH_WRITE_OBJECT))\n-\t\t\tdie(\"Failed to write blob object from '%s'\", ent->name);\n+\t\tif (stat(path.buf, &st))\n+\t\t\tdie_errno(\"Failed to stat '%s'\", path.buf);\n+\t\tif (index_path(blob_sha1, path.buf, &st, HASH_WRITE_OBJECT))\n+\t\t\tdie(\"Failed to write blob object from '%s'\", path.buf);\n \t\tif (add_note(partial_tree, obj_sha1, blob_sha1, NULL))\n \t\t\tdie(\"Failed to add resolved note '%s' to notes tree\",\n-\t\t\t ent->name);\n+\t\t\t path.buf);\n \t\tif (o->verbosity >= 4)\n \t\t\tprintf(\"Added resolved note for object %s: %s\\n\",\n \t\t\t\tsha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1));\n+\t\tstrbuf_setlen(&path, baselen);\n \t}\n\n \tstrbuf_attach(&sb_msg, msg, strlen(msg), strlen(msg) + 1);\n@@ -740,7 +749,8 @@ int notes_merge_commit(struct notes_merge_options *o,\n \tif (o->verbosity >= 4)\n \t\tprintf(\"Finalized notes merge commit: %s\\n\",\n \t\t\tsha1_to_hex(result_sha1));\n-\tfree(path);\n+\tstrbuf_release(&path);\n+\tclosedir(dir);\n \treturn 0;\n }\n\ndiff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh\nindex 0c531c3..d6d6ac6 100755\n--- a/t/t3310-notes-merge-manual-resolve.sh\n+++ b/t/t3310-notes-merge-manual-resolve.sh\n@@ -558,7 +558,7 @@ foo\n bar\n EOF\n\n-test_expect_failure 'switch cwd before committing notes merge' '\n+test_expect_success 'switch cwd before committing notes merge' '\n \tgit notes add -m foo HEAD &&\n \tgit notes --ref=other add -m bar HEAD &&\n \ttest_must_fail git notes merge refs/notes/other &&\n--\n1.7.9.2\n"},{"id":"186708","messageId":"CACsJy8A=u_U8qdVcWCBeksC-TvF_JC-=D5Zu6qco5XR7-vef2A@mail.gmail.com","threadId":"28755","inReplyTo":"1331563647-1909-2-git-send-email-johan@herland.net","subject":"Re: [PATCH 2/2] notes-merge: use opendir/readdir instead of using read_directory()","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-03-12T14:53:28Z","receivedAt":"2012-03-12T14:53:28Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Mar 12, 2012 at 9:47 PM, Johan Herland wrote:\n> This is a resurrection of pclouds' patch 2/11 in a patch series sent\n> last October for rewriting read_directory(). This patch doesn't\n> actually touch read_directory(), but instead rewrites\n> notes_merge_commit() to use opendir()/readdir() instead of\n> read_directory(). Since the usage of read_directory() is what caused\n> the bug that David found (in the previous patch), this rewrite happens\n> to fix that bug as well.\n\nHappy to help. And that reminds me I've got to revive the\nread_directory() rewrite soon. Got stuck at git-add because I aimed\ntoo high. Gaah...\n-- \nDuy\n"},{"id":"186921","messageId":"4F60593A.5070106@viscovery.net","threadId":"28755","inReplyTo":"1331563647-1909-2-git-send-email-johan@herland.net","subject":"[PATCH jh/notes-merge-in-git-dir-worktree] fixup! t3310 on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-03-14T08:39:22Z","receivedAt":"2012-03-14T08:39:22Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt \n\nOn Windows, a directory cannot be removed while it is the working\ndirectory of a process. \"git notes merge --commit\" attempts to remove\n.git/NOTES_MERGE_WORKTREE, but during the test the directory was still\n\"occupied\" by the shell. Move the command out of the subshell to release\nthe directory.\n\nSigned-off-by: Johannes Sixt \n---\n Feel free to squash this into 1/2.\n\n t/t3310-notes-merge-manual-resolve.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh\nindex d6d6ac6..6351877 100755\n--- a/t/t3310-notes-merge-manual-resolve.sh\n+++ b/t/t3310-notes-merge-manual-resolve.sh\n@@ -565,9 +565,9 @@ test_expect_success 'switch cwd before committing notes merge' '\n \t(\n \t\tcd .git/NOTES_MERGE_WORKTREE &&\n \t\techo \"foo\" > $(git rev-parse HEAD) &&\n-\t\techo \"bar\" >> $(git rev-parse HEAD) &&\n-\t\tgit notes merge --commit\n+\t\techo \"bar\" >> $(git rev-parse HEAD)\n \t) &&\n+\tgit notes merge --commit &&\n \tgit notes show HEAD > actual_notes &&\n \ttest_cmp expect_notes actual_notes\n '\n-- \n1.7.10.rc0.1198.g85187\n"},{"id":"186931","messageId":"CALKQrgdjYvkSBn8UORSsZecSVyhJbfU5tjU0hPJOYn1OMVxMyw@mail.gmail.com","threadId":"28755","inReplyTo":"4F60593A.5070106@viscovery.net","subject":"Re: [PATCH jh/notes-merge-in-git-dir-worktree] fixup! t3310 on Windows","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2012-03-14T11:39:35Z","receivedAt":"2012-03-14T11:39:35Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Mar 14, 2012 at 09:39, Johannes Sixt wrote:\n> From: Johannes Sixt \n>\n> On Windows, a directory cannot be removed while it is the working\n> directory of a process. \"git notes merge --commit\" attempts to remove\n> .git/NOTES_MERGE_WORKTREE, but during the test the directory was still\n> \"occupied\" by the shell. Move the command out of the subshell to release\n> the directory.\n>\n> Signed-off-by: Johannes Sixt \n> ---\n>  Feel free to squash this into 1/2.\n>\n>  t/t3310-notes-merge-manual-resolve.sh |    4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh\n> index d6d6ac6..6351877 100755\n> --- a/t/t3310-notes-merge-manual-resolve.sh\n> +++ b/t/t3310-notes-merge-manual-resolve.sh\n> @@ -565,9 +565,9 @@ test_expect_success 'switch cwd before committing notes merge' '\n>        (\n>                cd .git/NOTES_MERGE_WORKTREE &&\n>                echo \"foo\" > $(git rev-parse HEAD) &&\n> -               echo \"bar\" >> $(git rev-parse HEAD) &&\n> -               git notes merge --commit\n> +               echo \"bar\" >> $(git rev-parse HEAD)\n>        ) &&\n> +       git notes merge --commit &&\n\nNAK. This defeats the entire purpose of this test. The bug that we're\ntrying to solve is exactly the situation where the user has changed\ninto the .git/NOTES_MERGE_WORKTREE directory, and invokes 'git notes\nmerge --commit' from within. We need to find a different solution for\nthis on Windows. Maybe we should just abort 'git notes merge\n--commit/--abort' if the current directory is within\n.git/NOTES_MERGE_WORKTREE (and we're on Windows)?\n\n\n...Johan\n\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"186932","messageId":"4F60882E.90303@viscovery.net","threadId":"28755","inReplyTo":"CALKQrgdjYvkSBn8UORSsZecSVyhJbfU5tjU0hPJOYn1OMVxMyw@mail.gmail.com","subject":"Re: [PATCH jh/notes-merge-in-git-dir-worktree] fixup! t3310 on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-03-14T11:59:42Z","receivedAt":"2012-03-14T11:59:42Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/14/2012 12:39, schrieb Johan Herland:\n> On Wed, Mar 14, 2012 at 09:39, Johannes Sixt wrote:\n>> From: Johannes Sixt \n>>\n>> On Windows, a directory cannot be removed while it is the working\n>> directory of a process. \"git notes merge --commit\" attempts to remove\n>> .git/NOTES_MERGE_WORKTREE, but during the test the directory was still\n>> \"occupied\" by the shell. Move the command out of the subshell to release\n>> the directory.\n>>\n>> Signed-off-by: Johannes Sixt \n>> ---\n>> Feel free to squash this into 1/2.\n>>\n>> t/t3310-notes-merge-manual-resolve.sh | 4 ++--\n>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh\n>> index d6d6ac6..6351877 100755\n>> --- a/t/t3310-notes-merge-manual-resolve.sh\n>> +++ b/t/t3310-notes-merge-manual-resolve.sh\n>> @@ -565,9 +565,9 @@ test_expect_success 'switch cwd before committing notes merge' '\n>> (\n>> cd .git/NOTES_MERGE_WORKTREE &&\n>> echo \"foo\" > $(git rev-parse HEAD) &&\n>> - echo \"bar\" >> $(git rev-parse HEAD) &&\n>> - git notes merge --commit\n>> + echo \"bar\" >> $(git rev-parse HEAD)\n>> ) &&\n>> + git notes merge --commit &&\n> \n> NAK. This defeats the entire purpose of this test. The bug that we're\n> trying to solve is exactly the situation where the user has changed\n> into the .git/NOTES_MERGE_WORKTREE directory, and invokes 'git notes\n> merge --commit' from within. We need to find a different solution for\n> this on Windows. Maybe we should just abort 'git notes merge\n> --commit/--abort' if the current directory is within\n> .git/NOTES_MERGE_WORKTREE (and we're on Windows)?\n\nIsn't this an indication that something *VERY* wrong is happening? How do\nyou explain to POSIX people that you have just pulled the rug unter their\nfeet?\n\n$ git notes merge --commit\n$ git notes\nfatal: Unable to read current working directory: No such file or directory\n\nI doubt that the use-case that is tested here makes sense.\n\nOr .git/NOTES_MERGE_WORKTREE should not be removed. Would it be an option\nto clear it out only when it is needed, right before it is filled again?\n\n-- Hannes\n"},{"id":"186935","messageId":"87fwdbnzlw.fsf@zancas.localnet","threadId":"28755","inReplyTo":"4F60882E.90303@viscovery.net","subject":"Re: [PATCH jh/notes-merge-in-git-dir-worktree] fixup! t3310 on Windows","fromName":"David Bremner","fromEmail":"david@tethera.net","sentAt":"2012-03-14T12:20:11Z","receivedAt":"2012-03-14T12:20:11Z","isPatch":true,"sender":{"key":"david@tethera.net","avatar":null},"body":"\n> I doubt that the use-case that is tested here makes sense.\n\nTo me, the obvious workflow to resolve notes conflicts is is chdir into\nthis directory, edit files, and then call git notes merge --commit. The\n(implicit) requirement that one cannot call commit from within the\ndirectory you edited files was quite surprising to me. So in my opinion\nthe use case does make sense, which is why I submitted the bug in the\nfirst place.\n\nIn any case, it should not fail silently, whether or not one is required\nto chdir back to the worktree before calling git notes merge --commit.\n\nd\n"},{"id":"186937","messageId":"CALKQrgdWZM959OyrEp+WCCehczZmMA3K8_RAcf23aAczKBCfvA@mail.gmail.com","threadId":"28755","inReplyTo":"4F60882E.90303@viscovery.net","subject":"Re: [PATCH jh/notes-merge-in-git-dir-worktree] fixup! t3310 on Windows","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2012-03-14T12:56:44Z","receivedAt":"2012-03-14T12:56:44Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Mar 14, 2012 at 12:59, Johannes Sixt wrote:\n> Am 3/14/2012 12:39, schrieb Johan Herland:\n>> On Wed, Mar 14, 2012 at 09:39, Johannes Sixt wrote:\n>>> From: Johannes Sixt \n>>>\n>>> On Windows, a directory cannot be removed while it is the working\n>>> directory of a process. \"git notes merge --commit\" attempts to remove\n>>> .git/NOTES_MERGE_WORKTREE, but during the test the directory was still\n>>> \"occupied\" by the shell. Move the command out of the subshell to release\n>>> the directory.\n>>>\n>>> Signed-off-by: Johannes Sixt \n>>> ---\n>>>  Feel free to squash this into 1/2.\n>>>\n>>>  t/t3310-notes-merge-manual-resolve.sh |    4 ++--\n>>>  1 file changed, 2 insertions(+), 2 deletions(-)\n>>>\n>>> diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh\n>>> index d6d6ac6..6351877 100755\n>>> --- a/t/t3310-notes-merge-manual-resolve.sh\n>>> +++ b/t/t3310-notes-merge-manual-resolve.sh\n>>> @@ -565,9 +565,9 @@ test_expect_success 'switch cwd before committing notes merge' '\n>>>        (\n>>>                cd .git/NOTES_MERGE_WORKTREE &&\n>>>                echo \"foo\" > $(git rev-parse HEAD) &&\n>>> -               echo \"bar\" >> $(git rev-parse HEAD) &&\n>>> -               git notes merge --commit\n>>> +               echo \"bar\" >> $(git rev-parse HEAD)\n>>>        ) &&\n>>> +       git notes merge --commit &&\n>>\n>> NAK. This defeats the entire purpose of this test. The bug that we're\n>> trying to solve is exactly the situation where the user has changed\n>> into the .git/NOTES_MERGE_WORKTREE directory, and invokes 'git notes\n>> merge --commit' from within. We need to find a different solution for\n>> this on Windows. Maybe we should just abort 'git notes merge\n>> --commit/--abort' if the current directory is within\n>> .git/NOTES_MERGE_WORKTREE (and we're on Windows)?\n>\n> Isn't this an indication that something *VERY* wrong is happening? How do\n> you explain to POSIX people that you have just pulled the rug unter their\n> feet?\n>\n> $ git notes merge --commit\n> $ git notes\n> fatal: Unable to read current working directory: No such file or directory\n\nTrue.\n\n> I doubt that the use-case that is tested here makes sense.\n\nAs David wrote, the use case is likely to pop up among regular users.\nWe can't simply ignore it.\n\n> Or .git/NOTES_MERGE_WORKTREE should not be removed. Would it be an option\n> to clear it out only when it is needed, right before it is filled again?\n\nMaybe, but then we wouldn't be able to warn or abort in the case where\nthere is a previous unfinished notes merge, and the user tries to\nstart a new notes merge. Instead, we'd silently overwrite the previous\nunfinished notes merge...\n\nMaybe it's better to simply detect if cwd is inside\n.git/NOTES_MERGE_WORKTREE, and then abort, telling the user to chdir\nout before trying again?\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"186969","messageId":"7vlin3qdpt.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"CALKQrgdWZM959OyrEp+WCCehczZmMA3K8_RAcf23aAczKBCfvA@mail.gmail.com","subject":"Re: [PATCH jh/notes-merge-in-git-dir-worktree] fixup! t3310 on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-14T17:44:46Z","receivedAt":"2012-03-14T17:44:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n> On Wed, Mar 14, 2012 at 12:59, Johannes Sixt wrote:\n> ...\n>> I doubt that the use-case that is tested here makes sense.\n>\n> As David wrote, the use case is likely to pop up among regular users.\n> We can't simply ignore it.\n>\n>> Or .git/NOTES_MERGE_WORKTREE should not be removed. Would it be an option\n>> to clear it out only when it is needed, right before it is filled again?\n>\n> Maybe, but then we wouldn't be able to warn or abort in the case where\n> there is a previous unfinished notes merge, and the user tries to\n> start a new notes merge. Instead, we'd silently overwrite the previous\n> unfinished notes merge...\n\nWe cannot simply ignore it.\n\nYou _could_ do is perhaps to do something like:\n\n - when you notice that you need to ask the user to hand-merge a temporary\n file (i.e. when 'notes merge' is run), check if .git/N_M_W/done exists.\n\n - If you see that marker file, remove it everything in the\n directory, deposit the temporary file you want the user to edit,\n and expect the user to later tell you that he is done by \"notes\n merge --commit\";\n\n - If you don't, the user didn't give you \"notes merge --commit\"\n during the earlier run. Tell him to first abandon the previous\n round with \"notes merge --abandon\" or something.\n\n - \"notes merge --commit\" will just mark .git/N_M_W/ with \"done\".\n\n - \"notes merge --abandon\" will remove .git/N_M_W/.\n\nbut it only supports the simplest \"Run 'notes merge', cd to .git/N_M_W,\nedit all, run 'notes merge --commit'\" workflow. If the user decides to\nabandon instead of commit, he will be inside the problematic directory, so\ntrying to be more elaborate like the above does not really solve anything\nfundamental. The user has to come out of that temporary directory\neventually anyway.\n\nSo I think the following is not just an improvement over the current code,\nbut probably an acceptable solution, given its simplicity.\n\n> Maybe it's better to simply detect if cwd is inside\n> .git/NOTES_MERGE_WORKTREE, and then abort, telling the user to chdir\n> out before trying again?\n"},{"id":"187001","messageId":"1331769333-13890-1-git-send-email-johan@herland.net","threadId":"28755","inReplyTo":"7vlin3qdpt.fsf@alter.siamese.dyndns.org","subject":"[PATCH 3/2] notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2012-03-14T23:55:33Z","receivedAt":"2012-03-14T23:55:33Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"When a manual notes merge is committed or aborted, we need to remove the\ntemporary worktree at .git/NOTES_MERGE_WORKTREE. However, removing the\nentire directory is not good if the user ran the 'git notes merge\n--commit/--abort' from within that directory. On Windows, the directory\nremoval would simply fail, while on POSIX systems, users would suddenly\nfind themselves in an invalid current directory.\n\nTherefore, instead of deleting the entire directory, we delete everything\n_within_ the directory, and leave the (empty) directory in place.\n\nThis would cause a subsequent notes merge to abort, complaining about a\nprevious - unfinished - notes merge (due to the presence of\n.git/NOTES_MERGE_WORKTREE), so we also need to adjust this check to only\ntrigger when .git/NOTES_MERGE_WORKTREE is non-empty.\n\nFinally, adjust the t3310 manual notes merge testcases to correctly handle\nthe existence of an empty .git/NOTES_MERGE_WORKTREE directory.\n\nInspired-by: Junio C Hamano \nSigned-off-by: Johan Herland \n---\n\nHow about this solution? I believe it should solve all the cases.\n\nI'm torn about the new remove_everything_inside_dir(). Obviously it's a\ncopy-paste-modify of dir.c:remove_dir_recursively(), and could instead be\nimplemented by adding an extra flag to remove_dir_recursively(). However,\nadding a \"#define REMOVE_DIR_CONTENTS_BUT_NOT_DIR_ITSELF 04\" seemed even\nuglier to me...\n\nWhat do you think?\n\n\n...Johan\n\n\n notes-merge.c | 52 ++++++++++++++++++++++++++++++---\n t/t3310-notes-merge-manual-resolve.sh | 8 ++---\n 2 files changed, 52 insertions(+), 8 deletions(-)\n\ndiff --git a/notes-merge.c b/notes-merge.c\nindex 3a16af2..bf080fb 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -267,7 +267,8 @@ static void check_notes_merge_worktree(struct notes_merge_options *o)\n \t\t * Must establish NOTES_MERGE_WORKTREE.\n \t\t * Abort if NOTES_MERGE_WORKTREE already exists\n \t\t */\n-\t\tif (file_exists(git_path(NOTES_MERGE_WORKTREE))) {\n+\t\tif (file_exists(git_path(NOTES_MERGE_WORKTREE)) &&\n+\t\t !is_empty_dir(git_path(NOTES_MERGE_WORKTREE))) {\n \t\t\tif (advice_resolve_conflict)\n \t\t\t\tdie(\"You have not concluded your previous \"\n \t\t\t\t \"notes merge (%s exists).\\nPlease, use \"\n@@ -754,16 +755,59 @@ int notes_merge_commit(struct notes_merge_options *o,\n \treturn 0;\n }\n \n+/* Based on dir.c:remove_dir_recursively() */\n+static int remove_everything_inside_dir(struct strbuf *path)\n+{\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tint ret = 0, original_len = path->len, len;\n+\n+\tdir = opendir(path->buf);\n+\tif (!dir)\n+\t\treturn -1;\n+\tif (path->buf[original_len - 1] != '/')\n+\t\tstrbuf_addch(path, '/');\n+\n+\tlen = path->len;\n+\twhile ((e = readdir(dir)) != NULL) {\n+\t\tstruct stat st;\n+\t\tif (is_dot_or_dotdot(e->d_name))\n+\t\t\tcontinue;\n+\n+\t\tstrbuf_setlen(path, len);\n+\t\tstrbuf_addstr(path, e->d_name);\n+\t\tif (lstat(path->buf, &st))\n+\t\t\t; /* fall thru */\n+\t\telse if (S_ISDIR(st.st_mode)) {\n+\t\t\tif (!remove_dir_recursively(path, 0))\n+\t\t\t\tcontinue; /* happy */\n+\t\t} else if (!unlink(path->buf))\n+\t\t\tcontinue; /* happy, too */\n+\n+\t\t/* path too long, stat fails, or non-directory still exists */\n+\t\tret = -1;\n+\t\tbreak;\n+\t}\n+\tclosedir(dir);\n+\n+\tstrbuf_setlen(path, original_len);\n+\treturn ret;\n+}\n+\n int notes_merge_abort(struct notes_merge_options *o)\n {\n-\t/* Remove .git/NOTES_MERGE_WORKTREE directory and all files within */\n+\t/*\n+\t * Remove all files within .git/NOTES_MERGE_WORKTREE. We do not remove\n+\t * the .git/NOTES_MERGE_WORKTREE directory itself, since it might be\n+\t * the current working directory of the user.\n+\t */\n \tstruct strbuf buf = STRBUF_INIT;\n \tint ret;\n \n \tstrbuf_addstr(&buf, git_path(NOTES_MERGE_WORKTREE));\n \tif (o->verbosity >= 3)\n-\t\tprintf(\"Removing notes merge worktree at %s\\n\", buf.buf);\n-\tret = remove_dir_recursively(&buf, 0);\n+\t\tprintf(\"Removing notes merge worktree at %s/*\\n\", buf.buf);\n+\tret = remove_everything_inside_dir(&buf);\n \tstrbuf_release(&buf);\n \treturn ret;\n }\ndiff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh\nindex d6d6ac6..195bb97 100755\n--- a/t/t3310-notes-merge-manual-resolve.sh\n+++ b/t/t3310-notes-merge-manual-resolve.sh\n@@ -324,7 +324,7 @@ y and z notes on 4th commit\n EOF\n \tgit notes merge --commit &&\n \t# No .git/NOTES_MERGE_* files left\n-\ttest_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n+\ttest_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# Merge commit has pre-merge y and pre-merge z as parents\n \ttest \"$(git rev-parse refs/notes/m^1)\" = \"$(cat pre_merge_y)\" &&\n@@ -386,7 +386,7 @@ test_expect_success 'redo merge of z into m (== y) with default (\"manual\") resol\n test_expect_success 'abort notes merge' '\n \tgit notes merge --abort &&\n \t# No .git/NOTES_MERGE_* files left\n-\ttest_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n+\ttest_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# m has not moved (still == y)\n \ttest \"$(git rev-parse refs/notes/m)\" = \"$(cat pre_merge_y)\" &&\n@@ -453,7 +453,7 @@ EOF\n \t# Finalize merge\n \tgit notes merge --commit &&\n \t# No .git/NOTES_MERGE_* files left\n-\ttest_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n+\ttest_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# Merge commit has pre-merge y and pre-merge z as parents\n \ttest \"$(git rev-parse refs/notes/m^1)\" = \"$(cat pre_merge_y)\" &&\n@@ -542,7 +542,7 @@ EOF\n test_expect_success 'resolve situation by aborting the notes merge' '\n \tgit notes merge --abort &&\n \t# No .git/NOTES_MERGE_* files left\n-\ttest_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n+\ttest_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# m has not moved (still == w)\n \ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\" &&\n-- \n1.7.10.rc0.43.g35011\n"},{"id":"187020","messageId":"7vipi6l52w.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"1331769333-13890-1-git-send-email-johan@herland.net","subject":"Re: [PATCH 3/2] notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-15T07:02:31Z","receivedAt":"2012-03-15T07:02:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n> I'm torn about the new remove_everything_inside_dir(). Obviously it's a\n> copy-paste-modify of dir.c:remove_dir_recursively(), and could instead be\n> implemented by adding an extra flag to remove_dir_recursively(). However,\n> adding a \"#define REMOVE_DIR_CONTENTS_BUT_NOT_DIR_ITSELF 04\" seemed even\n> uglier to me...\n\nHmm, what ugliness am I missing when viewing the attached patch? It looks\nsimple and straightforward enough, at least to me.\n\n dir.c | 14 ++++++++++----\n dir.h | 1 +\n 2 files changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 0a78d00..6432728 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1178,6 +1178,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n \tstruct dirent *e;\n \tint ret = 0, original_len = path->len, len;\n \tint only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);\n+\tint keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);\n \tunsigned char submodule_head[20];\n \n \tif ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&\n@@ -1185,9 +1186,14 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n \t\t/* Do not descend and nuke a nested git work tree. */\n \t\treturn 0;\n \n+\tflag &= ~REMOVE_DIR_KEEP_TOPLEVEL;\n \tdir = opendir(path->buf);\n-\tif (!dir)\n-\t\treturn rmdir(path->buf);\n+\tif (!dir) {\n+\t\tif (!keep_toplevel)\n+\t\t\treturn rmdir(path->buf);\n+\t\telse\n+\t\t\treturn -1;\n+\t}\n \tif (path->buf[original_len - 1] != '/')\n \t\tstrbuf_addch(path, '/');\n \n@@ -1202,7 +1208,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n \t\tif (lstat(path->buf, &st))\n \t\t\t; /* fall thru */\n \t\telse if (S_ISDIR(st.st_mode)) {\n-\t\t\tif (!remove_dir_recursively(path, only_empty))\n+\t\t\tif (!remove_dir_recursively(path, flag))\n \t\t\t\tcontinue; /* happy */\n \t\t} else if (!only_empty && !unlink(path->buf))\n \t\t\tcontinue; /* happy, too */\n@@ -1214,7 +1220,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n \tclosedir(dir);\n \n \tstrbuf_setlen(path, original_len);\n-\tif (!ret)\n+\tif (!ret && !keep_toplevel)\n \t\tret = rmdir(path->buf);\n \treturn ret;\n }\ndiff --git a/dir.h b/dir.h\nindex dd6947e..58b6fc7 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -102,6 +102,7 @@ extern void setup_standard_excludes(struct dir_struct *dir);\n \n #define REMOVE_DIR_EMPTY_ONLY 01\n #define REMOVE_DIR_KEEP_NESTED_GIT 02\n+#define REMOVE_DIR_KEEP_TOPLEVEL 04\n extern int remove_dir_recursively(struct strbuf *path, int flag);\n \n /* tries to remove the path with empty directories along it, ignores ENOENT */\n-- \n1.7.10.rc1.22.g07e85\n"},{"id":"187022","messageId":"7v7gyml4g7.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"7vipi6l52w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/2] notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-15T07:16:08Z","receivedAt":"2012-03-15T07:16:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Johan Herland writes:\n>\n>> I'm torn about the new remove_everything_inside_dir(). Obviously it's a\n>> copy-paste-modify of dir.c:remove_dir_recursively(), and could instead be\n>> implemented by adding an extra flag to remove_dir_recursively(). However,\n>> adding a \"#define REMOVE_DIR_CONTENTS_BUT_NOT_DIR_ITSELF 04\" seemed even\n>> uglier to me...\n>\n> Hmm, what ugliness am I missing when viewing the attached patch? It looks\n> simple and straightforward enough, at least to me.\n>\n> dir.c | 14 ++++++++++----\n> dir.h | 1 +\n> 2 files changed, 11 insertions(+), 4 deletions(-)\n>\n> diff --git a/dir.c b/dir.c\n> index 0a78d00..6432728 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -1178,6 +1178,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n> \tstruct dirent *e;\n> \tint ret = 0, original_len = path->len, len;\n> \tint only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);\n> +\tint keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);\n> \tunsigned char submodule_head[20];\n> \n> \tif ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&\n> @@ -1185,9 +1186,14 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n> \t\t/* Do not descend and nuke a nested git work tree. */\n> \t\treturn 0;\n> \n> +\tflag &= ~REMOVE_DIR_KEEP_TOPLEVEL;\n\nNit. This needs to drop REMOVE_DIR_KEEP_NESTED_GIT as well in order to\npreserve the current behaviour.\n\nI actually suspect that the passing of \"only_empty\" in the original may be\na bug in a0f4afb (clean: require double -f options to nuke nested git\nrepository and work tree, 2009-06-30), and this patch might be a fix to\nthe bug, but I didn't think things through, and it is getting late, so...\n\n> \tdir = opendir(path->buf);\n> -\tif (!dir)\n> -\t\treturn rmdir(path->buf);\n> +\tif (!dir) {\n> +\t\tif (!keep_toplevel)\n> +\t\t\treturn rmdir(path->buf);\n> +\t\telse\n> +\t\t\treturn -1;\n> +\t}\n> \tif (path->buf[original_len - 1] != '/')\n> \t\tstrbuf_addch(path, '/');\n> \n> @@ -1202,7 +1208,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n> \t\tif (lstat(path->buf, &st))\n> \t\t\t; /* fall thru */\n> \t\telse if (S_ISDIR(st.st_mode)) {\n> -\t\t\tif (!remove_dir_recursively(path, only_empty))\n> +\t\t\tif (!remove_dir_recursively(path, flag))\n> \t\t\t\tcontinue; /* happy */\n> \t\t} else if (!only_empty && !unlink(path->buf))\n> \t\t\tcontinue; /* happy, too */\n"},{"id":"187023","messageId":"CALKQrgfpM-y=9O=h33jxirVoOO8dHHO8tWCR9RatxTottpRXFA@mail.gmail.com","threadId":"28755","inReplyTo":"7v7gyml4g7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/2] notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2012-03-15T07:39:30Z","receivedAt":"2012-03-15T07:39:30Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Thu, Mar 15, 2012 at 08:16, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>> Johan Herland writes:\n>>> I'm torn about the new remove_everything_inside_dir(). Obviously it's a\n>>> copy-paste-modify of dir.c:remove_dir_recursively(), and could instead be\n>>> implemented by adding an extra flag to remove_dir_recursively(). However,\n>>> adding a \"#define REMOVE_DIR_CONTENTS_BUT_NOT_DIR_ITSELF 04\" seemed even\n>>> uglier to me...\n>>\n>> Hmm, what ugliness am I missing when viewing the attached patch?  It looks\n>> simple and straightforward enough, at least to me.\n\nAgreed, you found a much more palatable name than I did. The patch\nbelow looks good to me, and should become patch #3 in this series,\nwith my \"3/2\" patch being adjusted accordingly and becoming patch #4.\nDo you want me to send the whole series again, or is it easier for you\nto simply fix it up yourself?\n\n>>  dir.c |   14 ++++++++++----\n>>  dir.h |    1 +\n>>  2 files changed, 11 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/dir.c b/dir.c\n>> index 0a78d00..6432728 100644\n>> --- a/dir.c\n>> +++ b/dir.c\n>> @@ -1178,6 +1178,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n>>       struct dirent *e;\n>>       int ret = 0, original_len = path->len, len;\n>>       int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);\n>> +     int keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);\n>>       unsigned char submodule_head[20];\n>>\n>>       if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&\n>> @@ -1185,9 +1186,14 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n>>               /* Do not descend and nuke a nested git work tree. */\n>>               return 0;\n>>\n>> +     flag &= ~REMOVE_DIR_KEEP_TOPLEVEL;\n>\n> Nit. This needs to drop REMOVE_DIR_KEEP_NESTED_GIT as well in order to\n> preserve the current behaviour.\n>\n> I actually suspect that the passing of \"only_empty\" in the original may be\n> a bug in a0f4afb (clean: require double -f options to nuke nested git\n> repository and work tree, 2009-06-30), and this patch might be a fix to\n> the bug, but I didn't think things through, and it is getting late, so...\n\nI noticed the same while looking at this function, and I think your\nanalysis is correct. As it stands, REMOVE_DIR_KEEP_NESTED_GIT only\napplies to .git folders located directly in the toplevel dir, and not\ninside a subdirectory. That strikes me as odd given the name of the\nflag.\n\n\nHave fun! :)\n\n...Johan\n\n>>       dir = opendir(path->buf);\n>> -     if (!dir)\n>> -             return rmdir(path->buf);\n>> +     if (!dir) {\n>> +             if (!keep_toplevel)\n>> +                     return rmdir(path->buf);\n>> +             else\n>> +                     return -1;\n>> +     }\n>>       if (path->buf[original_len - 1] != '/')\n>>               strbuf_addch(path, '/');\n>>\n>> @@ -1202,7 +1208,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n>>               if (lstat(path->buf, &st))\n>>                       ; /* fall thru */\n>>               else if (S_ISDIR(st.st_mode)) {\n>> -                     if (!remove_dir_recursively(path, only_empty))\n>> +                     if (!remove_dir_recursively(path, flag))\n>>                               continue; /* happy */\n>>               } else if (!only_empty && !unlink(path->buf))\n>>                       continue; /* happy, too */\n\n\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"187026","messageId":"7vzkbijnnn.fsf_-_@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"CALKQrgfpM-y=9O=h33jxirVoOO8dHHO8tWCR9RatxTottpRXFA@mail.gmail.com","subject":"Re* [PATCH 3/2] notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-15T08:04:12Z","receivedAt":"2012-03-15T08:04:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n> On Thu, Mar 15, 2012 at 08:16, Junio C Hamano wrote:\n> ...\n>> I actually suspect that the passing of \"only_empty\" in the original may be\n>> a bug in a0f4afb (clean: require double -f options to nuke nested git\n>> repository and work tree, 2009-06-30), and this patch might be a fix to\n>> the bug, but I didn't think things through, and it is getting late, so...\n>\n> I noticed the same while looking at this function, and I think your\n> analysis is correct. As it stands, REMOVE_DIR_KEEP_NESTED_GIT only\n> applies to .git folders located directly in the toplevel dir, and not\n> inside a subdirectory. That strikes me as odd given the name of the\n> flag.\n\nThis ended up to be totally unrelated to what you wanted to achieve, but\nhere is a potential fix. I only tested this by running the usual tests and\nadditional tests in this patch; we know the coverage of \"git clean\" is\nspotty and its implementation is lower quality than others, so please take\nit with a large grain of salt.\n\nThe basic idea is to report from the lower level of recursion if it\ndecided to leave something in the directory back to the upper level,\nand have the upper level refrain from removing itself (and when this\nhappens, that upper level in turn reports that it kept the directory\nit is in charge of to its own caller), to avoid returning -1 from the\nremove_dir_recursively() to the caller when we deliberately not run\nrmdir() on a directory to prevent \"git clean\" from issuing a warning()\nor exiting with an error.\n\n-- >8 --\nSubject: clean: preserve nested git worktree in subdirectories\n\nremove_dir_recursively() had a check to avoid removing the directory it\nwas asked to remove without recursing into it and report success when the\ndirectory is the top level of a working tree of a nested git repository,\nto protext such a repository from \"clean -f\" (without double -f). If a\nworking tree of a nested git repository is in a subdirectory of a toplevel\nproject, however, this protection did not apply.\n\nPass REMOVE_DIR_KEEP_NESTED_GIT flag down to the recursive removal\ncodepath, and also teach the higher level not to remove the directory it\nis asked to remove, when the recursed invocation did not remove the\ndirectory it was asked to remove due to a nested git repository, as it is\nnot an error to leave the parent directories of such a nested repository.\n\nSigned-off-by: Junio C Hamano \n---\n dir.c | 21 ++++++++++++++++-----\n t/t7300-clean.sh | 27 ++++++++++++++++++++++-----\n 2 files changed, 38 insertions(+), 10 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 6432728..0e09556 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1172,23 +1172,27 @@ int is_empty_dir(const char *path)\n \treturn ret;\n }\n \n-int remove_dir_recursively(struct strbuf *path, int flag)\n+static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)\n {\n \tDIR *dir;\n \tstruct dirent *e;\n-\tint ret = 0, original_len = path->len, len;\n+\tint ret = 0, original_len = path->len, len, kept_down = 0;\n \tint only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);\n \tint keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);\n \tunsigned char submodule_head[20];\n \n \tif ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&\n-\t !resolve_gitlink_ref(path->buf, \"HEAD\", submodule_head))\n+\t !resolve_gitlink_ref(path->buf, \"HEAD\", submodule_head)) {\n \t\t/* Do not descend and nuke a nested git work tree. */\n+\t\tif (kept_up)\n+\t\t\t*kept_up = 1;\n \t\treturn 0;\n+\t}\n \n \tflag &= ~REMOVE_DIR_KEEP_TOPLEVEL;\n \tdir = opendir(path->buf);\n \tif (!dir) {\n+\t\t/* an empty dir could be removed even if it is unreadble */\n \t\tif (!keep_toplevel)\n \t\t\treturn rmdir(path->buf);\n \t\telse\n@@ -1208,7 +1212,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n \t\tif (lstat(path->buf, &st))\n \t\t\t; /* fall thru */\n \t\telse if (S_ISDIR(st.st_mode)) {\n-\t\t\tif (!remove_dir_recursively(path, flag))\n+\t\t\tif (!remove_dir_recurse(path, flag, &kept_down))\n \t\t\t\tcontinue; /* happy */\n \t\t} else if (!only_empty && !unlink(path->buf))\n \t\t\tcontinue; /* happy, too */\n@@ -1220,11 +1224,18 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n \tclosedir(dir);\n \n \tstrbuf_setlen(path, original_len);\n-\tif (!ret && !keep_toplevel)\n+\tif (!ret && !keep_toplevel && !kept_down)\n \t\tret = rmdir(path->buf);\n+\telse if (kept_up)\n+\t\t*kept_up = 1;\n \treturn ret;\n }\n \n+int remove_dir_recursively(struct strbuf *path, int flag)\n+{\n+\treturn remove_dir_recurse(path, flag, NULL);\n+}\n+\n void setup_standard_excludes(struct dir_struct *dir)\n {\n \tconst char *path;\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex 800b536..ccfb54d 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -399,8 +399,8 @@ test_expect_success SANITY 'removal failure' '\n '\n \n test_expect_success 'nested git work tree' '\n-\trm -fr foo bar &&\n-\tmkdir foo bar &&\n+\trm -fr foo bar baz &&\n+\tmkdir -p foo bar baz/boo &&\n \t(\n \t\tcd foo &&\n \t\tgit init &&\n@@ -412,15 +412,24 @@ test_expect_success 'nested git work tree' '\n \t\tcd bar &&\n \t\t>goodbye.people\n \t) &&\n+\t(\n+\t\tcd baz/boo &&\n+\t\tgit init &&\n+\t\t>deeper.world\n+\t\tgit add . &&\n+\t\tgit commit -a -m deeply.nested\n+\t) &&\n \tgit clean -f -d &&\n \ttest -f foo/.git/index &&\n \ttest -f foo/hello.world &&\n+\ttest -f baz/boo/.git/index &&\n+\ttest -f baz/boo/deeper.world &&\n \t! test -d bar\n '\n \n test_expect_success 'force removal of nested git work tree' '\n-\trm -fr foo bar &&\n-\tmkdir foo bar &&\n+\trm -fr foo bar baz &&\n+\tmkdir -p foo bar baz/boo &&\n \t(\n \t\tcd foo &&\n \t\tgit init &&\n@@ -432,9 +441,17 @@ test_expect_success 'force removal of nested git work tree' '\n \t\tcd bar &&\n \t\t>goodbye.people\n \t) &&\n+\t(\n+\t\tcd baz/boo &&\n+\t\tgit init &&\n+\t\t>deeper.world\n+\t\tgit add . &&\n+\t\tgit commit -a -m deeply.nested\n+\t) &&\n \tgit clean -f -f -d &&\n \t! test -d foo &&\n-\t! test -d bar\n+\t! test -d bar &&\n+\t! test -d baz\n '\n \n test_expect_success 'git clean -e' '\n"},{"id":"187028","messageId":"7vvcm6jnaj.fsf@alter.siamese.dyndns.org","threadId":"28755","inReplyTo":"CALKQrgfpM-y=9O=h33jxirVoOO8dHHO8tWCR9RatxTottpRXFA@mail.gmail.com","subject":"Re: [PATCH 3/2] notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-15T08:12:04Z","receivedAt":"2012-03-15T08:12:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n> ... The patch\n> below looks good to me, and should become patch #3 in this series,\n> with my \"3/2\" patch being adjusted accordingly and becoming patch #4.\n> Do you want me to send the whole series again, or is it easier for you\n> to simply fix it up yourself?\n\nI'd rather not to do this myself, as this alleged #3 in v2 was written\nmerely as a response to the \"refactoring dir.c:remove_dir_recursively is\nugly\", without reading other parts of the patch (i.e. the logic you use to\ndecide when to call this function with what arguments) at all.\n\nThanks.\n"},{"id":"187029","messageId":"4F61A47A.2050205@viscovery.net","threadId":"28755","inReplyTo":"1331769333-13890-1-git-send-email-johan@herland.net","subject":"Re: [PATCH 3/2] notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-03-15T08:12:42Z","receivedAt":"2012-03-15T08:12:42Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/15/2012 0:55, schrieb Johan Herland:\n> When a manual notes merge is committed or aborted, we need to remove the\n> temporary worktree at .git/NOTES_MERGE_WORKTREE. However, removing the\n> entire directory is not good if the user ran the 'git notes merge\n> --commit/--abort' from within that directory. On Windows, the directory\n> removal would simply fail, while on POSIX systems, users would suddenly\n> find themselves in an invalid current directory.\n> \n> Therefore, instead of deleting the entire directory, we delete everything\n> _within_ the directory, and leave the (empty) directory in place.\n\nJust a data point: With this patch, the test passes on Windows.\n\n-- Hannes\n"}]}