{"thread":{"id":"4137","subject":"Allow one-way tree merge to remove old files","startedAt":"2006-05-14T17:43:50Z","lastAt":"2006-05-15T15:09:31Z","messageCount":8,"participants":["Linus Torvalds","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"19918","messageId":"Pine.LNX.4.64.0605141040210.3866@g5.osdl.org","threadId":"4137","inReplyTo":null,"subject":"Allow one-way tree merge to remove old files","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-14T17:43:50Z","receivedAt":"2006-05-14T17:43:50Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nFor some random reason (probably just because nobody noticed), the one-way \nmerge strategy didn't mark deleted files as deleted, so if you used\n\n\tgit-read-tree -m -u <newtree>\n\nit would update the files that got changed in the index, but it would not \ndelete the files that got deleted.\n\nThis should fix it, and I can't imagine that anybody depends on the old \nstrange \"update only existing files\" behaviour.\n\nSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n---\ndiff --git a/read-tree.c b/read-tree.c\nindex e926e4c..11157f4 100644\n--- a/read-tree.c\n+++ b/read-tree.c\n@@ -684,7 +684,7 @@ static int oneway_merge(struct cache_ent\n \t\t\t     merge_size);\n \n \tif (!a)\n-\t\treturn 0;\n+\t\treturn deleted_entry(old, NULL);\n \tif (old && same(old, a)) {\n \t\treturn keep_entry(old);\n \t}\n"},{"id":"19920","messageId":"7vy7x41omg.fsf@assigned-by-dhcp.cox.net","threadId":"4137","inReplyTo":"Pine.LNX.4.64.0605141040210.3866@g5.osdl.org","subject":"Re: Allow one-way tree merge to remove old files","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-14T17:51:19Z","receivedAt":"2006-05-14T17:51:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> For some random reason (probably just because nobody noticed), the one-way \n> merge strategy didn't mark deleted files as deleted, so if you used\n>\n> \tgit-read-tree -m -u <newtree>\n>\n> it would update the files that got changed in the index, but it would not \n> delete the files that got deleted.\n\nGood catch.  I think it is a leftover from the days we did not\nhave -u there; returning 0 was a way to say \"there is no cache\nentry resulting from this round of path-merge\".\n"},{"id":"19923","messageId":"Pine.LNX.4.64.0605141110150.3866@g5.osdl.org","threadId":"4137","inReplyTo":"Pine.LNX.4.64.0605141040210.3866@g5.osdl.org","subject":"Simplify \"git reset --hard\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-14T18:20:37Z","receivedAt":"2006-05-14T18:20:37Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nNow that the one-way merge strategy does the right thing wrt files that do \nnot exist in the result, just remove all the random crud we did in \"git \nreset\" to do this all by hand.\n\nInstead, just pass in \"-u\" to git-read-tree when we do a hard reset, and \ndepend on git-read-tree to update the working tree appropriately.\n\nThis basically means that git reset turns into\n\n\t# Always update the HEAD ref\n\tgit update-ref HEAD \"$rev\"\n\n\tcase \"--soft\"\n\t\t# do nothing to index/working tree\n\tcase \"--hard\"\n\t\t# read index _and_ update working tree\n\t\tgit-read-tree --reset -u \"$rev\"\n\tcase \"--mixed\"\n\t\t# update just index, report on working tree differences\n\t\tgit-read-tree --reset \"$rev\"\n\t\tgit-update-index --refresh\n\nwhich is what it was always semantically doing, it just did it in a\nrather strange way because it was written to not expect git-read-tree to \ndo anything to the working tree.\n\nSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n---\n\nNOTE! The switch to use \"git-read-tree -u\" does actually result in a real \nchange: we will now remove files that were in the index but not in HEAD \nbefore (ie files added with \"git add\"). I'd argue that this is a bug-fix.\n\n git-reset.sh |   50 ++++----------------------------------------------\n 1 files changed, 4 insertions(+), 46 deletions(-)\n\ndiff --git a/git-reset.sh b/git-reset.sh\nindex 6cb073c..0ee3e3e 100755\n--- a/git-reset.sh\n+++ b/git-reset.sh\n@@ -6,6 +6,7 @@ USAGE='[--mixed | --soft | --hard]  [<co\n tmp=${GIT_DIR}/reset.$$\n trap 'rm -f $tmp-*' 0 1 2 3 15\n \n+update=\n reset_type=--mixed\n case \"$1\" in\n --mixed | --soft | --hard)\n@@ -23,24 +24,7 @@ # We need to remember the set of paths t\n # behind before a hard reset, so that we can remove them.\n if test \"$reset_type\" = \"--hard\"\n then\n-\t{\n-\t\tgit-ls-files --stage -z\n-\t\tgit-rev-parse --verify HEAD 2>/dev/null &&\n-\t\tgit-ls-tree -r -z HEAD\n-\t} | perl -e '\n-\t    use strict;\n-\t    my %seen;\n-\t    $/ = \"\\0\";\n-\t    while (<>) {\n-\t\tchomp;\n-\t\tmy ($info, $path) = split(/\\t/, $_);\n-\t\tnext if ($info =~ / tree /);\n-\t\tif (!$seen{$path}) {\n-\t\t\t$seen{$path} = 1;\n-\t\t\tprint \"$path\\0\";\n-\t\t}\n-\t    }\n-\t' >$tmp-exists\n+\tupdate=-u\n fi\n \n # Soft reset does not touch the index file nor the working tree\n@@ -54,7 +38,7 @@ then\n \t\tdie \"Cannot do a soft reset in the middle of a merge.\"\n \tfi\n else\n-\tgit-read-tree --reset \"$rev\" || exit\n+\tgit-read-tree --reset $update \"$rev\" || exit\n fi\n \n # Any resets update HEAD to the head being switched to.\n@@ -68,33 +52,7 @@ git-update-ref HEAD \"$rev\"\n \n case \"$reset_type\" in\n --hard )\n-\t# Hard reset matches the working tree to that of the tree\n-\t# being switched to.\n-\tgit-checkout-index -f -u -q -a\n-\tgit-ls-files --cached -z |\n-\tperl -e '\n-\t\tuse strict;\n-\t\tmy (%keep, $fh);\n-\t\t$/ = \"\\0\";\n-\t\twhile (<STDIN>) {\n-\t\t\tchomp;\n-\t\t\t$keep{$_} = 1;\n-\t\t}\n-\t\topen $fh, \"<\", $ARGV[0]\n-\t\t\tor die \"cannot open $ARGV[0]\";\n-\t\twhile (<$fh>) {\n-\t\t\tchomp;\n-\t\t\tif (! exists $keep{$_}) {\n-\t\t\t\t# it is ok if this fails -- it may already\n-\t\t\t\t# have been culled by checkout-index.\n-\t\t\t\tunlink $_;\n-\t\t\t\twhile (s|/[^/]*$||) {\n-\t\t\t\t\trmdir($_) or last;\n-\t\t\t\t}\n-\t\t\t}\n-\t\t}\n-\t' $tmp-exists\n-\t;;\n+\t;; # Nothing else to do\n --soft )\n \t;; # Nothing else to do\n --mixed )\n"},{"id":"19975","messageId":"7v1wuvwzdv.fsf@assigned-by-dhcp.cox.net","threadId":"4137","inReplyTo":"Pine.LNX.4.64.0605141110150.3866@g5.osdl.org","subject":"Re: Simplify \"git reset --hard\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-15T06:55:24Z","receivedAt":"2006-05-15T06:55:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> Now that the one-way merge strategy does the right thing wrt files that do \n> not exist in the result, just remove all the random crud we did in \"git \n> reset\" to do this all by hand.\n>\n> Instead, just pass in \"-u\" to git-read-tree when we do a hard reset, and \n> depend on git-read-tree to update the working tree appropriately.\n\nWell, this is wrong.  Local modifications remain after your\nversion of \"git-reset --hard HEAD\". which is not what we want\nfrom a hard reset.\n\n-- >8 --\n\ndiff --git a/git-reset.sh b/git-reset.sh\nindex 0ee3e3e..ecc111b 100755\n--- a/git-reset.sh\n+++ b/git-reset.sh\n@@ -52,7 +52,7 @@ git-update-ref HEAD \"$rev\"\n \n case \"$reset_type\" in\n --hard )\n-\t;; # Nothing else to do\n+\tgit-checkout-index -f -q -u -a ;;\n --soft )\n \t;; # Nothing else to do\n --mixed )\n"},{"id":"19976","messageId":"7vwtcnvk76.fsf@assigned-by-dhcp.cox.net","threadId":"4137","inReplyTo":"7v1wuvwzdv.fsf@assigned-by-dhcp.cox.net","subject":"Re: Simplify \"git reset --hard\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-15T07:08:45Z","receivedAt":"2006-05-15T07:08:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n>> Instead, just pass in \"-u\" to git-read-tree when we do a hard reset, and \n>> depend on git-read-tree to update the working tree appropriately.\n>\n> Well, this is wrong.  Local modifications remain after your\n> version of \"git-reset --hard HEAD\". which is not what we want\n> from a hard reset.\n\n... and attempting to paper it over in git-reset.sh is also\nwrong.  Keep your \"--hard is noop\" change in git-reset.sh and\nreplace it with this would be the right fix.\n\n-- >8 --\n\ndiff --git a/read-tree.c b/read-tree.c\nindex 11157f4..c135f08 100644\n--- a/read-tree.c\n+++ b/read-tree.c\n@@ -686,6 +686,7 @@ static int oneway_merge(struct cache_ent\n \tif (!a)\n \t\treturn deleted_entry(old, NULL);\n \tif (old && same(old, a)) {\n+\t\told->ce_flags |= htons(CE_UPDATE);\n \t\treturn keep_entry(old);\n \t}\n \treturn merged_entry(a, NULL);\n"},{"id":"19977","messageId":"7vr72vvigy.fsf@assigned-by-dhcp.cox.net","threadId":"4137","inReplyTo":"7vwtcnvk76.fsf@assigned-by-dhcp.cox.net","subject":"Re: Simplify \"git reset --hard\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-15T07:46:05Z","receivedAt":"2006-05-15T07:46:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> Junio C Hamano <junkio@cox.net> writes:\n>\n>>> Instead, just pass in \"-u\" to git-read-tree when we do a hard reset, and \n>>> depend on git-read-tree to update the working tree appropriately.\n>>\n>> Well, this is wrong.  Local modifications remain after your\n>> version of \"git-reset --hard HEAD\". which is not what we want\n>> from a hard reset.\n>\n> ... and attempting to paper it over in git-reset.sh is also\n> wrong.  Keep your \"--hard is noop\" change in git-reset.sh and\n> replace it with this would be the right fix.\n\n-- >8 --\nread-tree -u one-way merge fix to check out locally modified paths.\n\nThe \"-u\" flag means \"update the working tree files\", but to\nother types of merges, it also implies \"I want to keep my local\nchanges\" -- because they prevent local changes from getting lost\nby using verify_uptodate.  The one-way merge is different from\nother merges in that its purpose is opposite of doing something\nelse while keeping unrelated local changes.  The point of\none-way merge is to nuke local changes.  So while it feels\nsomewhat wrong that this actively loses local changes, it is the\nright thing to do.\n\nThe earlier one marked old->ce_flags to be updated\nunconditionally, but that would cause 18,000 paths to be updated\nwhen you have only a few paths different from the HEAD you are\nswitching to, which is far worse than what we used to do in\ngit-reset by hand.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n\n---\n\n * Third time lucky ;-)\n\ndiff --git a/read-tree.c b/read-tree.c\nindex 11157f4..d847c6f 100644\n--- a/read-tree.c\n+++ b/read-tree.c\n@@ -686,6 +698,9 @@ static int oneway_merge(struct cache_ent\n \tif (!a)\n \t\treturn deleted_entry(old, NULL);\n \tif (old && same(old, a)) {\n+\t\tstruct stat st;\n+\t\tif (lstat(old->name, &st) || ce_match_stat(old, &st, 1))\n+\t\t\told->ce_flags |= htons(CE_UPDATE);\n \t\treturn keep_entry(old);\n \t}\n \treturn merged_entry(a, NULL);\n"},{"id":"19994","messageId":"Pine.LNX.4.64.0605150752490.3866@g5.osdl.org","threadId":"4137","inReplyTo":"7vr72vvigy.fsf@assigned-by-dhcp.cox.net","subject":"Re: Simplify \"git reset --hard\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-15T14:55:59Z","receivedAt":"2006-05-15T14:55:59Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 15 May 2006, Junio C Hamano wrote:\n>\n> read-tree -u one-way merge fix to check out locally modified paths.\n> \n> The \"-u\" flag means \"update the working tree files\", but to\n> other types of merges, it also implies \"I want to keep my local\n> changes\" -- because they prevent local changes from getting lost\n> by using verify_uptodate.  The one-way merge is different from\n> other merges in that its purpose is opposite of doing something\n> else while keeping unrelated local changes.  The point of\n> one-way merge is to nuke local changes.  So while it feels\n> somewhat wrong that this actively loses local changes, it is the\n> right thing to do.\n\nAck. On the other hand, I wonder if it might not make sense to have this \npart potentially depend on the \"--reset\" flag.\n\nThat way you wouldn't even have to apologize for it.\n\n\t\tLinus\n"},{"id":"19997","messageId":"Pine.LNX.4.64.0605150807030.3866@g5.osdl.org","threadId":"4137","inReplyTo":"Pine.LNX.4.64.0605150752490.3866@g5.osdl.org","subject":"Re: Simplify \"git reset --hard\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-15T15:09:31Z","receivedAt":"2006-05-15T15:09:31Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 15 May 2006, Linus Torvalds wrote:\n> \n> Ack. On the other hand, I wonder if it might not make sense to have this \n> part potentially depend on the \"--reset\" flag.\n> \n> That way you wouldn't even have to apologize for it.\n\nIe, something like this..\n\nThis should allow a two-way or three-way merge to akso ignore any dirty \nstate when you use \"--reset\", because they use \"verify_uptodate()\", which \nnow would set the CD_UPDATE flag on the ce rather than complain about it \nbeing different (if it survives the merge, of course - often it won't, but \nthen we won't care).\n\nThis is all totally untested, of course.\n\n\t\tLinus\n\n----\ndiff --git a/read-tree.c b/read-tree.c\nindex 7c83031..f2d674c 100644\n--- a/read-tree.c\n+++ b/read-tree.c\n@@ -13,6 +13,7 @@ #include \"cache-tree.h\"\n #include <sys/time.h>\n #include <signal.h>\n \n+static int reset = 0;\n static int merge = 0;\n static int update = 0;\n static int index_only = 0;\n@@ -419,6 +420,10 @@ static void verify_uptodate(struct cache\n \t\t\treturn;\n \t\terrno = 0;\n \t}\n+\tif (reset) {\n+\t\tce->ce_flags |= htons(CE_UPDATE);\n+\t\treturn;\n+\t}\n \tif (errno == ENOENT)\n \t\treturn;\n \tdie(\"Entry '%s' not uptodate. Cannot merge.\", ce->name);\n@@ -723,6 +728,11 @@ static int oneway_merge(struct cache_ent\n \t\treturn deleted_entry(old, NULL);\n \t}\n \tif (old && same(old, a)) {\n+\t\tif (reset) {\n+\t\t\tstruct stat st;\n+\t\t\tif (lstat(old->name, &st) || ce_match_stat(old, &st, 1))\n+\t\t\t\told->ce_flags |= htons(CE_UPDATE);\n+\t\t}\n \t\treturn keep_entry(old);\n \t}\n \treturn merged_entry(a, NULL);\n@@ -790,7 +800,7 @@ static struct cache_file cache_file;\n \n int main(int argc, char **argv)\n {\n-\tint i, newfd, reset, stage = 0;\n+\tint i, newfd, stage = 0;\n \tunsigned char sha1[20];\n \tmerge_fn_t fn = NULL;\n \n"}]}