{"thread":{"id":"2487","subject":"[PATCH] Stgit - gitmergeonefile.py: handle removal vs. changes","startedAt":"2005-11-13T19:42:25Z","lastAt":"2006-01-08T01:50:49Z","messageCount":9,"participants":["Paolo 'Blaisorblade' Giarrusso","Catalin Marinas","Blaisorblade","Chuck Lever"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"11737","messageId":"20051113194225.20447.57910.stgit@zion.home.lan","threadId":"2487","inReplyTo":null,"subject":"[PATCH] Stgit - gitmergeonefile.py: handle removal vs. changes","fromName":"Paolo 'Blaisorblade' Giarrusso","fromEmail":"blaisorblade@yahoo.it","sentAt":"2005-11-13T19:42:25Z","receivedAt":"2005-11-13T19:42:25Z","isPatch":true,"sender":{"key":"blaisorblade@yahoo.it","avatar":null},"body":"I just got a \"removal vs. changed\" conflict, which is unhandled by StGit. That\nis taken from git-merge-one-file resolver, but is bad, as stg resolved does not\nhandle unmerged entries (and probably it should be fixed too).\n\nSample patch included, but some thought must be done on it (see the comments I\nleft in).\n\nSigned-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>\n---\n\n gitmergeonefile.py |   24 ++++++++++++++++++++++++\n 1 files changed, 24 insertions(+), 0 deletions(-)\n\ndiff --git a/gitmergeonefile.py b/gitmergeonefile.py\nindex 1cba193..9344d33 100755\n--- a/gitmergeonefile.py\n+++ b/gitmergeonefile.py\n@@ -180,6 +180,30 @@ if orig_hash:\n             os.remove(path)\n         __remove_files()\n         sys.exit(os.system('git-update-index --remove -- %s' % path))\n+    # file deleted in one and changed in the other\n+    else:\n+        # Do something here - we must at least merge the entry in the cache,\n+        # instead of leaving it in U(nmerged) state. In fact, stg resolved\n+        # does not handle that.\n+\n+        # Do the same thing cogito does - remove the file in any case.\n+        os.system('git-update-index --remove -- %s' % path)\n+\n+        #if file1_hash:\n+            ## file deleted upstream and changed in the patch. The patch is\n+            ## probably going to move the changes elsewhere.\n+\n+            #os.system('git-update-index --remove -- %s' % path)\n+        #else:\n+            ## file deleted in the patch and changed upstream. We could re-delete\n+            ## it, but for now leave it there - and let the user check if he\n+            ## still wants to remove the file.\n+\n+            ## reset the cache to the first branch\n+            #os.system('git-update-index --cacheinfo %s %s %s'\n+                      #% (file1_mode, file1_hash, path))\n+        __conflict()\n+\n # file does not exist in origin\n else:\n     # file added in both\n"},{"id":"11882","messageId":"b0943d9e0511150154y2d2af24ck@mail.gmail.com","threadId":"2487","inReplyTo":"20051113194225.20447.57910.stgit@zion.home.lan","subject":"Re: [PATCH] Stgit - gitmergeonefile.py: handle removal vs. changes","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2005-11-15T09:54:43Z","receivedAt":"2005-11-15T09:54:43Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 13/11/05, Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it> wrote:\n> I just got a \"removal vs. changed\" conflict, which is unhandled by StGit. That\n> is taken from git-merge-one-file resolver, but is bad, as stg resolved does not\n> handle unmerged entries (and probably it should be fixed too).\n\nI think it 'stg resolved' should be fixed as well (in case there are\nunmerged entries for other reasons). My initial idea was to make\ngitmergeonefile not to leave any unmerged entries in the index. As you\ncould see, there are cases where it failed.\n\nI can see the following scenarios for a file:\n\n1. deleted in the base and modified by the patch. It should leave the\nfile in the tree together with file.older. Another option would be to\nremove the file and leave both file.older and file.remote in the tree\n(here .remote means the version in the patch) but I would prefer the\nfirst one.\n\n2. changed in the base but deleted by the patch. It should remove the\nfile from the tree but leave file.older and file.local. The other\noption is to leave the file in the tree but, as above, I prefer the\nfirst one.\n\nMaybe StGIT should try to track the renaming as well but I haven't\nplayed with this feature in GIT at all.\n\n--\nCatalin\n"},{"id":"12015","messageId":"200511161544.13825.blaisorblade@yahoo.it","threadId":"2487","inReplyTo":"b0943d9e0511150154y2d2af24ck@mail.gmail.com","subject":"Re: [PATCH] Stgit - gitmergeonefile.py: handle removal vs. changes","fromName":"Blaisorblade","fromEmail":"blaisorblade@yahoo.it","sentAt":"2005-11-16T14:44:12Z","receivedAt":"2005-11-16T14:44:12Z","isPatch":true,"sender":{"key":"blaisorblade@yahoo.it","avatar":null},"body":"On Tuesday 15 November 2005 10:54, Catalin Marinas wrote:\n> On 13/11/05, Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it> wrote:\n> > I just got a \"removal vs. changed\" conflict, which is unhandled by StGit.\n> > That is taken from git-merge-one-file resolver, but is bad, as stg\n> > resolved does not handle unmerged entries (and probably it should be\n> > fixed too).\n\n> I think it 'stg resolved' should be fixed as well (in case there are\n> unmerged entries for other reasons).\n\nYep, I was thinking that too but was too lazy to implement.\n\nActually, with .git/commits we are reimplementing handling of \"unmerged\" \nentries... it could be better to use the \"unmerged entry\" stgit idea. So \"stg \nresolved\" should modify the entries by itself.\n\n> My initial idea was to make \n> gitmergeonefile not to leave any unmerged entries in the index. As you\n> could see, there are cases where it failed.\n\nYep... it seems you took examples from git-merge-one-file, but that's lacking \n(but it's low-level so it's appropriate for it - it must leave unmerged \nentries when there are conflicts).\n\n> I can see the following scenarios for a file:\n\nIn both cases, we're going to have a conflict, so we leave file.\n{older,remote,local} as appropriate and already done.\n\n> 1. deleted in the base and modified by the patch. It should leave the\n> file in the tree together with file.older.\n\nWhy not leaving file.remote? We already do that in general, so we have a \nduplicate, but it's easier to understand.\n\n> Another option would be to \n> remove the file and leave both file.older and file.remote in the tree\n> (here .remote means the version in the patch)\n\nI remember that at times, but .remote is very confusing... I see that's the \nmishandling is induced by various sources, maybe including \"merge\" itself, \nbut that program (and possibly others) supports changing the labels, and this \nshould probably be done (using \"original\", \"patched\" and \"upstream\" \nprobably).\n\n> but I would prefer the \n> first one.\n\n> 2. changed in the base but deleted by the patch. It should remove the\n> file from the tree but leave file.older and file.local. The other\n> option is to leave the file in the tree but, as above, I prefer the\n> first one.\n\nThe policy about when to remove the file and when to leave it is very \npersonal... the user must anyway solve the conflict in some smart way... \nabout the defaults, anything would do, but if we really care we could leave \nthe user the choice.\n\nFor the Linux kernel, my experience is that when a file is removed it's either \nbecause it's renamed, it's refactored, or it's removed. In all these cases, \nthere's often little interest in reviving it in the patch... However it's \njust a slight preference.\n\n> Maybe StGIT should try to track the renaming as well but I haven't\n> played with this feature in GIT at all.\n\n-- \nInform me of my mistakes, so I can keep imitating Homer Simpson's \"Doh!\".\nPaolo Giarrusso, aka Blaisorblade (Skype ID \"PaoloGiarrusso\", ICQ 215621894)\nhttp://www.user-mode-linux.org/~blaisorblade\n\n\t\n\n\t\n\t\t\n___________________________________ \nYahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB \nhttp://mail.yahoo.it\n"},{"id":"12152","messageId":"b0943d9e0511171410y357fb0bfv@mail.gmail.com","threadId":"2487","inReplyTo":"200511161544.13825.blaisorblade@yahoo.it","subject":"Re: [PATCH] Stgit - gitmergeonefile.py: handle removal vs. changes","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2005-11-17T22:10:11Z","receivedAt":"2005-11-17T22:10:11Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 16/11/05, Blaisorblade <blaisorblade@yahoo.it> wrote:\n> On Tuesday 15 November 2005 10:54, Catalin Marinas wrote:\n> Actually, with .git/commits we are reimplementing handling of \"unmerged\"\n> entries... it could be better to use the \"unmerged entry\" stgit idea. So \"stg\n> resolved\" should modify the entries by itself.\n\nBut would a git-diff-tree still show the changes between the current\nfiles and the index if there are unmerged entries? I haven't tried it.\n\n> > My initial idea was to make\n> > gitmergeonefile not to leave any unmerged entries in the index. As you\n> > could see, there are cases where it failed.\n>\n> Yep... it seems you took examples from git-merge-one-file, but that's lacking\n> (but it's low-level so it's appropriate for it - it must leave unmerged\n> entries when there are conflicts).\n\nWhen I started writing StGIT, my main thoughts were driven towards the\npatch merging/commuting via the diff3 algorithm. I found it simpler to\ncopy the algorithm from git-merge-one-file since that wasn't my main\ninterest in StGIT. I also looked at how Cogito did it.\n\n> > I can see the following scenarios for a file:\n>\n> In both cases, we're going to have a conflict, so we leave file.\n> {older,remote,local} as appropriate and already done.\n>\n> > 1. deleted in the base and modified by the patch. It should leave the\n> > file in the tree together with file.older.\n>\n> Why not leaving file.remote? We already do that in general, so we have a\n> duplicate, but it's easier to understand.\n\nI agree with this.\n\n> > Another option would be to\n> > remove the file and leave both file.older and file.remote in the tree\n> > (here .remote means the version in the patch)\n>\n> I remember that at times, but .remote is very confusing... I see that's the\n> mishandling is induced by various sources, maybe including \"merge\" itself,\n> but that program (and possibly others) supports changing the labels, and this\n> should probably be done (using \"original\", \"patched\" and \"upstream\"\n> probably).\n\nI know that diff3/merge support labels. I don't exactly remember my\nreasons but I think that I chose those namings because StGIT was\nsupporting another type of merge where \"patched\" etc. did not apply.\n\nI agree that we should change them. I would rather use \"ancestor\",\n\"patch\" and \"base\" but I don't have a strong opinion.\n\n> > 2. changed in the base but deleted by the patch. It should remove the\n> > file from the tree but leave file.older and file.local. The other\n> > option is to leave the file in the tree but, as above, I prefer the\n> > first one.\n>\n> The policy about when to remove the file and when to leave it is very\n> personal... the user must anyway solve the conflict in some smart way...\n> about the defaults, anything would do, but if we really care we could leave\n> the user the choice.\n\nAt the moment, the conflicts usually leave the index in the state\nbefore pushing the patch. I think it should also leave the file and\njust mark it as conflict in .git/conflicts.\n\n--\nCatalin\n"},{"id":"12155","messageId":"437D0949.3060505@citi.umich.edu","threadId":"2487","inReplyTo":"b0943d9e0511171410y357fb0bfv@mail.gmail.com","subject":"Re: [PATCH] Stgit - gitmergeonefile.py: handle removal vs. changes","fromName":"Chuck Lever","fromEmail":"cel@citi.umich.edu","sentAt":"2005-11-17T22:50:49Z","receivedAt":"2005-11-17T22:50:49Z","isPatch":true,"sender":{"key":"cel@citi.umich.edu","avatar":null},"body":"Catalin Marinas wrote:\n> On 16/11/05, Blaisorblade <blaisorblade@yahoo.it> wrote:\n>>>Another option would be to\n>>>remove the file and leave both file.older and file.remote in the tree\n>>>(here .remote means the version in the patch)\n>>\n>>I remember that at times, but .remote is very confusing... I see that's the\n>>mishandling is induced by various sources, maybe including \"merge\" itself,\n>>but that program (and possibly others) supports changing the labels, and this\n>>should probably be done (using \"original\", \"patched\" and \"upstream\"\n>>probably).\n> \n> \n> I know that diff3/merge support labels. I don't exactly remember my\n> reasons but I think that I chose those namings because StGIT was\n> supporting another type of merge where \"patched\" etc. did not apply.\n> \n> I agree that we should change them. I would rather use \"ancestor\",\n> \"patch\" and \"base\" but I don't have a strong opinion.\n\njust a data point:\n\ni use \"original\" \"patch\" and \"older\" (set up in .stgitrc) because i \nfound the default labels to be confusing.\n\nbut \"original\" \"patch\" and \"upstream\" make sense to me.\n\n\nbegin:vcard\nfn:Chuck Lever\nn:Lever;Charles\norg:Network Appliance, Incorporated;Linux NFS Client Development\nadr:535 West William Street, Suite 3100;;Center for Information Technology Integration;Ann Arbor;MI;48103-4943;USA\nemail;internet:cel@citi.umich.edu\ntitle:Member of Technical Staff\ntel;work:+1 734 763 4415\ntel;fax:+1 734 763 4434\ntel;home:+1 734 668 1089\nx-mozilla-html:FALSE\nurl:http://www.monkey.org/~cel/\nversion:2.1\nend:vcard\n\n"},{"id":"12475","messageId":"b0943d9e0511211332h41e3850dt@mail.gmail.com","threadId":"2487","inReplyTo":"437D0949.3060505@citi.umich.edu","subject":"Re: [PATCH] Stgit - gitmergeonefile.py: handle removal vs. changes","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2005-11-21T21:32:40Z","receivedAt":"2005-11-21T21:32:40Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 17/11/05, Chuck Lever <cel@citi.umich.edu> wrote:\n> i use \"original\" \"patch\" and \"older\" (set up in .stgitrc) because i\n> found the default labels to be confusing.\n>\n> but \"original\" \"patch\" and \"upstream\" make sense to me.\n\nThese names would need to have a meaning for the result of the 'fold\n--threeway' and 'pick' commands. 'patch' and 'original' are ok but\n'upstream' might not make sense since for these commands it can\nrepresent the top of an existing patch.\n\n--\nCatalin\n"},{"id":"14102","messageId":"200512301859.51000.blaisorblade@yahoo.it","threadId":"2487","inReplyTo":"b0943d9e0511150154y2d2af24ck@mail.gmail.com","subject":"Re: [PATCH] Stgit - gitmergeonefile.py: handle removal vs. changes","fromName":"Blaisorblade","fromEmail":"blaisorblade@yahoo.it","sentAt":"2005-12-30T17:59:50Z","receivedAt":"2005-12-30T17:59:50Z","isPatch":true,"sender":{"key":"blaisorblade@yahoo.it","avatar":null},"body":"On Tuesday 15 November 2005 10:54, Catalin Marinas wrote:\n> On 13/11/05, Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it> wrote:\n> > I just got a \"removal vs. changed\" conflict, which is unhandled by StGit.\n> > That is taken from git-merge-one-file resolver, but is bad, as stg\n> > resolved does not handle unmerged entries (and probably it should be\n> > fixed too).\n>\n> I think it 'stg resolved' should be fixed as well (in case there are\n> unmerged entries for other reasons). My initial idea was to make\n> gitmergeonefile not to leave any unmerged entries in the index. As you\n> could see, there are cases where it failed.\n\nThe original patch hasn't been merged, nor (for what I see) anything else to \nfix this problem has been done.\n\nI assume the patch was lost waiting for the discussion to settle down, but the \npatch can be merged, even changing the default choices in any way (see \nbelow).\n\nAlso, another note: I just found Bruce Eckel mentioning pychecker, which is a \nstatic code checker for Python (to perform the checks a compiler would \nnormally do). I've not the time to investigate more myself, but I hope it can \nbe useful to you.\n\n> I can see the following scenarios for a file:\n\n> 1. deleted in the base and modified by the patch. It should leave the\n> file in the tree together with file.older. Another option would be to\n> remove the file and leave both file.older and file.remote in the tree\n> (here .remote means the version in the patch) but I would prefer the\n> first one.\n>\n> 2. changed in the base but deleted by the patch. It should remove the\n> file from the tree but leave file.older and file.local. The other\n> option is to leave the file in the tree but, as above, I prefer the\n> first one.\n>\n> Maybe StGIT should try to track the renaming as well but I haven't\n> played with this feature in GIT at all.\n\n-- \nInform me of my mistakes, so I can keep imitating Homer Simpson's \"Doh!\".\nPaolo Giarrusso, aka Blaisorblade (Skype ID \"PaoloGiarrusso\", ICQ 215621894)\nhttp://www.user-mode-linux.org/~blaisorblade\n\n\t\n\n\t\n\t\t\n___________________________________ \nYahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB \nhttp://mail.yahoo.it\n"},{"id":"14246","messageId":"43BFA499.3020202@gmail.com","threadId":"2487","inReplyTo":"200512301859.51000.blaisorblade@yahoo.it","subject":"Re: [PATCH] Stgit - gitmergeonefile.py: handle removal vs. changes","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2006-01-07T11:23:05Z","receivedAt":"2006-01-07T11:23:05Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"Blaisorblade wrote:\n\n>The original patch hasn't been merged, nor (for what I see) anything else to \n>fix this problem has been done.\n>  \n>\nIndeed, I forgot about it.\n\n>I assume the patch was lost waiting for the discussion to settle down, but the \n>patch can be merged, even changing the default choices in any way (see \n>below).\n>  \n>\nI merged it as it is. I will think about the default options once I get\nsome time to fix the .local, .older and .remote extensions (give them\nsome meaningful names).\n\n>Also, another note: I just found Bruce Eckel mentioning pychecker, which is a \n>static code checker for Python (to perform the checks a compiler would \n>normally do). I've not the time to investigate more myself, but I hope it can \n>be useful to you.\n>  \n>\nThanks. I'll give it a try.\n\nCatalin\n"},{"id":"14289","messageId":"43C06FF9.4070908@citi.umich.edu","threadId":"2487","inReplyTo":"43BFA499.3020202@gmail.com","subject":"Re: [PATCH] Stgit - gitmergeonefile.py: handle removal vs. changes","fromName":"Chuck Lever","fromEmail":"cel@citi.umich.edu","sentAt":"2006-01-08T01:50:49Z","receivedAt":"2006-01-08T01:50:49Z","isPatch":true,"sender":{"key":"cel@citi.umich.edu","avatar":null},"body":"Catalin Marinas wrote:\n>>Also, another note: I just found Bruce Eckel mentioning pychecker, which is a \n>>static code checker for Python (to perform the checks a compiler would \n>>normally do). I've not the time to investigate more myself, but I hope it can \n>>be useful to you.\n>> \n>>\n> \n> Thanks. I'll give it a try.\n\ni already ran pychecker against everything but gitmergeonefile.py... it\nfound a few things that you have already integrated (like that weird\nbehavior around using empty lists as the default value for function\narguments).  fyi...\n\n\nbegin:vcard\nfn:Chuck Lever\nn:Lever;Charles\norg:Network Appliance, Incorporated;Open Source NFS Client Development\nadr:535 West William Street, Suite 3100;;Center for Information Technology Integration;Ann Arbor;MI;48103-4943;USA\nemail;internet:cel@citi.umich.edu\ntitle:Member of Technical Staff\ntel;work:+1 734 763-4415\ntel;fax:+1 734 763 4434\ntel;home:+1 734 668-1089\nx-mozilla-html:FALSE\nurl:http://troy.citi.umich.edu/u/cel/\nversion:2.1\nend:vcard\n\n"}]}