{"thread":{"id":"29203","subject":"git-p4: labels","startedAt":"2011-12-18T18:33:39Z","lastAt":"2011-12-20T12:03:28Z","messageCount":4,"participants":["Luke Diamand","Vitor Antunes","Pete Wyckoff"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"181442","messageId":"4EEE3203.10605@diamand.org","threadId":"29203","inReplyTo":null,"subject":"git-p4: labels","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-12-18T18:33:39Z","receivedAt":"2011-12-18T18:33:39Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"I've been looking at fixing the --detect-labels flag in git-p4. I'm now \nthinking that the current way it's done is just all wrong.\n\nThe current code has a few annoying bugs which I mostly have fixes for:\n\n- it only works when there is no more than one label per changelist;\n\n- if the count of files in the p4 label doesn't match the count of files \nin the p4 changelist then the label gets dropped (by design);\n\n- git-p4 rebase ignores --detect-labels\n\n- it imports all the label/file mappings each time (I haven't fixed this \nyet)\n\nHowever, the thing that's more annoying is that it only imports labels \nwhen importing the changelist they are pointing at.\n\nSo, for example, if you do something like this:\n\np4 edit; p4 submit\np4 tag TAG1\ngit-p4 rebase\np4 tag TAG2\ngit-p4 rebase\n\nthen TAG1 will be detected, but TAG2 won't.\n\nThis is a particular nuisance for me, as I just have git-p4 running all \nthe time eating commits, so there's no scope for a human to delay the \ngit-p4 and pickup the label.\n\nI think what's needed is to do something completely different.\n\nThis is speculation at the moment, and I'd welcome comments before I \nstart hacking.\n\nThere will be a new _command_, import-labels. This command will find all \nthe labels that git knows about, and the labels that p4 knows about, and \nthen do the obvious diff. It will then create tags as needed directly \n(i.e. not using fast-import).\n\nIt will unfortunately need to hunt through the revision history looking \nfor the git hash <h> that matches p4 changelist <c>. This could get \nquite slow, although no worse than O(n).\n\nSomething like:\n\np4 edit; p4 submit\np4 tag TAG1\ngit-p4 rebase\ngit-p4 import-labels\np4 tag TAG2\ngit-p4 import-labels\n\nI'm going to try to work up a patch with some test cases but in the \nmeantime if anyone thinks I've missed something, please let me know.\n\nThanks\nLuke\n"},{"id":"181461","messageId":"CAOpHH-W1hE6bw51NKXrdf1QtF3T+v4aLwxMn+_5xPP5j=uJr3w@mail.gmail.com","threadId":"29203","inReplyTo":"4EEE3203.10605@diamand.org","subject":"Re: git-p4: labels","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2011-12-19T10:28:18Z","receivedAt":"2011-12-19T10:28:18Z","isPatch":false,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"On Sun, Dec 18, 2011 at 6:33 PM, Luke Diamand <luke@diamand.org> wrote:\n> I've been looking at fixing the --detect-labels flag in git-p4. I'm now\n> thinking that the current way it's done is just all wrong.\n>\n> The current code has a few annoying bugs which I mostly have fixes for:\n>\n> - it only works when there is no more than one label per changelist;\n>\n> - if the count of files in the p4 label doesn't match the count of files in\n> the p4 changelist then the label gets dropped (by design);\n>\n> - git-p4 rebase ignores --detect-labels\n>\n> - it imports all the label/file mappings each time (I haven't fixed this\n> yet)\n>\n> However, the thing that's more annoying is that it only imports labels when\n> importing the changelist they are pointing at.\n>\n> So, for example, if you do something like this:\n>\n> p4 edit; p4 submit\n> p4 tag TAG1\n> git-p4 rebase\n> p4 tag TAG2\n> git-p4 rebase\n>\n> then TAG1 will be detected, but TAG2 won't.\n>\n> This is a particular nuisance for me, as I just have git-p4 running all the\n> time eating commits, so there's no scope for a human to delay the git-p4 and\n> pickup the label.\n>\n> I think what's needed is to do something completely different.\n>\n> This is speculation at the moment, and I'd welcome comments before I start\n> hacking.\n>\n> There will be a new _command_, import-labels. This command will find all the\n> labels that git knows about, and the labels that p4 knows about, and then do\n> the obvious diff. It will then create tags as needed directly (i.e. not\n> using fast-import).\n>\n> It will unfortunately need to hunt through the revision history looking for\n> the git hash <h> that matches p4 changelist <c>. This could get quite slow,\n> although no worse than O(n).\n>\n> Something like:\n>\n> p4 edit; p4 submit\n> p4 tag TAG1\n> git-p4 rebase\n> git-p4 import-labels\n> p4 tag TAG2\n> git-p4 import-labels\n>\n> I'm going to try to work up a patch with some test cases but in the meantime\n> if anyone thinks I've missed something, please let me know.\n\nHi Luke,\n\nPersonally, I would prefer to keep the label import working together\nwith git-p4 rebase/sync so that it doesn't need to go over P4 history\ntwice. The sanity check currently implemented seems completely bogus. In\nmy opinion that check should be completely rewritten (maybe move it into\na specific class method?). On a first approach it should at least check\nif all files in the label exist in the repository/branch. Ideally, if\nnot all repository/branch files are labelled, then a special branch\nshould be created that would not contain those files and the label would\nbe applied to that. I'm not sure what to do in the case where the label\ncontains files that are not part of the repository/branch... maybe we\ncan simply ignore them with a Warning?\n\nIf you are really sure that this is not the way to go and that the\nimport-labels command needs to be created, then I would advise you to\nsearch P4 history backwards until you find a label that was already\nimported. This way the script only has to go over P4 history until the\nlast time import-labels was run. Of course, it still needs to do this\nfor all branches (hope you are not forgeting them ;).\n\n-- \nVitor Antunes\n"},{"id":"181511","messageId":"20111220014753.GD20591@padd.com","threadId":"29203","inReplyTo":"CAOpHH-W1hE6bw51NKXrdf1QtF3T+v4aLwxMn+_5xPP5j=uJr3w@mail.gmail.com","subject":"Re: git-p4: labels","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-12-20T01:47:53Z","receivedAt":"2011-12-20T01:47:53Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"vitor.hda@gmail.com wrote on Mon, 19 Dec 2011 10:28 +0000:\n> On Sun, Dec 18, 2011 at 6:33 PM, Luke Diamand <luke@diamand.org> wrote:\n> > I've been looking at fixing the --detect-labels flag in git-p4. I'm now\n> > thinking that the current way it's done is just all wrong.\n> >\n> > The current code has a few annoying bugs which I mostly have fixes for:\n> >\n> > - it only works when there is no more than one label per changelist;\n> >\n> > - if the count of files in the p4 label doesn't match the count of files in\n> > the p4 changelist then the label gets dropped (by design);\n> >\n> > - git-p4 rebase ignores --detect-labels\n> >\n> > - it imports all the label/file mappings each time (I haven't fixed this\n> > yet)\n> >\n> > However, the thing that's more annoying is that it only imports labels when\n> > importing the changelist they are pointing at.\n> >\n> > So, for example, if you do something like this:\n> >\n> > p4 edit; p4 submit\n> > p4 tag TAG1\n> > git-p4 rebase\n> > p4 tag TAG2\n> > git-p4 rebase\n> >\n> > then TAG1 will be detected, but TAG2 won't.\n> >\n> > This is a particular nuisance for me, as I just have git-p4 running all the\n> > time eating commits, so there's no scope for a human to delay the git-p4 and\n> > pickup the label.\n> >\n> > I think what's needed is to do something completely different.\n> >\n> > This is speculation at the moment, and I'd welcome comments before I start\n> > hacking.\n> >\n> > There will be a new _command_, import-labels. This command will find all the\n> > labels that git knows about, and the labels that p4 knows about, and then do\n> > the obvious diff. It will then create tags as needed directly (i.e. not\n> > using fast-import).\n> >\n> > It will unfortunately need to hunt through the revision history looking for\n> > the git hash <h> that matches p4 changelist <c>. This could get quite slow,\n> > although no worse than O(n).\n> >\n> > Something like:\n> >\n> > p4 edit; p4 submit\n> > p4 tag TAG1\n> > git-p4 rebase\n> > git-p4 import-labels\n> > p4 tag TAG2\n> > git-p4 import-labels\n> >\n> > I'm going to try to work up a patch with some test cases but in the meantime\n> > if anyone thinks I've missed something, please let me know.\n> \n> Hi Luke,\n> \n> Personally, I would prefer to keep the label import working together\n> with git-p4 rebase/sync so that it doesn't need to go over P4 history\n> twice. The sanity check currently implemented seems completely bogus. In\n> my opinion that check should be completely rewritten (maybe move it into\n> a specific class method?). On a first approach it should at least check\n> if all files in the label exist in the repository/branch. Ideally, if\n> not all repository/branch files are labelled, then a special branch\n> should be created that would not contain those files and the label would\n> be applied to that. I'm not sure what to do in the case where the label\n> contains files that are not part of the repository/branch... maybe we\n> can simply ignore them with a Warning?\n\nAgree we can at least keep what's currently working, especially\nwith the bug fixes mentioned.  It's pretty blatantly broken in\nthose cases.  I imagine for some flows, like where labels are old\ncompared to git checkouts, it's okay.\n\nBut adding a new command to go back and look for _new_ labels\nmakes sense too.  Finding the new ones isn't so bad, given that\np4 can just print them directly, and we keep them as tags in git.\n\n> If you are really sure that this is not the way to go and that the\n> import-labels command needs to be created, then I would advise you to\n> search P4 history backwards until you find a label that was already\n> imported. This way the script only has to go over P4 history until the\n> last time import-labels was run. Of course, it still needs to do this\n> for all branches (hope you are not forgeting them ;).\n\nYeah.  That's probably a good bet.  Find the tag that points to\nthe most recent change number and stop there.\n\nI'm not too worried about walking backwards looking for a given\nchange number either.  You could even bisect if you feel clever.  :)\nOff and on, it has felt like a good idea to keep that mapping\npermanent somewhere, matching up tags to change numbers.  This\nhas come up on the list outside of p4 discussions too.\n\nBut isn't there a step in label detection that looks an awful lot\nlike branch point detection?  You've got a label, which is a\nbunch of files, not a p4 change number.  Now you have to figure\nout the change number so you can go hunt that down in the git\nhistory.  Vitor's take on this was to use git diff machinery to\nfind it, not trawling through the p4 change/filelog/describe\nhistory, mainly because it's likely much faster to do it locally\nin git.\n\n\t\t-- Pete\n"},{"id":"181524","messageId":"CAOpHH-VUJBU5ygabw7YS_FHz6xMDtMkdWMNdH3Mnvp+Crbwh7Q@mail.gmail.com","threadId":"29203","inReplyTo":"20111220014753.GD20591@padd.com","subject":"Re: git-p4: labels","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2011-12-20T12:03:28Z","receivedAt":"2011-12-20T12:03:28Z","isPatch":false,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"On Tue, Dec 20, 2011 at 1:47 AM, Pete Wyckoff <pw@padd.com> wrote:\n> But adding a new command to go back and look for _new_ labels\n> makes sense too.  Finding the new ones isn't so bad, given that\n> p4 can just print them directly, and we keep them as tags in git.\n\nNow that you say that, it is now very difficult to detect branches on an\nalready imported repository. Maybe the new command could also do that?\nIn this case I would call it \"git-p4 resync\".\n\n> [...]\n> But isn't there a step in label detection that looks an awful lot\n> like branch point detection?  You've got a label, which is a\n> bunch of files, not a p4 change number.  Now you have to figure\n> out the change number so you can go hunt that down in the git\n> history.  Vitor's take on this was to use git diff machinery to\n> find it, not trawling through the p4 change/filelog/describe\n> history, mainly because it's likely much faster to do it locally\n> in git.\n\nIn labels this is easier and fast:\n\np4 changes -m 1 @label_name\n\nThen we could diff the label against the changelist. If equal simply tag\nthe commit, if different then we need to create a temporary branch to\ncommit the changes and tag that instead.\n\n-- \nVitor Antunes\n"}]}