{"thread":{"id":"24763","subject":"git merge, .gitignore, and silently overwriting untracked files","startedAt":"2010-08-17T05:21:31Z","lastAt":"2010-10-15T18:47:24Z","messageCount":39,"participants":["Joshua Jensen","Junio C Hamano","Clemens Buchacher","Jakub Narebski","Matthieu Moy","Holger Hellmuth","Heiko Voigt","Johannes Sixt","Kevin Ballard","Jonathan Nieder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"148228","messageId":"4C6A1C5B.4030304@workspacewhiz.com","threadId":"24763","inReplyTo":null,"subject":"git merge, .gitignore, and silently overwriting untracked files","fromName":"Joshua Jensen","fromEmail":"jjensen@workspacewhiz.com","sentAt":"2010-08-17T05:21:31Z","receivedAt":"2010-08-17T05:21:31Z","isPatch":false,"sender":{"key":"jjensen@workspacewhiz.com","avatar":"https://avatars.githubusercontent.com/u/111687?v=4"},"body":"  Git silently overwrote an important untracked file for me today.  I \nhave come up with the following transcript to reproduce the issue.\n\nIn a nutshell, if an untracked file is in .gitignore, a merge from a \nbranch where the file was added will OVERWRITE the untracked file \nwithout warning.\n\nThis doesn't seem right.\n\nWhat do others think?\n\nJosh\n\n-------------------------------\n\n/s/testgitoverwrite\n$ mkdir aaa\n\n/s/testgitoverwrite\n$ cd aaa\n\n/s/testgitoverwrite/aaa\n$ echo abc > abc.txt\n\n/s/testgitoverwrite/aaa\n$ ls -l\ntotal 1\n-rw-r--r--    1 Joshua   Administ        4 Aug 16 23:10 abc.txt\n\n/s/testgitoverwrite/aaa\n$ git init\nInitialized empty Git repository in s:/testgitoverwrite/aaa/.git/\n\n/s/testgitoverwrite/aaa (master)\n$ git add abc.txt\n\n/s/testgitoverwrite/aaa (master)\n$ git commit -m \"Added abc.txt\"\n[master (root-commit) 0e8bffb] Added abc.txt\n  1 files changed, 1 insertions(+), 0 deletions(-)\n  create mode 100644 abc.txt\n\n/s/testgitoverwrite/aaa (master)\n$ cd ..\n\n/s/testgitoverwrite\n$ git clone aaa bbb\nCloning into bbb...\ndone.\n\n/s/testgitoverwrite\n$ cd bbb\n\n/s/testgitoverwrite/bbb (master)\n$ ls -l\ntotal 1\n-rw-r--r--    1 Joshua   Administ        4 Aug 16 23:10 abc.txt\n\n/s/testgitoverwrite/bbb (master)\n$ echo \"Important stuff\" > important.txt\n\n/s/testgitoverwrite/bbb (master)\n$ git add important.txt\n\n/s/testgitoverwrite/bbb (master)\n$ git commit -m \"This file is SO important\"\n[master 2fe9ab6] This file is SO important\n  1 files changed, 1 insertions(+), 0 deletions(-)\n  create mode 100644 important.txt\n\n/s/testgitoverwrite/bbb (master)\n$ cd ..\n\n/s/testgitoverwrite\n$ cd aaa\n\n/s/testgitoverwrite/aaa (master)\n$ echo \"Local testing copy of important.txt\" > important.txt\n\n/s/testgitoverwrite/aaa (master)\n$ git status\n# On branch master\n# Untracked files:\n#   (use \"git add <file>...\" to include in what will be committed)\n#\n#       important.txt\nnothing added to commit but untracked files present (use \"git add\" to track)\n\n/s/testgitoverwrite/aaa (master)\n$ git pull ../bbb\nremote: Counting objects: 4, done.\nremote: Compressing objects: 100% (2/2), done.\nremote: Total 3 (delta 0), reused 0 (delta 0)\nUnpacking objects: 100% (3/3), done.\n From ../bbb\n  * branch            HEAD       -> FETCH_HEAD\nUpdating 0e8bffb..2fe9ab6\nerror: Untracked working tree file 'important.txt' would be overwritten \nby merge.  Aborting\n\n/s/testgitoverwrite/aaa (master)\n$ git pull --rebase ../bbb\n From ../bbb\n  * branch            HEAD       -> FETCH_HEAD\nFirst, rewinding head to replay your work on top of it...\nerror: Untracked working tree file 'important.txt' would be overwritten \nby merge.\ncould not detach HEAD\n\n/s/testgitoverwrite/aaa (master)\n$ echo important.txt > .gitignore\n\n/s/testgitoverwrite/aaa (master)\n$ git status\n# On branch master\n# Untracked files:\n#   (use \"git add <file>...\" to include in what will be committed)\n#\n#       .gitignore\nnothing added to commit but untracked files present (use \"git add\" to track)\n\n/s/testgitoverwrite/aaa (master)\n$ cat important.txt\nLocal testing copy of important.txt\n\n/s/testgitoverwrite/aaa (master)\n$ git pull ../bbb\n From ../bbb\n  * branch            HEAD       -> FETCH_HEAD\nUpdating 0e8bffb..2fe9ab6\nFast-forward\n  important.txt |    1 +\n  1 files changed, 1 insertions(+), 0 deletions(-)\n  create mode 100644 important.txt\n\n/s/testgitoverwrite/aaa (master)\n$ cat important.txt\nImportant stuff\n"},{"id":"148303","messageId":"7viq39avay.fsf@alter.siamese.dyndns.org","threadId":"24763","inReplyTo":"4C6A1C5B.4030304@workspacewhiz.com","subject":"Re: git merge, .gitignore, and silently overwriting untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-17T19:33:25Z","receivedAt":"2010-08-17T19:33:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There are only two kinds of untracked paths in git.  Ones that git is\nallowed to remove in order to carry out operations asked by the end user,\ni.e. ones that are trashable, are registered to the gitignore mechanism,\nand the ones that are precious are not in gitignore; the latter can\ninterfere with git and prevent it from carrying out certain operations.\nE.g. an untracked and unignored file F will prevent you from switching to\na branch that has file F tracked, or that has file F/G (iow, F is a\ndirectory in that branch) tracked.\n\nAncient git didn't honor gitignore and considered no files trashable,\nwhich caused a lot of trouble to users.  It may be illuminating to check\ndiscussions in the list archive during the period the following commits\nwere made.  f8a9d42 (read-tree: further loosen \"working file will be lost\"\ncheck., 2006-12-04), ed93b44 (merge: loosen overcautious \"working file\nwill be lost\" check., 2006-10-08), c819353 (Fix switching to a branch with\nD/F when current branch has file D., 2007-03-15).\n\nWe could enhance the mechanism to categorize untracked paths into three\ninstead of two: trashable, unknown and precious.\n"},{"id":"148425","messageId":"20100818233900.GA27531@localhost","threadId":"24763","inReplyTo":"7viq39avay.fsf@alter.siamese.dyndns.org","subject":"[PATCH] optionally disable overwriting of ignored files","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-08-18T23:39:00Z","receivedAt":"2010-08-18T23:39:00Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"By default, checkout and fast-forward merge will overwrite ignored\nfiles. Make this behavior configurable.\n\nIf overwriting ignored files is disabled, it can still be enabled\non the command line by passing -i to merge or checkout.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n\nHi,\n\nOn Tue, Aug 17, 2010 at 12:33:25PM -0700, Junio C Hamano wrote:\n>\n> Ancient git didn't honor gitignore and considered no files trashable,\n> which caused a lot of trouble to users.  It may be illuminating to check\n> discussions in the list archive during the period the following commits\n> were made.  f8a9d42 (read-tree: further loosen \"working file will be lost\"\n> check., 2006-12-04), ed93b44 (merge: loosen overcautious \"working file\n> will be lost\" check., 2006-10-08), c819353 (Fix switching to a branch with\n> D/F when current branch has file D., 2007-03-15).\n\nI am not convinced this is something that most users want.\n\nThe fact that adding a file to .gitignore also marks it as\ntrashable to git is quite surprising to me. This is something that\nthe gitignore manpage should warn about loudly. Instead, I think\nthis behavior is completely undocumented.\n\nAs far as I can tell from the code, only fast-forward merge and\ncheckout overwrite ignored files by default. A regular merge will\ncomplain about untracked files that would be overwritten (ignored\nor not). I am quite familiar with that behavior and I think it's\nuseful to be reminded of those untracked files rather than having\nthem overwritten. I thought that by extension, other git commands\nwould behave in the same way.\n\nHere is a patch to optionally disable overwriting of ignored files.\nBut I think we should even disable it by default. \n\nClemens\n\n Documentation/config.txt |    6 ++++++\n builtin/checkout.c       |    7 +++++++\n builtin/merge.c          |    9 +++++++++\n cache.h                  |    1 +\n config.c                 |    3 +++\n environment.c            |    1 +\n 6 files changed, 27 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f81fb91..eb9bb43 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -450,6 +450,12 @@ core.excludesfile::\n \tto the value of `$HOME` and \"{tilde}user/\" to the specified user's\n \thome directory.  See linkgit:gitignore[5].\n \n+core.overwriteignored::\n+\tUntracked files can get in the way of merge or checkout. If\n+\toverwriteignored is enabled, such files will be overwritten\n+\tto let fast-forward merges and checkouts succeed. Enabled\n+\tby default.\n+\n core.editor::\n \tCommands such as `commit` and `tag` that lets you edit\n \tmessages by launching an editor uses the value of this\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 5affb6f..121a6a3 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -389,6 +389,11 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\ttopts.gently = opts->merge && old->commit;\n \t\ttopts.verbose_update = !opts->quiet;\n \t\ttopts.fn = twoway_merge;\n+\t\tif (overwrite_ignored) {\n+\t\t\ttopts.dir = xcalloc(1, sizeof(*topts.dir));\n+\t\t\ttopts.dir->flags |= DIR_SHOW_IGNORED;\n+\t\t\ttopts.dir->exclude_per_dir = \".gitignore\";\n+\t\t}\n \t\ttree = parse_tree_indirect(old->commit ?\n \t\t\t\t\t   old->commit->object.sha1 :\n \t\t\t\t\t   (unsigned char *)EMPTY_TREE_SHA1_BIN);\n@@ -664,6 +669,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tOPT_SET_INT('3', \"theirs\", &opts.writeout_stage, \"stage\",\n \t\t\t    3),\n \t\tOPT_BOOLEAN('f', \"force\", &opts.force, \"force\"),\n+\t\tOPT_BOOLEAN('i', \"overwrite-ignored\", &overwrite_ignored,\n+\t\t  \"allow explicitly ignored files to be overwritten\"),\n \t\tOPT_BOOLEAN('m', \"merge\", &opts.merge, \"merge\"),\n \t\tOPT_STRING(0, \"conflict\", &conflict_style, \"style\",\n \t\t\t   \"conflict style (merge or diff3)\"),\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 4d31e09..47b12ba 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -185,6 +185,8 @@ static struct option builtin_merge_options[] = {\n \t\t\"allow fast-forward (default)\"),\n \tOPT_BOOLEAN(0, \"ff-only\", &fast_forward_only,\n \t\t\"abort if fast-forward is not possible\"),\n+\tOPT_BOOLEAN('i', \"overwrite-ignored\", &overwrite_ignored,\n+\t  \"allow explicitly ignored files to be overwritten\"),\n \tOPT_RERERE_AUTOUPDATE(&allow_rerere_auto),\n \tOPT_CALLBACK('s', \"strategy\", &use_strategies, \"strategy\",\n \t\t\"merge strategy to use\", option_parse_strategy),\n@@ -682,6 +684,7 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote\n \tstruct unpack_trees_options opts;\n \tstruct tree_desc t[MAX_UNPACK_TREES];\n \tint i, fd, nr_trees = 0;\n+\tstruct dir_struct dir;\n \tstruct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));\n \n \trefresh_cache(REFRESH_QUIET);\n@@ -691,6 +694,12 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote\n \tmemset(&trees, 0, sizeof(trees));\n \tmemset(&opts, 0, sizeof(opts));\n \tmemset(&t, 0, sizeof(t));\n+\tmemset(&dir, 0, sizeof(dir));\n+\tif (overwrite_ignored) {\n+\t\tdir.flags |= DIR_SHOW_IGNORED;\n+\t\tdir.exclude_per_dir = \".gitignore\";\n+\t\topts.dir = &dir;\n+\t}\n \n \topts.head_idx = 1;\n \topts.src_index = &the_index;\ndiff --git a/cache.h b/cache.h\nindex c9fa3df..dd1b8f7 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -548,6 +548,7 @@ extern size_t packed_git_window_size;\n extern size_t packed_git_limit;\n extern size_t delta_base_cache_limit;\n extern int read_replace_refs;\n+extern int overwrite_ignored;\n extern int fsync_object_files;\n extern int core_preload_index;\n extern int core_apply_sparse_checkout;\ndiff --git a/config.c b/config.c\nindex cdcf583..bd7956e 100644\n--- a/config.c\n+++ b/config.c\n@@ -563,6 +563,9 @@ static int git_default_core_config(const char *var, const char *value)\n \tif (!strcmp(var, \"core.excludesfile\"))\n \t\treturn git_config_pathname(&excludes_file, var, value);\n \n+\tif (!strcmp(var, \"core.overwriteignored\"))\n+\t\toverwrite_ignored = git_config_bool(var, value);\n+\n \tif (!strcmp(var, \"core.whitespace\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\ndiff --git a/environment.c b/environment.c\nindex 83d38d3..1b92f60 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -30,6 +30,7 @@ const char *apply_default_ignorewhitespace;\n int zlib_compression_level = Z_BEST_SPEED;\n int core_compression_level;\n int core_compression_seen;\n+int overwrite_ignored = 1;\n int fsync_object_files;\n size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;\n size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;\n-- \n1.7.2.1.1.g202c\n\n"},{"id":"148442","messageId":"m3aaoiq41v.fsf@localhost.localdomain","threadId":"24763","inReplyTo":"20100818233900.GA27531@localhost","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-08-19T10:41:08Z","receivedAt":"2010-08-19T10:41:08Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n>  Documentation/config.txt |    6 ++++++\n>  builtin/checkout.c       |    7 +++++++\n>  builtin/merge.c          |    9 +++++++++\n>  cache.h                  |    1 +\n>  config.c                 |    3 +++\n>  environment.c            |    1 +\n>  6 files changed, 27 insertions(+), 0 deletions(-)\n\nYou forgot to update Documentation/git-checkout.txt and\nDocumentation/git-merge.txt with the new '-i' option.\n\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 5affb6f..121a6a3 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n\n> @@ -664,6 +669,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>  \t\tOPT_SET_INT('3', \"theirs\", &opts.writeout_stage, \"stage\",\n>  \t\t\t    3),\n>  \t\tOPT_BOOLEAN('f', \"force\", &opts.force, \"force\"),\n> +\t\tOPT_BOOLEAN('i', \"overwrite-ignored\", &overwrite_ignored,\n> +\t\t  \"allow explicitly ignored files to be overwritten\"),\n>  \t\tOPT_BOOLEAN('m', \"merge\", &opts.merge, \"merge\"),\n>  \t\tOPT_STRING(0, \"conflict\", &conflict_style, \"style\",\n>  \t\t\t   \"conflict style (merge or diff3)\"),\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 4d31e09..47b12ba 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -185,6 +185,8 @@ static struct option builtin_merge_options[] = {\n>  \t\t\"allow fast-forward (default)\"),\n>  \tOPT_BOOLEAN(0, \"ff-only\", &fast_forward_only,\n>  \t\t\"abort if fast-forward is not possible\"),\n> +\tOPT_BOOLEAN('i', \"overwrite-ignored\", &overwrite_ignored,\n> +\t  \"allow explicitly ignored files to be overwritten\"),\n>  \tOPT_RERERE_AUTOUPDATE(&allow_rerere_auto),\n>  \tOPT_CALLBACK('s', \"strategy\", &use_strategies, \"strategy\",\n>  \t\t\"merge strategy to use\", option_parse_strategy),\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"148593","messageId":"20100820184838.GA8861@localhost","threadId":"24763","inReplyTo":"m3aaoiq41v.fsf@localhost.localdomain","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-08-20T18:48:38Z","receivedAt":"2010-08-20T18:48:38Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Thu, Aug 19, 2010 at 03:41:08AM -0700, Jakub Narebski wrote:\n> \n> You forgot to update Documentation/git-checkout.txt and\n> Documentation/git-merge.txt with the new '-i' option.\n\nThanks. I am just waiting for some kind of feedback. I'm not going\nto write documentation for something that won't get accepted.\n\nClemens\n"},{"id":"148594","messageId":"4C6ED102.2040002@workspacewhiz.com","threadId":"24763","inReplyTo":"20100820184838.GA8861@localhost","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Joshua Jensen","fromEmail":"jjensen@workspacewhiz.com","sentAt":"2010-08-20T19:01:22Z","receivedAt":"2010-08-20T19:01:22Z","isPatch":true,"sender":{"key":"jjensen@workspacewhiz.com","avatar":"https://avatars.githubusercontent.com/u/111687?v=4"},"body":"  ----- Original Message -----\nFrom: Clemens Buchacher\nDate: 8/20/2010 12:48 PM\n> On Thu, Aug 19, 2010 at 03:41:08AM -0700, Jakub Narebski wrote:\n>> You forgot to update Documentation/git-checkout.txt and\n>> Documentation/git-merge.txt with the new '-i' option.\n> Thanks. I am just waiting for some kind of feedback. I'm not going\n> to write documentation for something that won't get accepted.\nI like the idea... :)\n\nMy vote doesn't count, though...\n\nJosh\n"},{"id":"148606","messageId":"7v4oepaup7.fsf@alter.siamese.dyndns.org","threadId":"24763","inReplyTo":"20100818233900.GA27531@localhost","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-20T20:35:32Z","receivedAt":"2010-08-20T20:35:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"If (and this may be a big IF) it is reasonable to add paths to .gitignore\nthat you do not want to lose, then you would want to have three classes of\nuntracked paths: \"precious but ignored\", \"trashable\" (and by definition\nignored), and \"unignored\" (and by definition is not ignored and is\nprecious).\n\nAs I already pointed out, we don't support the \"precious but ignored\"\nclass.  So an obvious alternate solution to your particular case is not to\nadd such a path to the gitignore mechanism.\n\nI have a suspicion that the approach this patch takes would not help very\nmuch in the real life.  You just traded the lack of \"precious but ignored\"\nwith \"no file is trashable, even if it is listed in .gitignore\".\n\nGranted, as long as it is not default, it won't negatively affect people\nwho do not enable it, other than that it may add maintenance burden on the\nresulting bloated code, but I find it hard to swallow new code that does\nnot convincingly solve the real problem.\n\nBy the way, we seem to have a few longstanding bugs that trashes an\nunignored and untracked (hence by definition precious) path during branch\nswitching, and does not give a correct escape hatch.\n\nDoing this:\n\n    $ git checkout maint\n    $ echo foo >t/t2018-checkout-branch.sh\n    $ git checkout master\n\ndoes correctly error out because the unignored file needs to be\noverwritten.  But doing this after the above still errors out, which is\nprobably wrong:\n\n    $ echo t/t2018-checkout-branch.sh >>.git/info/exclude\n    $ git checkout master\n\nAfter doing the above:\n\n    $ ed .git/info/exclude ;# remove the extra entry in info/excludes\n    $d\n    w\n    q\n    $ rm t/t2018-checkout-branch.sh\n    $ echo foo >po\n    $ git checkout pu\n\nshould error out, as \"po\" is a directory that has tracked contents, and we\nnever said the untracked regular file \"po\" is trashable, but the above\nsequence happily checks the branch out.\n"},{"id":"148607","messageId":"7vtymp9fn1.fsf@alter.siamese.dyndns.org","threadId":"24763","inReplyTo":"20100818233900.GA27531@localhost","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-20T20:46:10Z","receivedAt":"2010-08-20T20:46:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 5affb6f..121a6a3 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -389,6 +389,11 @@ static int merge_working_tree(struct checkout_opts *opts,\n>  \t\ttopts.gently = opts->merge && old->commit;\n>  \t\ttopts.verbose_update = !opts->quiet;\n>  \t\ttopts.fn = twoway_merge;\n> +\t\tif (overwrite_ignored) {\n> +\t\t\ttopts.dir = xcalloc(1, sizeof(*topts.dir));\n> +\t\t\ttopts.dir->flags |= DIR_SHOW_IGNORED;\n> +\t\t\ttopts.dir->exclude_per_dir = \".gitignore\";\n> +\t\t}\n>  \t\ttree = parse_tree_indirect(old->commit ?\n>  \t\t\t\t\t   old->commit->object.sha1 :\n>  \t\t\t\t\t   (unsigned char *)EMPTY_TREE_SHA1_BIN);\n\nWhat is this patch based on?  I don't see a branch that has these two\nlines adjacent to each other as the preimage...\n\n>  \t\ttopts.fn = twoway_merge;\n>  \t\ttree = parse_tree_indirect(old->commit ?\n"},{"id":"148644","messageId":"20100821064844.GA29968@localhost","threadId":"24763","inReplyTo":"7vtymp9fn1.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] optionally disable overwriting of ignored files","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-08-21T06:48:44Z","receivedAt":"2010-08-21T06:48:44Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n\nOn Fri, Aug 20, 2010 at 01:46:10PM -0700, Junio C Hamano wrote:\n>         \n> What is this patch based on?\n          \nOops, I forgot to squash an intermediate commit. This is based on\nv1.7.2.1.\n\n Documentation/config.txt |    6 ++++++\n builtin/checkout.c       |   10 +++++++---\n builtin/merge.c          |   10 +++++++---\n cache.h                  |    1 +\n config.c                 |    3 +++\n environment.c            |    1 +\n 6 files changed, 25 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f81fb91..eb9bb43 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -450,6 +450,12 @@ core.excludesfile::\n \tto the value of `$HOME` and \"{tilde}user/\" to the specified user's\n \thome directory.  See linkgit:gitignore[5].\n \n+core.overwriteignored::\n+\tUntracked files can get in the way of merge or checkout. If\n+\toverwriteignored is enabled, such files will be overwritten\n+\tto let fast-forward merges and checkouts succeed. Enabled\n+\tby default.\n+\n core.editor::\n \tCommands such as `commit` and `tag` that lets you edit\n \tmessages by launching an editor uses the value of this\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 1994be9..121a6a3 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -389,9 +389,11 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\ttopts.gently = opts->merge && old->commit;\n \t\ttopts.verbose_update = !opts->quiet;\n \t\ttopts.fn = twoway_merge;\n-\t\ttopts.dir = xcalloc(1, sizeof(*topts.dir));\n-\t\ttopts.dir->flags |= DIR_SHOW_IGNORED;\n-\t\ttopts.dir->exclude_per_dir = \".gitignore\";\n+\t\tif (overwrite_ignored) {\n+\t\t\ttopts.dir = xcalloc(1, sizeof(*topts.dir));\n+\t\t\ttopts.dir->flags |= DIR_SHOW_IGNORED;\n+\t\t\ttopts.dir->exclude_per_dir = \".gitignore\";\n+\t\t}\n \t\ttree = parse_tree_indirect(old->commit ?\n \t\t\t\t\t   old->commit->object.sha1 :\n \t\t\t\t\t   (unsigned char *)EMPTY_TREE_SHA1_BIN);\n@@ -667,6 +669,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tOPT_SET_INT('3', \"theirs\", &opts.writeout_stage, \"stage\",\n \t\t\t    3),\n \t\tOPT_BOOLEAN('f', \"force\", &opts.force, \"force\"),\n+\t\tOPT_BOOLEAN('i', \"overwrite-ignored\", &overwrite_ignored,\n+\t\t  \"allow explicitly ignored files to be overwritten\"),\n \t\tOPT_BOOLEAN('m', \"merge\", &opts.merge, \"merge\"),\n \t\tOPT_STRING(0, \"conflict\", &conflict_style, \"style\",\n \t\t\t   \"conflict style (merge or diff3)\"),\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 37ce4f5..47b12ba 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -185,6 +185,8 @@ static struct option builtin_merge_options[] = {\n \t\t\"allow fast-forward (default)\"),\n \tOPT_BOOLEAN(0, \"ff-only\", &fast_forward_only,\n \t\t\"abort if fast-forward is not possible\"),\n+\tOPT_BOOLEAN('i', \"overwrite-ignored\", &overwrite_ignored,\n+\t  \"allow explicitly ignored files to be overwritten\"),\n \tOPT_RERERE_AUTOUPDATE(&allow_rerere_auto),\n \tOPT_CALLBACK('s', \"strategy\", &use_strategies, \"strategy\",\n \t\t\"merge strategy to use\", option_parse_strategy),\n@@ -693,9 +695,11 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote\n \tmemset(&opts, 0, sizeof(opts));\n \tmemset(&t, 0, sizeof(t));\n \tmemset(&dir, 0, sizeof(dir));\n-\tdir.flags |= DIR_SHOW_IGNORED;\n-\tdir.exclude_per_dir = \".gitignore\";\n-\topts.dir = &dir;\n+\tif (overwrite_ignored) {\n+\t\tdir.flags |= DIR_SHOW_IGNORED;\n+\t\tdir.exclude_per_dir = \".gitignore\";\n+\t\topts.dir = &dir;\n+\t}\n \n \topts.head_idx = 1;\n \topts.src_index = &the_index;\ndiff --git a/cache.h b/cache.h\nindex c9fa3df..dd1b8f7 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -548,6 +548,7 @@ extern size_t packed_git_window_size;\n extern size_t packed_git_limit;\n extern size_t delta_base_cache_limit;\n extern int read_replace_refs;\n+extern int overwrite_ignored;\n extern int fsync_object_files;\n extern int core_preload_index;\n extern int core_apply_sparse_checkout;\ndiff --git a/config.c b/config.c\nindex cdcf583..bd7956e 100644\n--- a/config.c\n+++ b/config.c\n@@ -563,6 +563,9 @@ static int git_default_core_config(const char *var, const char *value)\n \tif (!strcmp(var, \"core.excludesfile\"))\n \t\treturn git_config_pathname(&excludes_file, var, value);\n \n+\tif (!strcmp(var, \"core.overwriteignored\"))\n+\t\toverwrite_ignored = git_config_bool(var, value);\n+\n \tif (!strcmp(var, \"core.whitespace\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\ndiff --git a/environment.c b/environment.c\nindex 83d38d3..1b92f60 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -30,6 +30,7 @@ const char *apply_default_ignorewhitespace;\n int zlib_compression_level = Z_BEST_SPEED;\n int core_compression_level;\n int core_compression_seen;\n+int overwrite_ignored = 1;\n int fsync_object_files;\n size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;\n size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;\n-- \n1.7.2.1.1.g202c\n\n"},{"id":"148650","messageId":"20100821080512.GA30141@localhost","threadId":"24763","inReplyTo":"7v4oepaup7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-08-21T08:05:12Z","receivedAt":"2010-08-21T08:05:12Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Fri, Aug 20, 2010 at 01:35:32PM -0700, Junio C Hamano wrote:\n>\n> If (and this may be a big IF) it is reasonable to add paths to .gitignore\n> that you do not want to lose, then you would want to have three classes of\n> untracked paths: \"precious but ignored\", \"trashable\" (and by definition\n> ignored), and \"unignored\" (and by definition is not ignored and is\n> precious).\n\nI am not sure anybody would bother categorizing their files into\nseveral classes. On the other hand, .gitignore and\n.git/info/exclude may already serve such a purpose.\n\nFiles that go into a tracked .gitignore are most likely generated\nfiles, and therefore trashable. Files that go into\n.git/info/exclude or into an untracked .gitignore (e.g. echo '*' >\nprecious-simulation-results/.gitignore), are not always generated\nand may not be trashable. At least they would not likely get in the\nway of checkout or merge.\n\n(As you noted below, except for the untracked .gitignore part, this\nis how git behaves already.)\n\n> As I already pointed out, we don't support the \"precious but ignored\"\n> class.  So an obvious alternate solution to your particular case is not to\n> add such a path to the gitignore mechanism.\n\nThat would defeat the point of ignoring it.\n\n> I have a suspicion that the approach this patch takes would not help very\n> much in the real life.  You just traded the lack of \"precious but ignored\"\n> with \"no file is trashable, even if it is listed in .gitignore\".\n> \n> Granted, as long as it is not default, it won't negatively affect people\n> who do not enable it, other than that it may add maintenance burden on the\n> resulting bloated code, but I find it hard to swallow new code that does\n> not convincingly solve the real problem.\n\nYes, I am not a big fan of this solution either. But if we do not\nfind a better solution, I think having it configurable cannot hurt.\nThe code required is minimal. As far as I am concerned, we can even\nremove the -i part.\n\n> By the way, we seem to have a few longstanding bugs that trashes an\n> unignored and untracked (hence by definition precious) path during branch\n> switching, and does not give a correct escape hatch.\n> \n> Doing this:\n> \n>     $ git checkout maint\n>     $ echo foo >t/t2018-checkout-branch.sh\n>     $ git checkout master\n> \n> does correctly error out because the unignored file needs to be\n> overwritten.  But doing this after the above still errors out, which is\n> probably wrong:\n> \n>     $ echo t/t2018-checkout-branch.sh >>.git/info/exclude\n>     $ git checkout master\n\nI understand your point. But this is actually a great example. I\nhave a bunch of such tests, which are not in shape for upstream,\nbut I want to keep them around anyways (and run them). Do you\nreally think that an untracked test which was added to\n.git/info/exclude should be considered trashable? If it were a\ngenerated file, it would have been added to .gitignore.\n\n> After doing the above:\n> \n>     $ ed .git/info/exclude ;# remove the extra entry in info/excludes\n>     $d\n>     w\n>     q\n>     $ rm t/t2018-checkout-branch.sh\n>     $ echo foo >po\n>     $ git checkout pu\n> \n> should error out, as \"po\" is a directory that has tracked contents, and we\n> never said the untracked regular file \"po\" is trashable, but the above\n> sequence happily checks the branch out.\n\nOk, that's bad.\n\nClemens\n"},{"id":"148667","messageId":"20100821132356.GA31998@localhost","threadId":"24763","inReplyTo":"7v4oepaup7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-08-21T13:23:56Z","receivedAt":"2010-08-21T13:23:56Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Fri, Aug 20, 2010 at 01:35:32PM -0700, Junio C Hamano wrote:\n>\n>     $ echo foo >po\n>     $ git checkout pu\n> \n> should error out, as \"po\" is a directory that has tracked contents, and we\n> never said the untracked regular file \"po\" is trashable, but the above\n> sequence happily checks the branch out.\n\nLooks like this case is simply overlooked in verify_absent_1(). The\nfollowing takes the existing lstat_cache() code and deals with the\nFL_ERR case, which is when there is a file in the way of the\nleading path.\n\nThe patch below fixes the issue and passes the test suite, but it's\nlacking in various ways and I am probably breaking something in\nlstat_cache(), which I do not completely understand yet.\n\nSo it's going to take some more work to fix this properly.\n\nClemens\n\n cache.h        |    1 +\n symlinks.c     |   20 ++++++++++++++++++--\n unpack-trees.c |   25 ++++++++++++++++++++++++-\n 3 files changed, 43 insertions(+), 3 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex dd1b8f7..5a8a99d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -850,6 +850,7 @@ struct cache_def {\n extern int has_symlink_leading_path(const char *name, int len);\n extern int threaded_has_symlink_leading_path(struct cache_def *, const char *, int);\n extern int has_symlink_or_noent_leading_path(const char *name, int len);\n+extern int find_leading_path(const char *name, int len, const char **path, int *path_len);\n extern int has_dirs_only_path(const char *name, int len, int prefix_len);\n extern void schedule_dir_for_removal(const char *name, int len);\n extern void remove_scheduled_dirs(void);\ndiff --git a/symlinks.c b/symlinks.c\nindex 8860120..3f78168 100644\n--- a/symlinks.c\n+++ b/symlinks.c\n@@ -152,7 +152,7 @@ static int lstat_cache(struct cache_def *cache, const char *name, int len,\n \t * path types, FL_NOENT, FL_SYMLINK and FL_DIR, can be cached\n \t * for the moment!\n \t */\n-\tsave_flags = ret_flags & track_flags & (FL_NOENT|FL_SYMLINK);\n+\tsave_flags = ret_flags & track_flags & (FL_NOENT|FL_SYMLINK|FL_ERR);\n \tif (save_flags && last_slash > 0 && last_slash <= PATH_MAX) {\n \t\tcache->path[last_slash] = '\\0';\n \t\tcache->len = last_slash;\n@@ -199,7 +199,7 @@ int has_symlink_leading_path(const char *name, int len)\n \n /*\n  * Return non-zero if path 'name' has a leading symlink component or\n- * if some leading path component does not exists.\n+ * if some leading path component does not exist.\n  */\n int has_symlink_or_noent_leading_path(const char *name, int len)\n {\n@@ -210,6 +210,22 @@ int has_symlink_or_noent_leading_path(const char *name, int len)\n }\n \n /*\n+ * Stat for leading path.\n+ */\n+int find_leading_path(const char *name, int len, const char **path, int *path_len)\n+{\n+\tstruct cache_def *cache = &default_cache;\t/* FIXME */\n+\tint flags = lstat_cache(cache, name, len,\n+\t\t\t   FL_SYMLINK|FL_NOENT|FL_DIR|FL_ERR, USE_ONLY_LSTAT);\n+\t*path = cache->path;\n+\t*path_len = cache->len;\n+\tif (flags & FL_ERR)\n+\t\treturn -1;\n+\telse\n+\t\treturn flags & (FL_SYMLINK|FL_NOENT);\n+}\n+\n+/*\n  * Return non-zero if all path components of 'name' exists as a\n  * directory.  If prefix_len > 0, we will test with the stat()\n  * function instead of the lstat() function for a prefix length of\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 8cf0da3..250ed7c 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1016,12 +1016,35 @@ static int verify_absent_1(struct cache_entry *ce, const char *action,\n \t\t\t\t const char *error_msg)\n {\n \tstruct stat st;\n+\tconst char *path;\n+\tint path_len;\n+\tint ret;\n \n \tif (o->index_only || o->reset || !o->update)\n \t\treturn 0;\n \n-\tif (has_symlink_or_noent_leading_path(ce->name, ce_namelen(ce)))\n+\tret = find_leading_path(ce->name, ce_namelen(ce), &path, &path_len);\n+\tif (ret > 0)\n \t\treturn 0;\n+\telse if (ret < 0) {\n+\t\tstruct cache_entry *result;\n+\n+\t\t/* FIXME: respect ignores etc. as below */\n+\n+\t\t/*\n+\t\t * The previous round may already have decided to\n+\t\t * delete this path, which is in a subdirectory that\n+\t\t * is being replaced with a blob.\n+\t\t */\n+\t\tresult = index_name_exists(&o->result, path, path_len, 0);\n+\t\tif (result) {\n+\t\t\tif (result->ce_flags & CE_REMOVE)\n+\t\t\t\treturn 0;\n+\t\t}\n+\n+\t\treturn o->gently ? -1 :\n+\t\t\terror(ERRORMSG(o, would_lose_untracked), path, action);\n+\t}\n \n \tif (!lstat(ce->name, &st)) {\n \t\tint dtype = ce_to_dtype(ce);\n-- \n1.7.2.1.1.g202c\n\n"},{"id":"148701","messageId":"7viq335ct0.fsf@alter.siamese.dyndns.org","threadId":"24763","inReplyTo":"20100821080512.GA30141@localhost","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-22T07:25:31Z","receivedAt":"2010-08-22T07:25:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> I understand your point. But this is actually a great example. I\n> have a bunch of such tests, which are not in shape for upstream,\n> but I want to keep them around anyways (and run them). Do you\n> really think that an untracked test which was added to\n> .git/info/exclude should be considered trashable? If it were a\n> generated file, it would have been added to .gitignore.\n\nI agree that in such a workflow to keep untracked tests around, they\nshould not be considered trashable. But more importantly, as I have\nalready said, adding your untracked tests to exclude is a wrong thing to\ndo.\n\nTraditionally (think \"git status\" output without \"-s\") the way to remind\noneself that some day these paths need to be added when they are ready has\nbeen to keep them untracked but _not_ ignored, so that they will be listed\nin the output.\n\nQuite contrary to what you earlier said in another message, adding\nsuch a path that is not trashable does defeat the point of the \"ignore\"\nmechanism.\n"},{"id":"148704","messageId":"20100822082056.GA5156@localhost","threadId":"24763","inReplyTo":"7viq335ct0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-08-22T08:20:56Z","receivedAt":"2010-08-22T08:20:56Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sun, Aug 22, 2010 at 12:25:31AM -0700, Junio C Hamano wrote:\n> \n> Traditionally (think \"git status\" output without \"-s\") the way to remind\n> oneself that some day these paths need to be added when they are ready has\n> been to keep them untracked but _not_ ignored, so that they will be listed\n> in the output.\n\nBut they do not have to be added. I may never want to add them. So\nI do not want git status to keep reminding me. That's why I add\nthem to .git/info/exclude. And with the current behavior, I do not\nsee anything wrong with that, because git does not consider them\ntrashable.\n\n> Quite contrary to what you earlier said in another message, adding\n> such a path that is not trashable does defeat the point of the \"ignore\"\n> mechanism.\n\nIf I understand correctly, the reason for considering ignored paths\ntrashable is because they are likely to have been tracked in other\nversions of the project, and would therefore frequently get in the\nway of commands like checkout or merge.\n\nBut the same is not true for private ignores in .git/info/exclude.\nSuch paths most likely never were tracked and never will be. And in\nthe rare case that an ignored path turns out to be a nuisance, it's\neasy enough to remove it from .git/info/exclude.\n\nSo I see only advantages if such a path is not considered\ntrashable.\n\nClemens\n"},{"id":"148732","messageId":"vpqaaod7mpa.fsf@bauges.imag.fr","threadId":"24763","inReplyTo":"20100818233900.GA27531@localhost","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-23T08:33:21Z","receivedAt":"2010-08-23T08:33:21Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> By default, checkout and fast-forward merge will overwrite ignored\n> files. Make this behavior configurable.\n\nI'd use this option if it gets into git.git.\n\nI didn't follow the discussions when the feature was added, and I was\nbasically not aware that Git could trash my ignored files this way.\nI've always thought that Git took great care not to touch untracked\nfiles, and I found this good ...\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"148740","messageId":"vpqd3t9656k.fsf@bauges.imag.fr","threadId":"24763","inReplyTo":"20100818233900.GA27531@localhost","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-23T09:37:07Z","receivedAt":"2010-08-23T09:37:07Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> By default, checkout and fast-forward merge will overwrite ignored\n> files. Make this behavior configurable.\n\nJust mentionning an alternative, or complementary approach:\n\nInstead of overwritting completely a .gitignore-d file, Git could\nrename it, and warn the user kindly. For example:\n\n$ git merge\n...\nWarning: existing ignored file 'foo' renamed to 'foo~'\n\n(in case foo~ already exists, it's possible to use numbered backups\njust like \"mv --backup=existing\" does for example)\n\n\nThat could complement your patch if core.overwriteignored is a\nmultiple-choice option instead of a Boolean:\n\n- \"overwrite\" => current behavior\n- \"refuse\" => your proposal\n- \"rename\" => my proposal\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"148752","messageId":"4C727E17.5070707@ira.uka.de","threadId":"24763","inReplyTo":"vpqd3t9656k.fsf@bauges.imag.fr","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Holger Hellmuth","fromEmail":"hellmuth@ira.uka.de","sentAt":"2010-08-23T13:56:39Z","receivedAt":"2010-08-23T13:56:39Z","isPatch":true,"sender":{"key":"hellmuth@ira.uka.de","avatar":null},"body":"Am 23.08.2010 11:37, schrieb Matthieu Moy:\n> Clemens Buchacher <drizzd@aon.at> writes:\n> \n>> By default, checkout and fast-forward merge will overwrite ignored\n>> files. Make this behavior configurable.\n> \n> Just mentionning an alternative, or complementary approach:\n> \n> Instead of overwritting completely a .gitignore-d file, Git could\n> rename it, and warn the user kindly. For example:\n\n\nYou've got my vote. This is the only option that combines safety with\nminimal configuration hassle.\n\nI didn't know about this subtle difference between .gitignore and\n.git/info/exclude. And while this makes sense I expect a sizable\npercentage of users will never even know that .git/info/exclude exists.\n\nAnd in practice I guess a lot of files will get added to .gitignore even\nthough they only are relevant to one user (out of carelessness or\nignorance). I know I have added patterns without making a conscious\ndecision about their relevance to others.\n"},{"id":"148755","messageId":"20100823151146.GA15379@localhost","threadId":"24763","inReplyTo":"4C727E17.5070707@ira.uka.de","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-08-23T15:11:46Z","receivedAt":"2010-08-23T15:11:46Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Mon, Aug 23, 2010 at 03:56:39PM +0200, Holger Hellmuth wrote:\n> \n> You've got my vote. This is the only option that combines safety with\n> minimal configuration hassle.\n> \n> I didn't know about this subtle difference between .gitignore and\n> .git/info/exclude. And while this makes sense I expect a sizable\n> percentage of users will never even know that .git/info/exclude exists.\n\nI don't know if this subtle difference was even intentional. But it\nmakes sense, and we simply need to make it explicit by documenting\nit and by making it optional.\n\nAnd in many cases, that behavior makes sense. Imagine a generated\nfile is accidentally added to git, later removed from version\ncontrol and added to .gitignore instead. A common scenario, I am\nsure.\n\nNow if you start merging, rebasing or bisecting with such a\nhistory, you _will_ run into this problem all the time.\n\nRenaming is certainly a possibility, but it does not really solve\nthe problem. We will end up with a bunch of renamed, possibly\nprecious files in our work tree, which means we have to clean up\nmanually anyways.\n\nSo I think it is better to make the decision whether or not a file\nis precious at the time it is added to .gitignore. In other words,\nnever add precious files to .gitignore, but add them to\n.git/info/exclude instead.\n\n> And in practice I guess a lot of files will get added to\n> .gitignore even though they only are relevant to one user (out of\n> carelessness or ignorance). I know I have added patterns without\n> making a conscious decision about their relevance to others.\n\nThen you have been using it wrong, just like I have.\nIgnorant/careless users may still make that mistake, but if git\ndoes not overwrite such files by default, at least they made a\nconscious decision at some point to take that risk.\n\nThat is if we can make it the default.\n\nClemens\n"},{"id":"148756","messageId":"7veidp2ufh.fsf@alter.siamese.dyndns.org","threadId":"24763","inReplyTo":"20100823151146.GA15379@localhost","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-23T15:57:38Z","receivedAt":"2010-08-23T15:57:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> On Mon, Aug 23, 2010 at 03:56:39PM +0200, Holger Hellmuth wrote:\n>> \n>> You've got my vote. This is the only option that combines safety with\n>> minimal configuration hassle.\n>> \n>> I didn't know about this subtle difference between .gitignore and\n>> .git/info/exclude. And while this makes sense I expect a sizable\n>> percentage of users will never even know that .git/info/exclude exists.\n>\n> I don't know if this subtle difference was even intentional.\n\nI am sorry, what difference are you talking about?\n"},{"id":"148830","messageId":"20100824072854.GA22951@localhost","threadId":"24763","inReplyTo":"7veidp2ufh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-08-24T07:28:54Z","receivedAt":"2010-08-24T07:28:54Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Mon, Aug 23, 2010 at 08:57:38AM -0700, Junio C Hamano wrote:\n> Clemens Buchacher <drizzd@aon.at> writes:\n> \n> > On Mon, Aug 23, 2010 at 03:56:39PM +0200, Holger Hellmuth wrote:\n> >> \n> >> You've got my vote. This is the only option that combines safety with\n> >> minimal configuration hassle.\n> >> \n> >> I didn't know about this subtle difference between .gitignore and\n> >> .git/info/exclude. And while this makes sense I expect a sizable\n> >> percentage of users will never even know that .git/info/exclude exists.\n> >\n> > I don't know if this subtle difference was even intentional.\n> \n> I am sorry, what difference are you talking about?\n\nThe fact that files in .gitignore are considered trashable, and the\nones in .git/info/exclude are not.\n"},{"id":"148844","messageId":"7vpqx8x9te.fsf@alter.siamese.dyndns.org","threadId":"24763","inReplyTo":"20100824072854.GA22951@localhost","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-24T16:19:25Z","receivedAt":"2010-08-24T16:19:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> The fact that files in .gitignore are considered trashable, and the\n> ones in .git/info/exclude are not.\n\nIs that a fact?\n\nThere may be a subtle bug in the ignore handling code.  The entries in\nthese various places are never meant to produce different behaviours.\n"},{"id":"149476","messageId":"20100831184410.GA29001@book.hvoigt.net","threadId":"24763","inReplyTo":"vpqaaod7mpa.fsf@bauges.imag.fr","subject":"Re: Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-08-31T18:44:11Z","receivedAt":"2010-08-31T18:44:11Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Mon, Aug 23, 2010 at 10:33:21AM +0200, Matthieu Moy wrote:\n> Clemens Buchacher <drizzd@aon.at> writes:\n> \n> > By default, checkout and fast-forward merge will overwrite ignored\n> > files. Make this behavior configurable.\n> \n> I'd use this option if it gets into git.git.\n> \n> I didn't follow the discussions when the feature was added, and I was\n> basically not aware that Git could trash my ignored files this way.\n> I've always thought that Git took great care not to touch untracked\n> files, and I found this good ...\n\nI agree here. I would like to have this as an option as well. And I\nthink adding a \"ignored but precious\" category does not make sense in\npractise. My guess is that a user would only know _after_ git trashed\nsome precious file that it belongs into that category;) I have told my\nusers that git is very careful with their files and I would like to keep\nthis promise.\n\nI was actually never annoyed by a failed checkout because of an\nuntracked file in the way. (A 'git clean -fx' always fixes this).\n\nThanks.\n\nHeiko\n"},{"id":"153058","messageId":"1286632380-7002-1-git-send-email-drizzd@aon.at","threadId":"24763","inReplyTo":"7v4oepaup7.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/5] do not overwrite untracked files in leading path","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-10-09T13:52:55Z","receivedAt":"2010-10-09T13:52:55Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Hi,\n\nOn Fri, Aug 20, 2010 at 01:35:32PM -0700, Junio C Hamano wrote:\n>\n>     $ echo foo >po\n>     $ git checkout pu\n> \n> should error out, as \"po\" is a directory that has tracked\n> contents, and we never said the untracked regular file \"po\" is\n> trashable, but the above sequence happily checks the branch out.\n\nThe following patch series is aimed at fixing this bug.\n\nClemens\n\n[PATCH 1/5] t7607: use test_commit and test_must_fail\n[PATCH 2/5] t7607: add leading-path tests\n[PATCH 3/5] add function check_ok_to_remove()\n[PATCH 4/5] lstat_cache: optionally return match_len\n[PATCH 5/5] do not overwrite files in leading path\n\n cache.h                    |    2 +-\n symlinks.c                 |   64 +++++++++++++++-------\n t/t7607-merge-overwrite.sh |  129 +++++++++++++++++++++++++++++---------------\n unpack-trees.c             |  121 ++++++++++++++++++++++++-----------------\n 4 files changed, 200 insertions(+), 116 deletions(-)\n"},{"id":"153061","messageId":"1286632380-7002-2-git-send-email-drizzd@aon.at","threadId":"24763","inReplyTo":"7v4oepaup7.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/5] t7607: use test_commit and test_must_fail","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-10-09T13:52:56Z","receivedAt":"2010-10-09T13:52:56Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Also make sure that aborted merges do not leave\nMERGE_HEAD except in the \"will not overwrite\nremoved file\" test, where we cannot do so. See\nalso the discussion in the following thread.\n\nhttp://mid.gmane.org/7vskopwxej.fsf@gitster.siamese.dyndns.org\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n t/t7607-merge-overwrite.sh |   78 +++++++++++++++++++------------------------\n 1 files changed, 35 insertions(+), 43 deletions(-)\n\ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex d82349a..6ed40b1 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -7,81 +7,73 @@ Do not overwrite changes.'\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-\techo c0 > c0.c &&\n-\tgit add c0.c &&\n-\tgit commit -m c0 &&\n-\tgit tag c0 &&\n-\techo c1 > c1.c &&\n-\tgit add c1.c &&\n-\tgit commit -m c1 &&\n-\tgit tag c1 &&\n+\ttest_commit c0 &&\n+\ttest_commit c1 &&\n+\ttest_commit \"c1a\" \"c1.t\" \"c1 a\" &&\n \tgit reset --hard c0 &&\n-\techo c2 > c2.c &&\n-\tgit add c2.c &&\n-\tgit commit -m c2 &&\n-\tgit tag c2 &&\n-\tgit reset --hard c1 &&\n-\techo \"c1 a\" > c1.c &&\n-\tgit add c1.c &&\n-\tgit commit -m \"c1 a\" &&\n-\tgit tag c1a &&\n+\ttest_commit c2 &&\n \techo \"VERY IMPORTANT CHANGES\" > important\n '\n \n test_expect_success 'will not overwrite untracked file' '\n \tgit reset --hard c1 &&\n-\tcat important > c2.c &&\n+\tcp important c2.t &&\n \ttest_must_fail git merge c2 &&\n-\ttest_cmp important c2.c\n+\t! test -f .git/MERGE_HEAD &&\n+\ttest_cmp important c2.t\n '\n \n test_expect_success 'will not overwrite new file' '\n \tgit reset --hard c1 &&\n-\tcat important > c2.c &&\n-\tgit add c2.c &&\n+\tcp important c2.t &&\n+\tgit add c2.t &&\n \ttest_must_fail git merge c2 &&\n-\ttest_cmp important c2.c\n+\t! test -f .git/MERGE_HEAD &&\n+\ttest_cmp important c2.t\n '\n \n test_expect_success 'will not overwrite staged changes' '\n \tgit reset --hard c1 &&\n-\tcat important > c2.c &&\n-\tgit add c2.c &&\n-\trm c2.c &&\n+\tcp important c2.t &&\n+\tgit add c2.t &&\n+\trm c2.t &&\n \ttest_must_fail git merge c2 &&\n-\tgit checkout c2.c &&\n-\ttest_cmp important c2.c\n+\t! test -f .git/MERGE_HEAD &&\n+\tgit checkout c2.t &&\n+\ttest_cmp important c2.t\n '\n \n test_expect_success 'will not overwrite removed file' '\n \tgit reset --hard c1 &&\n-\tgit rm c1.c &&\n-\tgit commit -m \"rm c1.c\" &&\n-\tcat important > c1.c &&\n+\tgit rm c1.t &&\n+\tgit commit -m \"rm c1.t\" &&\n+\tcp important c1.t &&\n \ttest_must_fail git merge c1a &&\n-\ttest_cmp important c1.c\n+\ttest_cmp important c1.t\n '\n \n test_expect_success 'will not overwrite re-added file' '\n \tgit reset --hard c1 &&\n-\tgit rm c1.c &&\n-\tgit commit -m \"rm c1.c\" &&\n-\tcat important > c1.c &&\n-\tgit add c1.c &&\n+\tgit rm c1.t &&\n+\tgit commit -m \"rm c1.t\" &&\n+\tcp important c1.t &&\n+\tgit add c1.t &&\n \ttest_must_fail git merge c1a &&\n-\ttest_cmp important c1.c\n+\t! test -f .git/MERGE_HEAD &&\n+\ttest_cmp important c1.t\n '\n \n test_expect_success 'will not overwrite removed file with staged changes' '\n \tgit reset --hard c1 &&\n-\tgit rm c1.c &&\n-\tgit commit -m \"rm c1.c\" &&\n-\tcat important > c1.c &&\n-\tgit add c1.c &&\n-\trm c1.c &&\n+\tgit rm c1.t &&\n+\tgit commit -m \"rm c1.t\" &&\n+\tcp important c1.t &&\n+\tgit add c1.t &&\n+\trm c1.t &&\n \ttest_must_fail git merge c1a &&\n-\tgit checkout c1.c &&\n-\ttest_cmp important c1.c\n+\t! test -f .git/MERGE_HEAD &&\n+\tgit checkout c1.t &&\n+\ttest_cmp important c1.t\n '\n \n test_done\n-- \n1.7.1.571.gba4d01\n"},{"id":"153060","messageId":"1286632380-7002-3-git-send-email-drizzd@aon.at","threadId":"24763","inReplyTo":"7v4oepaup7.fsf@alter.siamese.dyndns.org","subject":"[PATCH 2/5] t7607: add leading-path tests","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-10-09T13:52:57Z","receivedAt":"2010-10-09T13:52:57Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n\nI added a test for symlinks as well, even though from the code it\nlooks like overwriting untracked symlinks is intentionally allowed.\nOne could argue that symlinks are valuable just like regular files.\nOn the other hand, the potential loss of information is relatively\nsmall.\n\nFor consistency, I would prefer to treat symlinks just like regular\nfiles, but I do not feel strongly about it.\n\nClemens\n\n t/t7607-merge-overwrite.sh |   51 ++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 51 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex 6ed40b1..01b070b 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -12,6 +12,12 @@ test_expect_success 'setup' '\n \ttest_commit \"c1a\" \"c1.t\" \"c1 a\" &&\n \tgit reset --hard c0 &&\n \ttest_commit c2 &&\n+\tgit reset --hard c0 &&\n+\tmkdir sub &&\n+\techo \"sub/f\" > sub/f &&\n+\tgit add sub/f &&\n+\tgit commit -m sub &&\n+\tgit tag sub &&\n \techo \"VERY IMPORTANT CHANGES\" > important\n '\n \n@@ -23,6 +29,14 @@ test_expect_success 'will not overwrite untracked file' '\n \ttest_cmp important c2.t\n '\n \n+test_expect_success 'will overwrite tracked file' '\n+\tgit reset --hard c1 &&\n+\tcp important c2.t &&\n+\tgit add c2.t &&\n+\tgit commit -m important &&\n+\tgit checkout c2\n+'\n+\n test_expect_success 'will not overwrite new file' '\n \tgit reset --hard c1 &&\n \tcp important c2.t &&\n@@ -76,4 +90,41 @@ test_expect_success 'will not overwrite removed file with staged changes' '\n \ttest_cmp important c1.t\n '\n \n+test_expect_success 'will not overwrite untracked subtree' '\n+\tgit reset --hard c0 &&\n+\trm -rf sub &&\n+\tmkdir -p sub/f &&\n+\tcp important sub/f/important &&\n+\ttest_must_fail git merge sub &&\n+\t! test -f .git/MERGE_HEAD &&\n+\ttest_cmp important sub/f/important\n+'\n+\n+test_expect_failure 'will not overwrite untracked file in leading path' '\n+\tgit reset --hard c0 &&\n+\trm -rf sub &&\n+\tcp important sub &&\n+\ttest_must_fail git merge sub &&\n+\t! test -f .git/MERGE_HEAD &&\n+\ttest_cmp important sub\n+'\n+\n+test_expect_failure 'will not overwrite untracked symlink in leading path' '\n+\tgit reset --hard c0 &&\n+\trm -rf sub &&\n+\tmkdir sub2 &&\n+\tln -s sub2 sub &&\n+\ttest_must_fail git merge sub &&\n+\t! test -f .git/MERGE_HEAD\n+'\n+\n+test_expect_success 'will not be confused by symlink in leading path' '\n+\tgit reset --hard c0 &&\n+\trm -rf sub &&\n+\tln -s sub2 sub &&\n+\tgit add sub &&\n+\tgit commit -m ln &&\n+\tgit checkout sub\n+'\n+\n test_done\n-- \n1.7.1.571.gba4d01\n"},{"id":"153062","messageId":"1286632380-7002-4-git-send-email-drizzd@aon.at","threadId":"24763","inReplyTo":"7v4oepaup7.fsf@alter.siamese.dyndns.org","subject":"[PATCH 3/5] add function check_ok_to_remove()","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-10-09T13:52:58Z","receivedAt":"2010-10-09T13:52:58Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"This wraps some inline code into the function check_ok_to_remove(),\nwhich will later be used for leading path components as well.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n unpack-trees.c |  107 ++++++++++++++++++++++++++++++-------------------------\n 1 files changed, 58 insertions(+), 49 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 803445a..df1c920 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1127,14 +1127,65 @@ static int verify_clean_subdirectory(struct cache_entry *ce,\n  * See if we can find a case-insensitive match in the index that also\n  * matches the stat information, and assume it's that other file!\n  */\n-static int icase_exists(struct unpack_trees_options *o, struct cache_entry *dst, struct stat *st)\n+static int icase_exists(struct unpack_trees_options *o, const char *name, int len, struct stat *st)\n {\n \tstruct cache_entry *src;\n \n-\tsrc = index_name_exists(o->src_index, dst->name, ce_namelen(dst), 1);\n+\tsrc = index_name_exists(o->src_index, name, len, 1);\n \treturn src && !ie_match_stat(o->src_index, src, st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);\n }\n \n+static int check_ok_to_remove(const char *name, int len, int dtype,\n+\t\t\t      struct cache_entry *ce, struct stat *st,\n+\t\t\t      enum unpack_trees_error_types error_type,\n+\t\t\t      struct unpack_trees_options *o)\n+{\n+\tstruct cache_entry *result;\n+\n+\t/*\n+\t * It may be that the 'lstat()' succeeded even though\n+\t * target 'ce' was absent, because there is an old\n+\t * entry that is different only in case..\n+\t *\n+\t * Ignore that lstat() if it matches.\n+\t */\n+\tif (ignore_case && icase_exists(o, name, len, st))\n+\t\treturn 0;\n+\n+\tif (o->dir && excluded(o->dir, name, &dtype))\n+\t\t/*\n+\t\t * ce->name is explicitly excluded, so it is Ok to\n+\t\t * overwrite it.\n+\t\t */\n+\t\treturn 0;\n+\tif (S_ISDIR(st->st_mode)) {\n+\t\t/*\n+\t\t * We are checking out path \"foo\" and\n+\t\t * found \"foo/.\" in the working tree.\n+\t\t * This is tricky -- if we have modified\n+\t\t * files that are in \"foo/\" we would lose\n+\t\t * them.\n+\t\t */\n+\t\tif (verify_clean_subdirectory(ce, error_type, o) < 0)\n+\t\t\treturn -1;\n+\t\treturn 0;\n+\t}\n+\n+\t/*\n+\t * The previous round may already have decided to\n+\t * delete this path, which is in a subdirectory that\n+\t * is being replaced with a blob.\n+\t */\n+\tresult = index_name_exists(&o->result, name, len, 0);\n+\tif (result) {\n+\t\tif (result->ce_flags & CE_REMOVE)\n+\t\t\treturn 0;\n+\t}\n+\n+\treturn o->gently ? -1 :\n+\t\tadd_rejected_path(o, error_type, name);\n+}\n+\n /*\n  * We do not want to remove or overwrite a working tree file that\n  * is not tracked, unless it is ignored.\n@@ -1151,55 +1202,13 @@ static int verify_absent_1(struct cache_entry *ce,\n \tif (has_symlink_or_noent_leading_path(ce->name, ce_namelen(ce)))\n \t\treturn 0;\n \n-\tif (!lstat(ce->name, &st)) {\n-\t\tint dtype = ce_to_dtype(ce);\n-\t\tstruct cache_entry *result;\n-\n-\t\t/*\n-\t\t * It may be that the 'lstat()' succeeded even though\n-\t\t * target 'ce' was absent, because there is an old\n-\t\t * entry that is different only in case..\n-\t\t *\n-\t\t * Ignore that lstat() if it matches.\n-\t\t */\n-\t\tif (ignore_case && icase_exists(o, ce, &st))\n-\t\t\treturn 0;\n-\n-\t\tif (o->dir && excluded(o->dir, ce->name, &dtype))\n-\t\t\t/*\n-\t\t\t * ce->name is explicitly excluded, so it is Ok to\n-\t\t\t * overwrite it.\n-\t\t\t */\n-\t\t\treturn 0;\n-\t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\t/*\n-\t\t\t * We are checking out path \"foo\" and\n-\t\t\t * found \"foo/.\" in the working tree.\n-\t\t\t * This is tricky -- if we have modified\n-\t\t\t * files that are in \"foo/\" we would lose\n-\t\t\t * them.\n-\t\t\t */\n-\t\t\tif (verify_clean_subdirectory(ce, error_type, o) < 0)\n-\t\t\t\treturn -1;\n-\t\t\treturn 0;\n-\t\t}\n-\n-\t\t/*\n-\t\t * The previous round may already have decided to\n-\t\t * delete this path, which is in a subdirectory that\n-\t\t * is being replaced with a blob.\n-\t\t */\n-\t\tresult = index_name_exists(&o->result, ce->name, ce_namelen(ce), 0);\n-\t\tif (result) {\n-\t\t\tif (result->ce_flags & CE_REMOVE)\n-\t\t\t\treturn 0;\n-\t\t}\n-\n-\t\treturn o->gently ? -1 :\n-\t\t\tadd_rejected_path(o, error_type, ce->name);\n-\t}\n+\tif (!lstat(ce->name, &st))\n+\t\treturn check_ok_to_remove(ce->name, ce_namelen(ce),\n+\t\t\t\tce_to_dtype(ce), ce, &st,\n+\t\t\t\terror_type, o);\n \treturn 0;\n }\n+\n static int verify_absent(struct cache_entry *ce,\n \t\t\t enum unpack_trees_error_types error_type,\n \t\t\t struct unpack_trees_options *o)\n-- \n1.7.1.571.gba4d01\n"},{"id":"153059","messageId":"1286632380-7002-5-git-send-email-drizzd@aon.at","threadId":"24763","inReplyTo":"7v4oepaup7.fsf@alter.siamese.dyndns.org","subject":"[PATCH 4/5] lstat_cache: optionally return match_len","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-10-09T13:52:59Z","receivedAt":"2010-10-09T13:52:59Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Return match_len so that the caller can know which leading path\ncomponent matched.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n symlinks.c |   43 +++++++++++++++++++++++++++----------------\n 1 files changed, 27 insertions(+), 16 deletions(-)\n\ndiff --git a/symlinks.c b/symlinks.c\nindex 8860120..b7343eb 100644\n--- a/symlinks.c\n+++ b/symlinks.c\n@@ -64,11 +64,13 @@ static inline void reset_lstat_cache(struct cache_def *cache)\n  * of the prefix, where the cache should use the stat() function\n  * instead of the lstat() function to test each path component.\n  */\n-static int lstat_cache(struct cache_def *cache, const char *name, int len,\n-\t\t       int track_flags, int prefix_len_stat_func)\n+static int lstat_cache_matchlen(struct cache_def *cache,\n+\t\t\t\tconst char *name, int len,\n+\t\t\t\tint *ret_flags, int track_flags,\n+\t\t\t\tint prefix_len_stat_func)\n {\n \tint match_len, last_slash, last_slash_dir, previous_slash;\n-\tint match_flags, ret_flags, save_flags, max_len, ret;\n+\tint save_flags, max_len, ret;\n \tstruct stat st;\n \n \tif (cache->track_flags != track_flags ||\n@@ -90,13 +92,13 @@ static int lstat_cache(struct cache_def *cache, const char *name, int len,\n \t\tmatch_len = last_slash =\n \t\t\tlongest_path_match(name, len, cache->path, cache->len,\n \t\t\t\t\t   &previous_slash);\n-\t\tmatch_flags = cache->flags & track_flags & (FL_NOENT|FL_SYMLINK);\n+\t\t*ret_flags = cache->flags & track_flags & (FL_NOENT|FL_SYMLINK);\n \n \t\tif (!(track_flags & FL_FULLPATH) && match_len == len)\n \t\t\tmatch_len = last_slash = previous_slash;\n \n-\t\tif (match_flags && match_len == cache->len)\n-\t\t\treturn match_flags;\n+\t\tif (*ret_flags && match_len == cache->len)\n+\t\t\treturn match_len;\n \t\t/*\n \t\t * If we now have match_len > 0, we would know that\n \t\t * the matched part will always be a directory.\n@@ -105,16 +107,16 @@ static int lstat_cache(struct cache_def *cache, const char *name, int len,\n \t\t * a substring of the cache on a path component basis,\n \t\t * we can return immediately.\n \t\t */\n-\t\tmatch_flags = track_flags & FL_DIR;\n-\t\tif (match_flags && len == match_len)\n-\t\t\treturn match_flags;\n+\t\t*ret_flags = track_flags & FL_DIR;\n+\t\tif (*ret_flags && len == match_len)\n+\t\t\treturn match_len;\n \t}\n \n \t/*\n \t * Okay, no match from the cache so far, so now we have to\n \t * check the rest of the path components.\n \t */\n-\tret_flags = FL_DIR;\n+\t*ret_flags = FL_DIR;\n \tlast_slash_dir = last_slash;\n \tmax_len = len < PATH_MAX ? len : PATH_MAX;\n \twhile (match_len < max_len) {\n@@ -133,16 +135,16 @@ static int lstat_cache(struct cache_def *cache, const char *name, int len,\n \t\t\tret = lstat(cache->path, &st);\n \n \t\tif (ret) {\n-\t\t\tret_flags = FL_LSTATERR;\n+\t\t\t*ret_flags = FL_LSTATERR;\n \t\t\tif (errno == ENOENT)\n-\t\t\t\tret_flags |= FL_NOENT;\n+\t\t\t\t*ret_flags |= FL_NOENT;\n \t\t} else if (S_ISDIR(st.st_mode)) {\n \t\t\tlast_slash_dir = last_slash;\n \t\t\tcontinue;\n \t\t} else if (S_ISLNK(st.st_mode)) {\n-\t\t\tret_flags = FL_SYMLINK;\n+\t\t\t*ret_flags = FL_SYMLINK;\n \t\t} else {\n-\t\t\tret_flags = FL_ERR;\n+\t\t\t*ret_flags = FL_ERR;\n \t\t}\n \t\tbreak;\n \t}\n@@ -152,7 +154,7 @@ static int lstat_cache(struct cache_def *cache, const char *name, int len,\n \t * path types, FL_NOENT, FL_SYMLINK and FL_DIR, can be cached\n \t * for the moment!\n \t */\n-\tsave_flags = ret_flags & track_flags & (FL_NOENT|FL_SYMLINK);\n+\tsave_flags = *ret_flags & track_flags & (FL_NOENT|FL_SYMLINK);\n \tif (save_flags && last_slash > 0 && last_slash <= PATH_MAX) {\n \t\tcache->path[last_slash] = '\\0';\n \t\tcache->len = last_slash;\n@@ -176,7 +178,16 @@ static int lstat_cache(struct cache_def *cache, const char *name, int len,\n \t} else {\n \t\treset_lstat_cache(cache);\n \t}\n-\treturn ret_flags;\n+\treturn match_len;\n+}\n+\n+static int lstat_cache(struct cache_def *cache, const char *name, int len,\n+\t\t       int track_flags, int prefix_len_stat_func)\n+{\n+\tint flags;\n+\t(void)lstat_cache_matchlen(cache, name, len, &flags, track_flags,\n+\t\t\tprefix_len_stat_func);\n+\treturn flags;\n }\n \n #define USE_ONLY_LSTAT  0\n-- \n1.7.1.571.gba4d01\n"},{"id":"153063","messageId":"1286632380-7002-6-git-send-email-drizzd@aon.at","threadId":"24763","inReplyTo":"7v4oepaup7.fsf@alter.siamese.dyndns.org","subject":"[PATCH 5/5] do not overwrite files in leading path","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-10-09T13:53:00Z","receivedAt":"2010-10-09T13:53:00Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"If the work tree contains an untracked file x, and\nunpack-trees wants to checkout a path x/*, the\nfile x is removed unconditionally.\n\nInstead, apply the same checks that are normally\nused for untracked files, and abort if the file\ncannot be removed.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n\nIt's not exactly pretty. But it's the best solution I could find\nwithout major refactoring.\n\nClemens\n\n cache.h                    |    2 +-\n symlinks.c                 |   21 ++++++++++++++++-----\n t/t7607-merge-overwrite.sh |    2 +-\n unpack-trees.c             |   16 +++++++++++++---\n 4 files changed, 31 insertions(+), 10 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 2ef2fa3..f65d6f9 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -852,7 +852,7 @@ struct cache_def {\n \n extern int has_symlink_leading_path(const char *name, int len);\n extern int threaded_has_symlink_leading_path(struct cache_def *, const char *, int);\n-extern int has_symlink_or_noent_leading_path(const char *name, int len);\n+extern int check_leading_path(const char *name, int len);\n extern int has_dirs_only_path(const char *name, int len, int prefix_len);\n extern void schedule_dir_for_removal(const char *name, int len);\n extern void remove_scheduled_dirs(void);\ndiff --git a/symlinks.c b/symlinks.c\nindex b7343eb..3cacebd 100644\n--- a/symlinks.c\n+++ b/symlinks.c\n@@ -209,15 +209,26 @@ int has_symlink_leading_path(const char *name, int len)\n }\n \n /*\n- * Return non-zero if path 'name' has a leading symlink component or\n+ * Return zero if path 'name' has a leading symlink component or\n  * if some leading path component does not exists.\n+ *\n+ * Return -1 if leading path exists and is a directory.\n+ *\n+ * Return path length if leading path exists and is neither a\n+ * directory nor a symlink.\n  */\n-int has_symlink_or_noent_leading_path(const char *name, int len)\n+int check_leading_path(const char *name, int len)\n {\n \tstruct cache_def *cache = &default_cache;\t/* FIXME */\n-\treturn lstat_cache(cache, name, len,\n-\t\t\t   FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT) &\n-\t\t(FL_SYMLINK|FL_NOENT);\n+\tint flags;\n+\tint match_len = lstat_cache_matchlen(cache, name, len, &flags,\n+\t\t\t   FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT);\n+\tif (flags & (FL_SYMLINK|FL_NOENT))\n+\t\treturn 0;\n+\telse if (flags & FL_DIR)\n+\t\treturn -1;\n+\telse\n+\t\treturn match_len;\n }\n \n /*\ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex 01b070b..412db15 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -100,7 +100,7 @@ test_expect_success 'will not overwrite untracked subtree' '\n \ttest_cmp important sub/f/important\n '\n \n-test_expect_failure 'will not overwrite untracked file in leading path' '\n+test_expect_success 'will not overwrite untracked file in leading path' '\n \tgit reset --hard c0 &&\n \trm -rf sub &&\n \tcp important sub &&\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex df1c920..6816113 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -182,7 +182,7 @@ static void display_error_msgs(struct unpack_trees_options *o)\n  */\n static void unlink_entry(struct cache_entry *ce)\n {\n-\tif (has_symlink_or_noent_leading_path(ce->name, ce_namelen(ce)))\n+\tif (!check_leading_path(ce->name, ce_namelen(ce)))\n \t\treturn;\n \tif (remove_or_warn(ce->ce_mode, ce->name))\n \t\treturn;\n@@ -1194,18 +1194,28 @@ static int verify_absent_1(struct cache_entry *ce,\n \t\t\t\t enum unpack_trees_error_types error_type,\n \t\t\t\t struct unpack_trees_options *o)\n {\n+\tint len;\n \tstruct stat st;\n \n \tif (o->index_only || o->reset || !o->update)\n \t\treturn 0;\n \n-\tif (has_symlink_or_noent_leading_path(ce->name, ce_namelen(ce)))\n+\tlen = check_leading_path(ce->name, ce_namelen(ce));\n+\tif (!len)\n \t\treturn 0;\n+\telse if (len > 0) {\n+\t\tchar path[PATH_MAX + 1];\n+\t\tmemcpy(path, ce->name, len);\n+\t\tpath[len] = 0;\n+\t\tlstat(path, &st);\n \n-\tif (!lstat(ce->name, &st))\n+\t\treturn check_ok_to_remove(path, len, DT_UNKNOWN, NULL, &st,\n+\t\t\t\terror_type, o);\n+\t} else if (!lstat(ce->name, &st))\n \t\treturn check_ok_to_remove(ce->name, ce_namelen(ce),\n \t\t\t\tce_to_dtype(ce), ce, &st,\n \t\t\t\terror_type, o);\n+\n \treturn 0;\n }\n \n-- \n1.7.1.571.gba4d01\n"},{"id":"153069","messageId":"201010092114.51881.j6t@kdbg.org","threadId":"24763","inReplyTo":"1286632380-7002-3-git-send-email-drizzd@aon.at","subject":"Re: [PATCH 2/5] t7607: add leading-path tests","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-10-09T19:14:51Z","receivedAt":"2010-10-09T19:14:51Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 9. Oktober 2010, Clemens Buchacher wrote:\n> +test_expect_failure 'will not overwrite untracked symlink in leading path'\n\n> +test_expect_success 'will not be confused by symlink in leading path' '\n\nPlease guard these two with a SYMLINKS prerequisite.\n\n-- Hannes\n"},{"id":"153107","messageId":"617F82D8-DDDE-422C-B4F9-9D35B24BB420@sb.org","threadId":"24763","inReplyTo":"20100821080512.GA30141@localhost","subject":"Re: [PATCH] optionally disable overwriting of ignored files","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-10-09T22:39:27Z","receivedAt":"2010-10-09T22:39:27Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"On Aug 21, 2010, at 1:05 AM, Clemens Buchacher wrote:\n\n> Files that go into a tracked .gitignore are most likely generated\n> files, and therefore trashable. Files that go into\n> .git/info/exclude or into an untracked .gitignore (e.g. echo '*' >\n> precious-simulation-results/.gitignore), are not always generated\n> and may not be trashable. At least they would not likely get in the\n> way of checkout or merge.\n\nI can think of at least one common case that doesn't match this pattern - A web app that has a config file. What I've seen frequently done is the config file is named something like foo.config.template, and that's tracked, and you're encouraged to copy this to foo.config and change the values. This file may not want to be tracked because it contains sensitive information like database passwords, so the repo may stick it in the .gitignore file to ensure that this never gets added. However, this is most definitely a \"precious\" file. For this case, and others like it, I am strongly in favor of Junio's suggestion to add a 3rd category \"precious but untracked\".\n\n-Kevin Ballard"},{"id":"153119","messageId":"20101010063527.GC23100@burratino","threadId":"24763","inReplyTo":"1286632380-7002-2-git-send-email-drizzd@aon.at","subject":"Re: [PATCH 1/5] t7607: use test_commit and test_must_fail","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-10T06:35:27Z","receivedAt":"2010-10-10T06:35:27Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Clemens Buchacher wrote:\n\n> [Subject: t7607: use test_commit and test_must_fail]\n>\n> Also make sure that aborted merges do not leave\n> MERGE_HEAD except in the \"will not overwrite\n> removed file\" test, where we cannot do so.\n\nMotivation?\n\n> See\n> also the discussion in the following thread.\n>\n> http://mid.gmane.org/7vskopwxej.fsf@gitster.siamese.dyndns.org\n\nThat subthread is about teaching merge-recursive to write its results\nto the index first without touching the worktree for robustness.  It\nis not immediately obvious what that has to do with not leaving\nMERGE_HEAD.  Could you summarize the relevant part for the commit\nmessage so the reader does not have to track it down?\n\n> --- a/t/t7607-merge-overwrite.sh\n> +++ b/t/t7607-merge-overwrite.sh\n> @@ -7,81 +7,73 @@ Do not overwrite changes.'\n>  . ./test-lib.sh\n>  \n>  test_expect_success 'setup' '\n> -\techo c0 > c0.c &&\n> -\tgit add c0.c &&\n> -\tgit commit -m c0 &&\n> -\tgit tag c0 &&\n> +\ttest_commit c0 &&\n\nBehavior changes:\n\n . filename is c0.t instead of c0.c\n . uses test_tick for timestamp\n\nSounds good.\n\n> -\techo c1 > c1.c &&\n> -\tgit add c1.c &&\n> -\tgit commit -m c1 &&\n> -\tgit tag c1 &&\n> +\ttest_commit c1 &&\n\nAnother almost-noop (good).\n\n> -\tgit reset --hard c0 &&\n> -\techo c2 > c2.c &&\n> -\tgit add c2.c &&\n> -\tgit commit -m c2 &&\n> -\tgit tag c2 &&\n> -\tgit reset --hard c1 &&\n> -\techo \"c1 a\" > c1.c &&\n> -\tgit add c1.c &&\n> -\tgit commit -m \"c1 a\" &&\n> -\tgit tag c1a &&\n> +\ttest_commit \"c1a\" \"c1.t\" \"c1 a\" &&\n> +\tgit reset --hard c0 &&\n> +\ttest_commit c2 &&\n\nThe reader might not remember the c1.t filename.  Maybe\nit would be good to be explicit in the 'test_commit c1' line:\n\n\ttest_commit c1 c1.t\n\n?  Or maybe a comment could help?\n\n\t# Commit rewriting the file from c1\n\n>  \techo \"VERY IMPORTANT CHANGES\" > important\n>  '\n>  \n>  test_expect_success 'will not overwrite untracked file' '\n>  \tgit reset --hard c1 &&\n> -\tcat important > c2.c &&\n> +\tcp important c2.t &&\n>  \ttest_must_fail git merge c2 &&\n> -\ttest_cmp important c2.c\n> +\t! test -f .git/MERGE_HEAD &&\n> +\ttest_cmp important c2.t\n>  '\n\nNit: if backportability is not important, maybe\n\n\ttest_path_is_missing .git/MERGE_HEAD &&\n\nfor better error messages when running tests with -v and\nthe file is present?\n\n[...]\n> -\tcat important > c2.c &&\n> -\tgit add c2.c &&\n> -\trm c2.c &&\n> +\tcp important c2.t &&\n> +\tgit add c2.t &&\n> +\trm c2.t &&\n\nThese changes make for a lot of noise.  Why not specify the\nfilenames in the setup test to keep the .c extension?\n\nThe main change (checking that MERGE_HEAD is not present\nfor a merge that fails due to pre-merge checks) seems good.\nThanks.\n"},{"id":"153122","messageId":"20101010083543.GA12186@localhost","threadId":"24763","inReplyTo":"20101010063527.GC23100@burratino","subject":"[PATCH 1/5 v2] t7607: use test-lib functions and check MERGE_HEAD","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-10-10T08:35:43Z","receivedAt":"2010-10-10T08:35:43Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Use the test_commit and test_path_is_missing\nfunctions from the test library.\n\nAlso make sure that a merge which fails due to\npre-merge checks aborts properly and does not\nleave MERGE_HEAD behind.\n\nThe \"will not overwrite removed file\" test is an\nexception to this. It notices the untracked file\nat a stage where the merge is already well under\nway. Therefore we cannot abort the merge without\nmajor restructuring. See the following thread for\nmore details.\n\nhttp://mid.gmane.org/7vskopwxej.fsf@gitster.siamese.dyndns.org\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n\nOn Sun, Oct 10, 2010 at 01:35:27AM -0500, Jonathan Nieder wrote:\n> \n> The main change (checking that MERGE_HEAD is not present\n> for a merge that fails due to pre-merge checks) seems good.\n\nThanks. This new version incorporates all of your suggestions.\n\nClemens\n\n t/t7607-merge-overwrite.sh |   38 +++++++++++++++-----------------------\n 1 files changed, 15 insertions(+), 23 deletions(-)\n\ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex d82349a..b8fab54 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -7,48 +7,38 @@ Do not overwrite changes.'\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-\techo c0 > c0.c &&\n-\tgit add c0.c &&\n-\tgit commit -m c0 &&\n-\tgit tag c0 &&\n-\techo c1 > c1.c &&\n-\tgit add c1.c &&\n-\tgit commit -m c1 &&\n-\tgit tag c1 &&\n+\ttest_commit c0 c0.c &&\n+\ttest_commit c1 c1.c &&\n+\ttest_commit c1a c1.c \"c1 a\" &&\n \tgit reset --hard c0 &&\n-\techo c2 > c2.c &&\n-\tgit add c2.c &&\n-\tgit commit -m c2 &&\n-\tgit tag c2 &&\n-\tgit reset --hard c1 &&\n-\techo \"c1 a\" > c1.c &&\n-\tgit add c1.c &&\n-\tgit commit -m \"c1 a\" &&\n-\tgit tag c1a &&\n+\ttest_commit c2 c2.c &&\n \techo \"VERY IMPORTANT CHANGES\" > important\n '\n \n test_expect_success 'will not overwrite untracked file' '\n \tgit reset --hard c1 &&\n-\tcat important > c2.c &&\n+\tcp important c2.c &&\n \ttest_must_fail git merge c2 &&\n+\ttest_path_is_missing .git/MERGE_HEAD &&\n \ttest_cmp important c2.c\n '\n \n test_expect_success 'will not overwrite new file' '\n \tgit reset --hard c1 &&\n-\tcat important > c2.c &&\n+\tcp important c2.c &&\n \tgit add c2.c &&\n \ttest_must_fail git merge c2 &&\n+\ttest_path_is_missing .git/MERGE_HEAD &&\n \ttest_cmp important c2.c\n '\n \n test_expect_success 'will not overwrite staged changes' '\n \tgit reset --hard c1 &&\n-\tcat important > c2.c &&\n+\tcp important c2.c &&\n \tgit add c2.c &&\n \trm c2.c &&\n \ttest_must_fail git merge c2 &&\n+\ttest_path_is_missing .git/MERGE_HEAD &&\n \tgit checkout c2.c &&\n \ttest_cmp important c2.c\n '\n@@ -57,7 +47,7 @@ test_expect_success 'will not overwrite removed file' '\n \tgit reset --hard c1 &&\n \tgit rm c1.c &&\n \tgit commit -m \"rm c1.c\" &&\n-\tcat important > c1.c &&\n+\tcp important c1.c &&\n \ttest_must_fail git merge c1a &&\n \ttest_cmp important c1.c\n '\n@@ -66,9 +56,10 @@ test_expect_success 'will not overwrite re-added file' '\n \tgit reset --hard c1 &&\n \tgit rm c1.c &&\n \tgit commit -m \"rm c1.c\" &&\n-\tcat important > c1.c &&\n+\tcp important c1.c &&\n \tgit add c1.c &&\n \ttest_must_fail git merge c1a &&\n+\ttest_path_is_missing .git/MERGE_HEAD &&\n \ttest_cmp important c1.c\n '\n \n@@ -76,10 +67,11 @@ test_expect_success 'will not overwrite removed file with staged changes' '\n \tgit reset --hard c1 &&\n \tgit rm c1.c &&\n \tgit commit -m \"rm c1.c\" &&\n-\tcat important > c1.c &&\n+\tcp important c1.c &&\n \tgit add c1.c &&\n \trm c1.c &&\n \ttest_must_fail git merge c1a &&\n+\ttest_path_is_missing .git/MERGE_HEAD &&\n \tgit checkout c1.c &&\n \ttest_cmp important c1.c\n '\n-- \n1.7.1.571.gba4d01\n\n"},{"id":"153123","messageId":"20101010083858.GB12186@localhost","threadId":"24763","inReplyTo":"201010092114.51881.j6t@kdbg.org","subject":"[PATCH 2/5 v2] t7607: add leading-path tests","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-10-10T08:38:58Z","receivedAt":"2010-10-10T08:38:58Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n\nThis patch depends on PATCH 1/5 v2. The remaining patches 3 to 5\nstill apply cleanly.\n\nOn Sat, Oct 09, 2010 at 09:14:51PM +0200, Johannes Sixt wrote:\n> On Samstag, 9. Oktober 2010, Clemens Buchacher wrote:\n> > +test_expect_failure 'will not overwrite untracked symlink in leading path'\n> \n> > +test_expect_success 'will not be confused by symlink in leading path' '\n> \n> Please guard these two with a SYMLINKS prerequisite.\n\nDone. Thank you.\n\nClemens\n\n t/t7607-merge-overwrite.sh |   51 ++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 51 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex b8fab54..77fcaa2 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -12,6 +12,12 @@ test_expect_success 'setup' '\n \ttest_commit c1a c1.c \"c1 a\" &&\n \tgit reset --hard c0 &&\n \ttest_commit c2 c2.c &&\n+\tgit reset --hard c0 &&\n+\tmkdir sub &&\n+\techo \"sub/f\" > sub/f &&\n+\tgit add sub/f &&\n+\tgit commit -m sub &&\n+\tgit tag sub &&\n \techo \"VERY IMPORTANT CHANGES\" > important\n '\n \n@@ -23,6 +29,14 @@ test_expect_success 'will not overwrite untracked file' '\n \ttest_cmp important c2.c\n '\n \n+test_expect_success 'will overwrite tracked file' '\n+\tgit reset --hard c1 &&\n+\tcp important c2.c &&\n+\tgit add c2.c &&\n+\tgit commit -m important &&\n+\tgit checkout c2\n+'\n+\n test_expect_success 'will not overwrite new file' '\n \tgit reset --hard c1 &&\n \tcp important c2.c &&\n@@ -76,4 +90,41 @@ test_expect_success 'will not overwrite removed file with staged changes' '\n \ttest_cmp important c1.c\n '\n \n+test_expect_success 'will not overwrite untracked subtree' '\n+\tgit reset --hard c0 &&\n+\trm -rf sub &&\n+\tmkdir -p sub/f &&\n+\tcp important sub/f/important &&\n+\ttest_must_fail git merge sub &&\n+\ttest_path_is_missing .git/MERGE_HEAD &&\n+\ttest_cmp important sub/f/important\n+'\n+\n+test_expect_failure 'will not overwrite untracked file in leading path' '\n+\tgit reset --hard c0 &&\n+\trm -rf sub &&\n+\tcp important sub &&\n+\ttest_must_fail git merge sub &&\n+\ttest_path_is_missing .git/MERGE_HEAD &&\n+\ttest_cmp important sub\n+'\n+\n+test_expect_failure SYMLINKS 'will not overwrite untracked symlink in leading path' '\n+\tgit reset --hard c0 &&\n+\trm -rf sub &&\n+\tmkdir sub2 &&\n+\tln -s sub2 sub &&\n+\ttest_must_fail git merge sub &&\n+\ttest_path_is_missing .git/MERGE_HEAD\n+'\n+\n+test_expect_success SYMLINKS 'will not be confused by symlink in leading path' '\n+\tgit reset --hard c0 &&\n+\trm -rf sub &&\n+\tln -s sub2 sub &&\n+\tgit add sub &&\n+\tgit commit -m ln &&\n+\tgit checkout sub\n+'\n+\n test_done\n-- \n1.7.1.571.gba4d01\n\n"},{"id":"153447","messageId":"7vhbgpkcbp.fsf@alter.siamese.dyndns.org","threadId":"24763","inReplyTo":"20101010083543.GA12186@localhost","subject":"Re: [PATCH 1/5 v2] t7607: use test-lib functions and check MERGE_HEAD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-13T21:33:14Z","receivedAt":"2010-10-13T21:33:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> --ew6BAiZeqk4r7MaW\n> Content-Type: text/plain; charset=us-ascii\n> Content-Disposition: inline\n> Content-Transfer-Encoding: quoted-printable\n\nPlease don't do this...\n\n>\n> --ew6BAiZeqk4r7MaW\n> Content-Type: application/pgp-signature; name=\"signature.asc\"\n> Content-Description: Digital signature\n> Content-Disposition: inline\n>\n> -----BEGIN PGP SIGNATURE-----\n> Version: GnuPG v1.4.10 (GNU/Linux)\n>\n> iQEcBAEBAgAGBQJMsXrfAAoJELKdZexG8uqM/BwH/Ai2w+DFW4L9D1SivLoxDL8/\n> Z3YDRrx5oJa35ZhbBwInGJx7xwXG3Pn/mx0avGwoRwQe9cRufp6AO5hHUq3U1LNv\n> ZaP4RdlScMqeuKUu8mrbjJs4kumL/sjZ59MRnBZzX1Ovdq/GbKhJqidYTvmHQc6e\n> 0sngXx9Jf1WlS7m1sDztYPRJ3z2lF0js+BNHLIVOi6CgSbBJYdQzeLrvO/BVX9V0\n> P2F0bGZLWTqqOSLMav5jcYFAgIv8mRqxjre+1IviFuGTuu5hX7yTx++qAiV8CK0f\n> Fpe0HNQlYahbytd9qlOoqsbV5fuXkOoFqYHKmT6461u31QHaYPyBwsSWgjkF9IE=\n> =3pEL\n> -----END PGP SIGNATURE-----\n\nNor this.\n"},{"id":"153449","messageId":"7v7hhlkbtz.fsf@alter.siamese.dyndns.org","threadId":"24763","inReplyTo":"1286632380-7002-4-git-send-email-drizzd@aon.at","subject":"Re: [PATCH 3/5] add function check_ok_to_remove()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-13T21:43:52Z","receivedAt":"2010-10-13T21:43:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> This wraps some inline code into the function check_ok_to_remove(),\n> which will later be used for leading path components as well.\n>\n> Signed-off-by: Clemens Buchacher <drizzd@aon.at>\n\nNice depth reduction.\n"},{"id":"153452","messageId":"7v1v7tkb6g.fsf@alter.siamese.dyndns.org","threadId":"24763","inReplyTo":"1286632380-7002-6-git-send-email-drizzd@aon.at","subject":"Re: [PATCH 5/5] do not overwrite files in leading path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-13T21:57:59Z","receivedAt":"2010-10-13T21:57:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> If the work tree contains an untracked file x, and\n> unpack-trees wants to checkout a path x/*, the\n> file x is removed unconditionally.\n>\n> Instead, apply the same checks that are normally\n> used for untracked files, and abort if the file\n> cannot be removed.\n>\n\nToo short a line is also hard to read as too long a line.\n\n> diff --git a/cache.h b/cache.h\n> index 2ef2fa3..f65d6f9 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -852,7 +852,7 @@ struct cache_def {\n>  \n>  extern int has_symlink_leading_path(const char *name, int len);\n>  extern int threaded_has_symlink_leading_path(struct cache_def *, const char *, int);\n> -extern int has_symlink_or_noent_leading_path(const char *name, int len);\n> +extern int check_leading_path(const char *name, int len);\n\nThis is an API regression.  \"check\" in function name \"check-blah\" does not\ntell what you are checking (is it checking if the path is Ok?  is it\nchecking if the path has symlinked components?  is it checking if some\npart of the path overlaps with tracked content?  is it checking if you\nhave already run lstat(2) on some of the leading path components?), nor\nfor what purpose the check can be used (it is Ok to overwrite it?, is it\nOk to create a directory there?  is it Ok to create a file there?).\n\nAt least the old name told us what it checked, didn't it?\n\nThe approach you took looks sound, though.  What did you feel was \"not\nexactly pretty\" about it?\n\nThanks.\n"},{"id":"153457","messageId":"7v62x5iwj7.fsf@alter.siamese.dyndns.org","threadId":"24763","inReplyTo":"20101010083543.GA12186@localhost","subject":"Re: [PATCH 1/5 v2] t7607: use test-lib functions and check MERGE_HEAD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-13T21:59:40Z","receivedAt":"2010-10-13T21:59:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n>  test_expect_success 'will not overwrite untracked file' '\n>  \tgit reset --hard c1 &&\n> -\tcat important > c2.c &&\n> +\tcp important c2.c &&\n>  \ttest_must_fail git merge c2 &&\n> +\ttest_path_is_missing .git/MERGE_HEAD &&\n>  \ttest_cmp important c2.c\n>  '\n>  \n>  test_expect_success 'will not overwrite new file' '\n>  \tgit reset --hard c1 &&\n> -\tcat important > c2.c &&\n> +\tcp important c2.c &&\n\nWhy?\n"},{"id":"153461","messageId":"20101013223425.GB2558@localhost","threadId":"24763","inReplyTo":"7v1v7tkb6g.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/5] do not overwrite files in leading path","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-10-13T22:34:25Z","receivedAt":"2010-10-13T22:34:25Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Wed, Oct 13, 2010 at 02:57:59PM -0700, Junio C Hamano wrote:\n> Clemens Buchacher <drizzd@aon.at> writes:\n> \n> > If the work tree contains an untracked file x, and\n> > unpack-trees wants to checkout a path x/*, the\n> > file x is removed unconditionally.\n> >\n> > Instead, apply the same checks that are normally\n> > used for untracked files, and abort if the file\n> > cannot be removed.\n> >\n> \n> Too short a line is also hard to read as too long a line.\n\nOk.\n\n> > diff --git a/cache.h b/cache.h\n> > index 2ef2fa3..f65d6f9 100644\n> > --- a/cache.h\n> > +++ b/cache.h\n> > @@ -852,7 +852,7 @@ struct cache_def {\n> >  \n> >  extern int has_symlink_leading_path(const char *name, int len);\n> >  extern int threaded_has_symlink_leading_path(struct cache_def *, const char *, int);\n> > -extern int has_symlink_or_noent_leading_path(const char *name, int len);\n> > +extern int check_leading_path(const char *name, int len);\n> \n> This is an API regression.  \"check\" in function name \"check-blah\" does not\n> tell what you are checking (is it checking if the path is Ok?  is it\n> checking if the path has symlinked components?  is it checking if some\n> part of the path overlaps with tracked content?  is it checking if you\n> have already run lstat(2) on some of the leading path components?), nor\n> for what purpose the check can be used (it is Ok to overwrite it?, is it\n> Ok to create a directory there?  is it Ok to create a file there?).\n>\n> At least the old name told us what it checked, didn't it?\n\nYes, but that's not what it does any more. I worded it vague on\npurpose, because there is really no way to explain all it does in\none function name. This is the full description.\n\n/*\n * Return zero if path 'name' has a leading symlink component or\n * if some leading path component does not exists.\n *\n * Return -1 if leading path exists and is a directory.\n *\n * Return path length if leading path exists and is neither a\n * directory nor a symlink.\n */\n\nBut if you prefer, we can call it \"verify_clean_leading_path\"?\n\n> The approach you took looks sound, though.  What did you feel was \"not\n> exactly pretty\" about it?\n\nFirst, check_leading_path bothers me. It does too many things and\nencodes all of that in one integer return value.\n\nSecond, I also have to copy the offending path manually to a local\npath variable, even though lstat_cache already has that path stored\nin a buffer.\n\nThird, check_ok_to_remove expects a cache_entry. But in case of a\nmatch in check_leading_path, we do not have one and pass NULL to\ncheck_ok_to_rmove. It will not be used anyways, because\ncheck_leading_path should return > 0 only for regular files, and\nthe cache_entry is only used for directories in\nverify_clean_subdirectory.\n\nIf lstat returns with an error other than NOENT, or if\ncheck_ok_to_remove is called with anything other than a directory\nand cache_entry is NULL, we get a segmentation fault. Before, an\nerror was simply ignored. I don't know which is worse.\n\nOn the other hand, I think the patch fixes a nasty bug, so maybe we\ncan accept those issues for now.\n\nClemens\n"},{"id":"153587","messageId":"20101015064835.GA12119@localhost","threadId":"24763","inReplyTo":"20101013223425.GB2558@localhost","subject":"Re: [PATCH 5/5] do not overwrite files in leading path","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-10-15T06:48:35Z","receivedAt":"2010-10-15T06:48:35Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Thu, Oct 14, 2010 at 12:34:25AM +0200, Clemens Buchacher wrote:\n> \n> If lstat returns with an error other than NOENT, or if\n> check_ok_to_remove is called with anything other than a directory\n> and cache_entry is NULL, we get a segmentation fault. Before, an\n> error was simply ignored. I don't know which is worse.\n\nI suppose we only need the following additional changes.\n\n- die if lstat returns an error other than ENOENT.\n\n- Rewrite verify_clean_subdirectory to not require a cache_entry.\n\n- Expose lstat result and path cache to the caller of\n  lstat_cache_matchlen() in verify_absent_1().\n\n- rewrite check_leading_path (or 'verify_clean_path') to\n  check the full path and return zero if\n  + the leading path contains a symlink or\n  + the leading path exists, but the full path does not\n  and returns the path length of the offending entry otherwise.\n  \nI think that's manageable and much cleaner. I will start to work on\nit as soon as possible.\n\nThank you for your review.\n\nClemens\n"},{"id":"153594","messageId":"7vfww7e1j7.fsf@alter.siamese.dyndns.org","threadId":"24763","inReplyTo":"20101015064835.GA12119@localhost","subject":"Re: [PATCH 5/5] do not overwrite files in leading path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-15T18:47:24Z","receivedAt":"2010-10-15T18:47:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> On Thu, Oct 14, 2010 at 12:34:25AM +0200, Clemens Buchacher wrote:\n>> \n>> If lstat returns with an error other than NOENT, or if\n>> check_ok_to_remove is called with anything other than a directory\n>> and cache_entry is NULL, we get a segmentation fault. Before, an\n>> error was simply ignored. I don't know which is worse.\n>\n> I suppose we only need the following additional changes.\n>\n> - die if lstat returns an error other than ENOENT.\n>\n> - Rewrite verify_clean_subdirectory to not require a cache_entry.\n>\n> - Expose lstat result and path cache to the caller of\n>   lstat_cache_matchlen() in verify_absent_1().\n>\n> - rewrite check_leading_path (or 'verify_clean_path') to\n>   check the full path and return zero if\n>   + the leading path contains a symlink or\n>   + the leading path exists, but the full path does not\n>   and returns the path length of the offending entry otherwise.\n>   \n> I think that's manageable and much cleaner. I will start to work on\n> it as soon as possible.\n\nThanks.  I like seeing people thinking things through ;-).\n"}]}