{"thread":{"id":"43386","subject":"Re: \"stgit clean\" has problems with removed generated files","startedAt":"2006-11-23T16:11:35Z","lastAt":"2006-12-06T13:12:53Z","messageCount":5,"participants":["Catalin Marinas","Yann Dirson","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"297489","messageId":"20061123161135.GK5443@nan92-1-81-57-214-146.fbx.proxad.net","threadId":"43386","inReplyTo":null,"subject":"\"stgit clean\" has problems with removed generated files","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2006-11-23T16:11:35Z","receivedAt":"2006-11-23T16:11:35Z","isPatch":false,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"I recently encountered a problem with \"stgit clean\", but it really is a\ngeneric issue, which can also impact other subcommands.\n\nIn a kernel tree, the precise problem I had is due to generated files\ncommitted by error in an upstream branch (a BSP from a well-known\nvendor, indeed ;). The first patch in my stgir stack does some cleanup\nby removing them from git control (so that \"make dep\" does not cause\nthem to change every so often).\n\nNow when I want to run \"stg clean\" for applied patches, stgit first\nwants to pop the whole stack, including that patch, which triggers the\nfollowing error:\n\nfatal: Untracked working tree file 'include/asm-arm/constants.h' would be overwritten by merge. stg clean: git-read-tree failed (local changes maybe?) \n\n\nObviously, removing all such files by hand allows to run \"stg clean\", as\ndoes (floating and) popping that patch and deleting it, or running \"stg clean\n--unappplied\". The 1st approach is intrusive, and forces the user to\nrerun \"make dep\", which is not very friendy; the 2nd one is too error-prone for\nan operation that ought to be risk-less (delete has no --undo). The 3rd option\nis probably the best, but is surely not so intuitive.\n\nThe root issue seems to be that stgit has problem with such generated\nfiles, ie., files that were removed from version-control, but can still\nlegitimately exist in the tree. Dealing with them could possibly be\ndone (eg. allowing to back them up, and restore them when pushing the\nannoying patch), but is not a trivial issue (eg. we still need to guard\nthe user against real conflicts).\n\nAt least we could put a notice about this case in the doc, so users\ndon't get too surprised.\n\n\nI could think of several (non-exclusive) possibilities to deal with the\nprecise problem I got, which should allow.\n\nFirst, when cleaning patches, we could first look up which patches are\nto be removed, and only pop the necessary ones.\n\nSecond, we could generalize the \"clean\" subcommand to accept arbitrary\nranges, not only the \"applied\" and \"unapplied\" ones. A special case\nwould be \"stg clean that-patch\", which would be a secure version of \"stg\ndelete\".\n\nBTW, maybe it would those make sense to allowthose special ranges in\nmost places a range is valid.\n\n\nBest regards,\n-- \n"},{"id":"298212","messageId":"b0943d9e0611230833y5ffc14d3q922a821744ad05d@mail.gmail.com","threadId":"43386","inReplyTo":"20061123161135.GK5443@nan92-1-81-57-214-146.fbx.proxad.net","subject":"Re: \"stgit clean\" has problems with removed generated files","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2006-11-23T16:33:42Z","receivedAt":"2006-11-23T16:33:42Z","isPatch":false,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 23/11/06, Yann Dirson wrote:\n> In a kernel tree, the precise problem I had is due to generated files\n> committed by error in an upstream branch (a BSP from a well-known\n> vendor, indeed ;). The first patch in my stgir stack does some cleanup\n> by removing them from git control (so that \"make dep\" does not cause\n> them to change every so often).\n>\n> Now when I want to run \"stg clean\" for applied patches, stgit first\n> wants to pop the whole stack, including that patch, which triggers the\n> following error:\n>\n> fatal: Untracked working tree file 'include/asm-arm/constants.h' would be overwritten by merge.\n\nThat's a git error and I think it is the correct behaviour. It is\nsafer to notify that a local file is overridden by a merge/switch\noperation rather than just losing its content.\n\n> Obviously, removing all such files by hand allows to run \"stg clean\", as\n> does (floating and) popping that patch and deleting it, or running \"stg clean\n> --unappplied\".\n\nMaybe 'stg clean' should only pop to the first empty applied patch\nrather than popping all of them as it is also more efficient.\n\n> The root issue seems to be that stgit has problem with such generated\n> files, ie., files that were removed from version-control, but can still\n> legitimately exist in the tree. Dealing with them could possibly be\n> done (eg. allowing to back them up, and restore them when pushing the\n> annoying patch), but is not a trivial issue (eg. we still need to guard\n> the user against real conflicts).\n\nThat's a GIT issue more than an StGIT one, unless GIT already has a\nway to deal with this and StGIT doesn't pass the right options.\n\n> First, when cleaning patches, we could first look up which patches are\n> to be removed, and only pop the necessary ones.\n\nOK, I mentioned it above as well. This should really be done for\nefficiency but it might not solve the problem if the empty patch is\ndeeper into the stack.\n\n> Second, we could generalize the \"clean\" subcommand to accept arbitrary\n> ranges, not only the \"applied\" and \"unapplied\" ones. A special case\n> would be \"stg clean that-patch\", which would be a secure version of \"stg\n> delete\".\n\nEasy to fix as well.\n\n> BTW, maybe it would those make sense to allowthose special ranges in\n> most places a range is valid.\n\nIs there any other place where ranges could be used but aren't?\n\n-- \n"},{"id":"296605","messageId":"20061123192831.GL5443@nan92-1-81-57-214-146.fbx.proxad.net","threadId":"43386","inReplyTo":"b0943d9e0611230833y5ffc14d3q922a821744ad05d@mail.gmail.com","subject":"Re: \"stgit clean\" has problems with removed generated files","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2006-11-23T19:28:31Z","receivedAt":"2006-11-23T19:28:31Z","isPatch":false,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"On Thu, Nov 23, 2006 at 04:33:42PM +0000, Catalin Marinas wrote:\n> >fatal: Untracked working tree file 'include/asm-arm/constants.h' would be \n> >overwritten by merge.\n> \n> That's a git error and I think it is the correct behaviour. It is\n> safer to notify that a local file is overridden by a merge/switch\n> operation rather than just losing its content.\n\nRight, I do not discuss the behaviour of git here. But when I first\nencountered this issue, I was really wondering about what was\nhappenning. It would be really helpful in such a case, if stgit was\nable to pinpoint the precise patch which could not be popped. It could\nalso be helpful to tell when popping patches - currently it's done\n\"behind my back\", and I could only understand what was happenning by\nreading the code.\n\n\n> >The root issue seems to be that stgit has problem with such generated\n> >files, ie., files that were removed from version-control, but can still\n> >legitimately exist in the tree. Dealing with them could possibly be\n> >done (eg. allowing to back them up, and restore them when pushing the\n> >annoying patch), but is not a trivial issue (eg. we still need to guard\n> >the user against real conflicts).\n> \n> That's a GIT issue more than an StGIT one, unless GIT already has a\n> way to deal with this and StGIT doesn't pass the right options.\n\nI'm not sure if git itself should add support for this. IMHO, such an issue\nis more related to the nature of patch management - git itself does not\nhave a need to repetedly pop/push paches, with related merge\nhassles like this one.\n\n\n> >First, when cleaning patches, we could first look up which patches are\n> >to be removed, and only pop the necessary ones.\n> \n> OK, I mentioned it above as well. This should really be done for\n> efficiency but it might not solve the problem if the empty patch is\n> deeper into the stack.\n\nRight, it could be advertised as good practice to burry those near the\nbottom of the stack, and anyway to keep actively-changing ones near the\ntop.\n\n\n> >BTW, maybe it would those make sense to allowthose special ranges in\n> >most places a range is valid.\n> \n> Is there any other place where ranges could be used but aren't?\n\nHm, let's see... I'd say \"export\" (I have missed it already), \"files\"\nand maybe \"commit\" and \"pick\", although the latter would require a syntax\nfor ranges in other branch.\n\nWhile reviewing the various commands for this, I realized that \"stg pop\n\" semantics is significantly different from \"stg push \" -\nie. it is an equivalent of \"goto\". What about turning it into a\nfloat+pop, to better match the \"push\" behaviour ?\n\nI also realized that \"stg help \" output does not go through to\nthe pager, while eg. the help for \"mail\" is quite long.\n\nBest regards,\n-- \n"},{"id":"296756","messageId":"ek69tf$bp9$1@sea.gmane.org","threadId":"43386","inReplyTo":"20061123192831.GL5443@nan92-1-81-57-214-146.fbx.proxad.net","subject":"Re: \"stgit clean\" has problems with removed generated files","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-11-24T08:20:35Z","receivedAt":"2006-11-24T08:20:35Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Yann Dirson wrote:\n\n> On Thu, Nov 23, 2006 at 04:33:42PM +0000, Catalin Marinas wrote:\n>> >fatal: Untracked working tree file 'include/asm-arm/constants.h' would be \n>> >overwritten by merge.\n>> \n>> That's a git error and I think it is the correct behaviour. It is\n>> safer to notify that a local file is overridden by a merge/switch\n>> operation rather than just losing its content.\n> \n> Right, I do not discuss the behaviour of git here. But when I first\n> encountered this issue, I was really wondering about what was\n> happenning. It would be really helpful in such a case, if stgit was\n> able to pinpoint the precise patch which could not be popped. It could\n> also be helpful to tell when popping patches - currently it's done\n> \"behind my back\", and I could only understand what was happenning by\n> reading the code.\n\nI think it was corrected in git, or is being corrected (meaning it is\nin 'next' or in 'pu'); it being relaxing \"would be overwritten by merge\"\ncheck for files which were, but are not, under version control.\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n\n"},{"id":"294581","messageId":"b0943d9e0612060512w272d656dwa3ef5b6875abf8fa@mail.gmail.com","threadId":"43386","inReplyTo":"20061123192831.GL5443@nan92-1-81-57-214-146.fbx.proxad.net","subject":"Re: \"stgit clean\" has problems with removed generated files","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2006-12-06T13:12:53Z","receivedAt":"2006-12-06T13:12:53Z","isPatch":false,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 23/11/06, Yann Dirson wrote:\n> On Thu, Nov 23, 2006 at 04:33:42PM +0000, Catalin Marinas wrote:\n> > >First, when cleaning patches, we could first look up which patches are\n> > >to be removed, and only pop the necessary ones.\n\nI fixed this.\n\n> > Is there any other place where ranges could be used but aren't?\n>\n> Hm, let's see... I'd say \"export\" (I have missed it already), \"files\"\n> and maybe \"commit\" and \"pick\", although the latter would require a syntax\n> for ranges in other branch.\n\nI changed the export command to take a range of patches as arguments\n(and a --dir option for the export directory).\n\n> While reviewing the various commands for this, I realized that \"stg pop\n> \" semantics is significantly different from \"stg push \" -\n> ie. it is an equivalent of \"goto\". What about turning it into a\n> float+pop, to better match the \"push\" behaviour ?\n\nThanks for the suggestion, I fixed pop to take patch ranges now.\n\n> I also realized that \"stg help \" output does not go through to\n> the pager, while eg. the help for \"mail\" is quite long.\n\nAdded this feature as well. I'll try to push the changes tonight.\n\n-- \n"}]}