{"thread":{"id":"4626","subject":"stgit: bunch of bugreports/wishes","startedAt":"2006-06-22T22:14:25Z","lastAt":"2006-08-14T19:24:22Z","messageCount":4,"participants":["Yann Dirson","Catalin Marinas"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"22314","messageId":"20060622221425.GA7851@nowhere.earth","threadId":"4626","inReplyTo":null,"subject":"stgit: bunch of bugreports/wishes","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2006-06-22T22:14:25Z","receivedAt":"2006-06-22T22:14:25Z","isPatch":false,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"Here are a number of problems I encountered while playing with\nuncommit with 0.10:\n\n- uncommit ignores grafts.  This causes \"uncommit -n\" to through\n\"graft merges\" without asking, and surely gives unexpected result\nwhen a graft is used to change an ancestor rather than adding one.\n\n- the previous behaviour causes strange interactions with merge\ncommits (eg. \"stg show\" cannot show the diff)\n\n- uncommit could allow to go through a merge (indeed ignoring grafts\nas it did was a gain of time for me, since it was in a couple of cases\nprecisely what I wanted to do ;).  One possible interface to such a\nfeature would be to allow this if HEAD is the merge, and the branch to\nfollow is explicitely specified.  The issue mentionned above seems to\nindicate that when uncommitting a merge, a new commit for the\nnewly-uncommitted patch has to be created right away, to ensure it has\na single parent.\n\n- uncommit could be more flexible to help with mass-uncommitting,\neg. with something like \"--to <commit>\" (to avoid counting manually),\nor \"--to-merge\" to cleanly stop on first merge instead of failing\nthere.  This may have an impact on how uncommits are numbered.\n\n- uncommit synopsis is incomplete (lacks \" | -n <n> <basename>\")\n\n- after mass-uncommitting, more help to look at the stack would be\nneeded.  Eg. a \"stg series\" flag to print more commit info (author,\nfiles), or to limit the listing to a given author (like \"stg patches\"\nlimits for a file).\n\n\nAdditionally, a couple of non-uncommit-related thing:\n\n- when a push is not committed because of a conflict, looking at the\nprevious diff for the patch would help.  Maybe something like \"stg\nshow --old\" ?\n\n- the help string for push should say \"patches\", and possibly document\nmore precisely the syntax, something like:\n\n--- a/stgit/commands/push.py\n+++ b/stgit/commands/push.py\n@@ -24,8 +24,8 @@ from stgit.utils import *\n from stgit import stack, git\n \n \n-help = 'push a patch on top of the series'\n-usage = \"\"\"%prog [options] [<patch1> [<patch2>...]]\n+help = 'push patches on top of the series'\n+usage = \"\"\"%prog [options] [<patch1> [<patch2>...] | -n <n> <patchroot>]\n \n Push a patch (defaulting to the first unapplied one) or range of\n patches to the stack. The 'push' operation allows patch reordering by\n\n\n- \"push --undo\" is not robust.  On the occasion reproduced below, I\nhad to rollback the push myself by hand-modifying the stgit data,\nwhich took me some effort.  I'll have to gather precise info, but the\nissue occurs on patch reordering, on a genuine conflict, and seems to\nbe involve a change to a non-existent file, when that file would have\nbeen added by a non-applied patch originally below the one I attempted\nto apply.\n\nI do agree there is a conflict, but \"push --undo\" should definitely be\nable to rollback in any case.\n\nHere is the log:\n\n========\nNow at patch \"patch9\"\nPushing patch \"patch10\"...The merge failed during \"push\". Use \"refresh\"\nafter fixing the conflicts\nstg push: GIT index merging failed (possible conflicts)\n\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\nstg push: local changes in the tree. Use \"refresh\" to commit them\n\nydirson$ stg push --undo\nUndoing the \"patch10\" push...stg push: ['git-diff-index', 'HEAD', 'path/to/the/file.java'] failed (fatal: ambiguous argument\n'path/to/the/file.java': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions)\n========\n\nBest regards,\n-- \nYann Dirson    <ydirson@altern.org> |\nDebian-related: <dirson@debian.org> |   Support Debian GNU/Linux:\n                                    |  Freedom, Power, Stability, Gratis\n     http://ydirson.free.fr/        | Check <http://www.debian.org/>\n"},{"id":"22612","messageId":"b0943d9e0606261404s38d4e316ja1af671f2d0d6e73@mail.gmail.com","threadId":"4626","inReplyTo":"20060622221425.GA7851@nowhere.earth","subject":"Re: stgit: bunch of bugreports/wishes","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2006-06-26T21:04:07Z","receivedAt":"2006-06-26T21:04:07Z","isPatch":false,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 22/06/06, Yann Dirson <ydirson@altern.org> wrote:\n> Here are a number of problems I encountered while playing with\n> uncommit with 0.10:\n\n\"uncommit\" was really intended as generating some simple patches from\na linear list of commits (maybe for undoing a \"stg commit\" or after a\ngit-am to modify some patches before pushint upstream). History\nre-writing is somehow outside StGIT's goals.\n\n> - uncommit ignores grafts.  This causes \"uncommit -n\" to through\n> \"graft merges\" without asking, and surely gives unexpected result\n> when a graft is used to change an ancestor rather than adding one.\n[...]\n\nI could fix \"uncommit\" to fail at this point but, as I said above, I\nwouldn't add extra features to this command.\n\nMaybe you can explain your workflow a bit as I don't see the need for\nmass uncommitting.\n\n> - uncommit could be more flexible to help with mass-uncommitting,\n> eg. with something like \"--to <commit>\" (to avoid counting manually),\n> or \"--to-merge\" to cleanly stop on first merge instead of failing\n> there.  This may have an impact on how uncommits are numbered.\n>\n> - uncommit synopsis is incomplete (lacks \" | -n <n> <basename>\")\n>\n> - after mass-uncommitting, more help to look at the stack would be\n> needed.  Eg. a \"stg series\" flag to print more commit info (author,\n> files), or to limit the listing to a given author (like \"stg patches\"\n> limits for a file).\n\nThese would be good indeed. I also had a plan to generate the patch\nname from the subject line (i.e. replacing the spaces with a dash) to\nbe more meaningful. But got really busy with my job recently and\ndidn't have time.\n\n> - when a push is not committed because of a conflict, looking at the\n> previous diff for the patch would help.  Maybe something like \"stg\n> show --old\" ?\n\n\"stg show <patch>//top.old\" should show it (well, with a bit more\ntyping than --old).\n\n> - the help string for push should say \"patches\", and possibly document\n> more precisely the syntax, something like:\n\nI plan to change the syntax of push a bit to allow things like\npatch1..patch2 without the --to option (the latter would still be\nthere but taking a single patch).\n\n> -help = 'push a patch on top of the series'\n> -usage = \"\"\"%prog [options] [<patch1> [<patch2>...]]\n> +help = 'push patches on top of the series'\n> +usage = \"\"\"%prog [options] [<patch1> [<patch2>...] | -n <n> <patchroot>]\n\nDoes the <patchroot> syntax work?\n\n> - \"push --undo\" is not robust.  On the occasion reproduced below, I\n> had to rollback the push myself by hand-modifying the stgit data,\n> which took me some effort.  I'll have to gather precise info, but the\n> issue occurs on patch reordering, on a genuine conflict, and seems to\n> be involve a change to a non-existent file, when that file would have\n> been added by a non-applied patch originally below the one I attempted\n> to apply.\n[...]\n> ydirson$ stg push --undo\n> Undoing the \"patch10\" push...stg push: ['git-diff-index', 'HEAD', 'path/to/the/file.java'] failed (fatal: ambiguous argument\n> 'path/to/the/file.java': unknown revision or path not in the working tree.\n> Use '--' to separate paths from revisions)\n\nI got this problem as well. StGIT needs fixing but I think a quick\nworkaround is to create an empty file (touch patch/to/the/file.java)\nbefore the undo and git-diff-index will be happy.\n\nThanks for the bug reports/suggestions.\n\n-- \nCatalin\n"},{"id":"23279","messageId":"b0943d9e0607050729v75a5e6deq6baff0350b422122@mail.gmail.com","threadId":"4626","inReplyTo":"20060622221425.GA7851@nowhere.earth","subject":"Re: stgit: bunch of bugreports/wishes","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2006-07-05T14:29:15Z","receivedAt":"2006-07-05T14:29:15Z","isPatch":false,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 22/06/06, Yann Dirson <ydirson@altern.org> wrote:\n> - \"push --undo\" is not robust.  On the occasion reproduced below, I\n> had to rollback the push myself by hand-modifying the stgit data,\n> which took me some effort.  I'll have to gather precise info, but the\n> issue occurs on patch reordering, on a genuine conflict, and seems to\n> be involve a change to a non-existent file, when that file would have\n> been added by a non-applied patch originally below the one I attempted\n> to apply.\n\nI fixed this bug and I'll push the changes tonight. It was caused by a\n\"git-diff-index\" call with files but without the \"--\" argument. When\nthe file was actually missing from the repository, git was confused\nand returned an error. The 'status --reset' command was failing in\nthis situation as well.\n\n-- \nCatalin\n"},{"id":"25278","messageId":"20060814192422.GA20336@nan92-1-81-57-214-146.fbx.proxad.net","threadId":"4626","inReplyTo":"b0943d9e0606261404s38d4e316ja1af671f2d0d6e73@mail.gmail.com","subject":"Re: stgit: bunch of bugreports/wishes","fromName":"Yann Dirson","fromEmail":"dwitch@nan92-1-81-57-214-146.fbx.proxad.net","sentAt":"2006-08-14T19:24:22Z","receivedAt":"2006-08-14T19:24:22Z","isPatch":false,"sender":{"key":"dwitch@nan92-1-81-57-214-146.fbx.proxad.net","avatar":null},"body":"Hi Catalin,\n\nI was a bit out of time lately, following the birth of my 3rd child.\nNow seems like a good moment to followup on this issue.\n\nOn Mon, Jun 26, 2006 at 10:04:07PM +0100, Catalin Marinas wrote:\n> On 22/06/06, Yann Dirson <ydirson@altern.org> wrote:\n> >Here are a number of problems I encountered while playing with\n> >uncommit with 0.10:\n> \n> \"uncommit\" was really intended as generating some simple patches from\n> a linear list of commits (maybe for undoing a \"stg commit\" or after a\n> git-am to modify some patches before pushint upstream). History\n> re-writing is somehow outside StGIT's goals.\n\nBut uncommitting commits to an stgit stack is surely rarely done without\ntaking advantage of stgit main features afterwards, and that is history\nrewriting anyway.  It's just another way of looking at it :)\n\n\n> >- uncommit ignores grafts.  This causes \"uncommit -n\" to through\n> >\"graft merges\" without asking, and surely gives unexpected result\n> >when a graft is used to change an ancestor rather than adding one.\n> [...]\n> \n> I could fix \"uncommit\" to fail at this point but, as I said above, I\n> wouldn't add extra features to this command.\n\nThis is most probably fixed by one of the 2 patches I just posted.  The\nproblem is not specific to uncommit at all, it impacts all commands\nneeding to look at the parents of a commit.\n\nBTW, while I'm at it - I only changed the commit constructor to stop\nreading from the commit object for parent identification, there may be\nsimilar standard ways to access the other informations.\n\n \n> Maybe you can explain your workflow a bit as I don't see the need for\n> mass uncommitting.\n\nThe team is using CVS for history, and I'm working on a git mirror of\nthat repository.  In this repository we're importing upstream sources on\na branch, developping our own stuff in the CVS trunk, merging new\nupstream versions as they are shipped to us.\n\nThen we ship some of our changes upstream, and this is where I'd like to\nuse the power of stgit, to change the cvs history into a set of\ncoherent, feature-sized patches, to be fed upstream.\n\nI manually maintain a grafts file to record the merges (and occasionally\nget rid of cvs artifacts in the imported history), so my tree is full\nof merges and grafts.\n\n\nMore recently, the work I've started today is about similarly\nreorganizing the history of a linux-2.4 BSP for a custom board, to make\nit more easy to maintain it.  In that case, I've taken a slightly different\napproach, since I knew \"uncommit\" would not help: I have started a new\nstgit branch, and \"stgit pick\"'d the individual patches from various\nbranches - but that was only reasonable because the number of patches\nwas low.\n\n\n> >- when a push is not committed because of a conflict, looking at the\n> >previous diff for the patch would help.  Maybe something like \"stg\n> >show --old\" ?\n> \n> \"stg show <patch>//top.old\" should show it (well, with a bit more\n> typing than --old).\n\nHm, that does not seem to work with 0.10:\n\n$ stg show pick-parent//top.old\nstg show: Unknown revision: pick-parent//top.old^{commit}\n\n\n> >-help = 'push a patch on top of the series'\n> >-usage = \"\"\"%prog [options] [<patch1> [<patch2>...]]\n> >+help = 'push patches on top of the series'\n> >+usage = \"\"\"%prog [options] [<patch1> [<patch2>...] | -n <n> <patchroot>]\n> \n> Does the <patchroot> syntax work?\n\nI mean, that when using -n, the string is not a patchname, but a root\nthat gets suffixed by numbers, and in that case only one name can be\ngiven.\n\n\nWhile I'm at it, here is another list of problems I hit today with 0.10:\n\n- \"stg fold\" short usage does not tell what <file> is used for                                                                                                                       \n- I did not find a clean way to fold into current patch only the part of a patch\n  impacted a given file.  Whereas I can use \"stg show | filterdiff | stg\n  fold\" for file modifications, that fails to work for permission\n  changes and the like.\n  \n- \"stg patches\" only reports about currently applied patches, it would\n  be very useful to report on unapplied patches as well\n\n- \"stg patches -d\" is broken, but that may be fixed in HEAD already\n  (missing -- when calling git-diff-tree)\n\n- commit.get_parent() silently returns 1st parent when there are several\n  ones.  I think it should raise an exception instead, and callers must\n  guard against failure.\n\n- pick badly fails when hitting conflicts.  This is especially confusing when the\n  conflicts only comes from grafts being ignored.  It is especially bad when using\n  \"pick\" to create the 1st patch in a series : then \"stg delete\" on the partially\n  created patch cannot recover the failure (IIRC it complains there is\n  no current patch)\n\n- Looks like trying to resolve a conflict by removing the file currently require\n  a precise ordering of operations, or we need manual recovering.  In\n  the following example, the mach-type.h files was removed in a patch\n  below, and the conflict occurs because the current patch was modifying\n  it.  This may as well be fixed by the -- in diff-index call, but I'd\n  rather report it in case there is another remaining issue:\n\n$ stg status\nC include/asm-arm/mach-types.h\n[...]\n$ stg rm include/asm-arm/mach-types.h\n$ stg resolved\ninclude/asm-arm/mach-types.h\nstg resolved: ['git-diff-index', 'HEAD', 'include/asm-arm/mach-types.h']\nfailed (fatal: ambiguous argument 'include/asm-arm/mach-types.h':\nunknown\nrevision or path not in the working tree.\nUse '--' to separate paths from revisions)\nydirson@unx-4791:linux-git[906]$ stg status\nC include/asm-arm/mach-types.h\n\nEmergency fixup:\n\n$ :>include/asm-arm/mach-types.h\n$ cg-add include/asm-arm/mach-types.h\nAdding file include/asm-arm/mach-types.h\n$ stg resolved\ninclude/asm-arm/mach-types.h\n$ cg-rm include/asm-arm/mach-types.h\nRemoving file include/asm-arm/mach-types.h\n\n\n- \"pick --fold\" embeds \\n's in sha1's ?  (this needs to be checked, it\n  could be a side-effect of my initial implementation of using rev-list\n  to identify parents)\n\n$ ~/tools/git/stgit-0.10/stg pick --fold\n8d8ecef22870e6a6b063d95d9b90d69e8391a0d0\nFolding commit 8d8ecef22870e6a6b063d95d9b90d69e8391a0d0...stg pick:\n['git-diff-tree', '-p', 'de47893112debe4cd3871162a0d1df0b73000e94\\n',\n'8d8ecef22870e6a6b063d95d9b90d69e8391a0d0'] failed (fatal: ambiguous\nargument 'de47893112debe4cd3871162a0d1df0b73000e94\n': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions)\n\nwhereas:\n\n$ ~/tools/git/stgit-0.10/stg pick\n8d8ecef22870e6a6b063d95d9b90d69e8391a0d0 -n import-fix\nImporting commit 8d8ecef22870e6a6b063d95d9b90d69e8391a0d0... done\nNow at patch \"import-fix\"\n\nBest regards,\n-- \nYann.\n"}]}