{"thread":{"id":"28682","subject":"[PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.","startedAt":"2011-10-14T21:51:50Z","lastAt":"2011-10-17T22:37:31Z","messageCount":8,"participants":["Andrei Warkentin","Tor Arvid Lund","Luke Diamand","Pete Wyckoff"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"177686","messageId":"1318629110-15232-1-git-send-email-andreiw@vmware.com","threadId":"28682","inReplyTo":null,"subject":"[PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.","fromName":"Andrei Warkentin","fromEmail":"andreiw@vmware.com","sentAt":"2011-10-14T21:51:50Z","receivedAt":"2011-10-14T21:51:50Z","isPatch":true,"sender":{"key":"andreiw@vmware.com","avatar":null},"body":"Many users of p4/sd use changelists for review, regression\ntests and batch builds, thus changes are almost never directly\nsubmitted.\n\nThis new config option lets a 'p4 change -i' run instead of\nthe 'p4 submit -i'.\n\nSigned-off-by: Andrei Warkentin \n---\n contrib/fast-import/git-p4 | 16 ++++++++++++----\n contrib/fast-import/git-p4.txt | 10 ++++++++++\n 2 files changed, 22 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 2f7b270..19c295b 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -959,7 +959,10 @@ class P4Submit(Command, P4UserMap):\n submitTemplate = message[:message.index(separatorLine)]\n if self.isWindows:\n submitTemplate = submitTemplate.replace(\"\\r\\n\", \"\\n\")\n- p4_write_pipe(\"submit -i\", submitTemplate)\n+ if gitConfig(\"git-p4.changeOnSubmit\"):\n+ p4_write_pipe(\"change -i\", submitTemplate)\n+ else:\n+ p4_write_pipe(\"subadasdmit -i\", submitTemplate)\n \n if self.preserveUser:\n if p4User:\n@@ -981,9 +984,14 @@ class P4Submit(Command, P4UserMap):\n file = open(fileName, \"w+\")\n file.write(self.prepareLogMessage(template, logMessage))\n file.close()\n- print (\"Perforce submit template written as %s. \"\n- + \"Please review/edit and then use p4 submit -i < %s to submit directly!\"\n- % (fileName, fileName))\n+ if gitConfig(\"git-p4.changeOnSubmit\"):\n+ print (\"Perforce submit template written as %s. \"\n+ + \"Please review/edit and then use p4 change -i < %s to create changelist!\"\n+ % (fileName, fileName))\n+ else:\n+ print (\"Perforce submit template written as %s. \"\n+ + \"Please review/edit and then use p4 submit -i < %s to submit directly!\"\n+ % (fileName, fileName))\n \n def run(self, args):\n if len(args) == 0:\ndiff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt\nindex 52003ae..3a3a815 100644\n--- a/contrib/fast-import/git-p4.txt\n+++ b/contrib/fast-import/git-p4.txt\n@@ -180,6 +180,16 @@ git-p4.allowSubmit\n \n git config [--global] git-p4.allowSubmit false\n \n+git-p4.changeOnSubmit\n+\n+ git config [--global] git-p4.changeOnSubmit false\n+\n+Most places using p4/sourcedepot don't actually want you submit\n+changes directly, and changelists are used to do regression testing,\n+batch builds and review, hence, by setting this parameter to\n+true you acknowledge you end up creating a changelist which you\n+must then manually commit.\n+\n git-p4.syncFromOrigin\n \n A useful setup may be that you have a periodically updated git repository\n-- \n1.7.4.1\n"},{"id":"177688","messageId":"CA+DMoH-HqA0DCyUSttO-iYO0rUHq1nLqM9W0imAOjHC5H1r_9w@mail.gmail.com","threadId":"28682","inReplyTo":"1318629110-15232-1-git-send-email-andreiw@vmware.com","subject":"Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.","fromName":"Tor Arvid Lund","fromEmail":"torarvid@gmail.com","sentAt":"2011-10-14T22:31:45Z","receivedAt":"2011-10-14T22:31:45Z","isPatch":true,"sender":{"key":"torarvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/439758?v=4"},"body":"Hi Andrei, and thanks for trying to help improve git-p4! :-)\n\n2011/10/14 Andrei Warkentin :\n> Many users of p4/sd use changelists for review, regression\n> tests and batch builds, thus changes are almost never directly\n> submitted.\n\nJust out of curiosity... what is 'sd'?\n\n> This new config option lets a 'p4 change -i' run instead of\n> the 'p4 submit -i'.\n\nWell... I have to say that I'm not crazy about this patch... I don't\nthink it is very elegant to have a config flag that says that \"when\nthe user says 'git p4 submit', then don't submit, but do something\nelse instead\".\n\nI would much rather have made a patch to introduce some new command\nlike 'git p4 change'.\n\n> Signed-off-by: Andrei Warkentin \n> ---\n>  contrib/fast-import/git-p4     |   16 ++++++++++++----\n>  contrib/fast-import/git-p4.txt |   10 ++++++++++\n>  2 files changed, 22 insertions(+), 4 deletions(-)\n>\n> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n> index 2f7b270..19c295b 100755\n> --- a/contrib/fast-import/git-p4\n> +++ b/contrib/fast-import/git-p4\n> @@ -959,7 +959,10 @@ class P4Submit(Command, P4UserMap):\n>                 submitTemplate = message[:message.index(separatorLine)]\n>                 if self.isWindows:\n>                     submitTemplate = submitTemplate.replace(\"\\r\\n\", \"\\n\")\n> -                p4_write_pipe(\"submit -i\", submitTemplate)\n> +                if gitConfig(\"git-p4.changeOnSubmit\"):\n> +                    p4_write_pipe(\"change -i\", submitTemplate)\n> +                else:\n> +                    p4_write_pipe(\"subadasdmit -i\", submitTemplate)\n\n... 'subadasdmit'? Did some debug/test code sneak in to your patch?\n\n>\n>                 if self.preserveUser:\n>                     if p4User:\n> @@ -981,9 +984,14 @@ class P4Submit(Command, P4UserMap):\n>             file = open(fileName, \"w+\")\n>             file.write(self.prepareLogMessage(template, logMessage))\n>             file.close()\n> -            print (\"Perforce submit template written as %s. \"\n> -                   + \"Please review/edit and then use p4 submit -i < %s to submit directly!\"\n> -                   % (fileName, fileName))\n> +            if gitConfig(\"git-p4.changeOnSubmit\"):\n> +                print (\"Perforce submit template written as %s. \"\n> +                       + \"Please review/edit and then use p4 change -i < %s to create changelist!\"\n> +                       % (fileName, fileName))\n> +            else:\n> +                print (\"Perforce submit template written as %s. \"\n> +                       + \"Please review/edit and then use p4 submit -i < %s to submit directly!\"\n> +                       % (fileName, fileName))\n>\n>     def run(self, args):\n>         if len(args) == 0:\n> diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt\n> index 52003ae..3a3a815 100644\n> --- a/contrib/fast-import/git-p4.txt\n> +++ b/contrib/fast-import/git-p4.txt\n> @@ -180,6 +180,16 @@ git-p4.allowSubmit\n>\n>   git config [--global] git-p4.allowSubmit false\n>\n> +git-p4.changeOnSubmit\n> +\n> +  git config [--global] git-p4.changeOnSubmit false\n> +\n> +Most places using p4/sourcedepot don't actually want you submit\n> +changes directly, and changelists are used to do regression testing,\n> +batch builds and review, hence, by setting this parameter to\n> +true you acknowledge you end up creating a changelist which you\n> +must then manually commit.\n> +\n\nIt might be just me, but I never heard of 'sourcedepot' before this\npatch... Google tells me that it might be some old version of p4 that\nMicrosoft used many many years ago. If that's the case, maybe it\ndoesn't add much value to talk about it in git-p4 docs.. (??)\n\nAnd you claim 'most places don't want people to submit directly'. I'm\nnot sure I agree with that, and anyway it seems like the git-p4 docs\nshould be phrased more neutral than that.\n\n\nMy advice would be, as I mentioned earlier, to rework this into a\npatch introducing a separate command 'git p4 change' instead of this\nconfig flag.\n\nHave a good one!\n\n Tor Arvid\n\n>  git-p4.syncFromOrigin\n>\n>  A useful setup may be that you have a periodically updated git repository\n> --\n> 1.7.4.1\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"177698","messageId":"811639890.180572.1318632957147.JavaMail.root@zimbra-prod-mbox-2.vmware.com","threadId":"28682","inReplyTo":"CA+DMoH-HqA0DCyUSttO-iYO0rUHq1nLqM9W0imAOjHC5H1r_9w@mail.gmail.com","subject":"Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.","fromName":"Andrei Warkentin","fromEmail":"awarkentin@vmware.com","sentAt":"2011-10-14T22:55:57Z","receivedAt":"2011-10-14T22:55:57Z","isPatch":true,"sender":{"key":"awarkentin@vmware.com","avatar":null},"body":"Hi Tor,\n\nThanks for the review!\n\n----- Original Message -----\n> Just out of curiosity... what is 'sd'?\n> \n\nSourceDepot, a p4 fork that is used elsewhere, not by me though ;).\n\n> > This new config option lets a 'p4 change -i' run instead of\n> > the 'p4 submit -i'.\n> \n> Well... I have to say that I'm not crazy about this patch... I don't\n> think it is very elegant to have a config flag that says that \"when\n> the user says 'git p4 submit', then don't submit, but do something\n> else instead\".\n> \n> I would much rather have made a patch to introduce some new command\n> like 'git p4 change'.\n> \n\nAgreed, how about something like this?\n\nThe commands dict maps command name to class and optional dict passed to cmd.run(). That way 'change'\ncan really mean P4Submit with an extra parameter not to submit but to do a changelist instead. The\nreason why I initially made the config flag was because I didn't want to copy-paste P4Submit into P4Change.\n\ncommands = {\n \"debug\" : [ P4Debug, {} ]\n \"submit\" : [ P4Submit, { \"doChange\" : 0 } ]\n \"commit\" : [ P4Submit, { \"doChange\" : 0 } ]\n \"change\" : [ P4Submit, { \"doChange\" : 1 } ]\n \"sync\" : [ P4Sync, {} ],\n \"rebase\" : [ P4Rebase, {} ],\n \"clone\" : [ P4Clone, {} ],\n \"rollback\" : [ P4RollBack, {} ],\n \"branches\" : [ P4Branches, {} ]\n}\n\nA\n"},{"id":"177744","messageId":"4E99E8D2.6020107@diamand.org","threadId":"28682","inReplyTo":"1318629110-15232-1-git-send-email-andreiw@vmware.com","subject":"Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-10-15T20:10:58Z","receivedAt":"2011-10-15T20:10:58Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 14/10/11 22:51, Andrei Warkentin wrote:\n> Many users of p4/sd use changelists for review, regression\n> tests and batch builds, thus changes are almost never directly\n> submitted.\n>\n> This new config option lets a 'p4 change -i' run instead of\n> the 'p4 submit -i'.\n>\n> Signed-off-by: Andrei Warkentin\n> ---\n> contrib/fast-import/git-p4 | 16 ++++++++++++----\n> contrib/fast-import/git-p4.txt | 10 ++++++++++\n> 2 files changed, 22 insertions(+), 4 deletions(-)\n>\n> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n> index 2f7b270..19c295b 100755\n> --- a/contrib/fast-import/git-p4\n> +++ b/contrib/fast-import/git-p4\n> @@ -959,7 +959,10 @@ class P4Submit(Command, P4UserMap):\n> submitTemplate = message[:message.index(separatorLine)]\n> if self.isWindows:\n> submitTemplate = submitTemplate.replace(\"\\r\\n\", \"\\n\")\n> - p4_write_pipe(\"submit -i\", submitTemplate)\n> + if gitConfig(\"git-p4.changeOnSubmit\"):\n> + p4_write_pipe(\"change -i\", submitTemplate)\n> + else:\n> + p4_write_pipe(\"subadasdmit -i\", submitTemplate)\n\n\nWhat does \"p4 subadasmit\" do? That's a new command to me!\n\n(This patch also fails to apply cleanly to my shell-metacharacter patch).\n\n>\n> if self.preserveUser:\n> if p4User:\n> @@ -981,9 +984,14 @@ class P4Submit(Command, P4UserMap):\n> file = open(fileName, \"w+\")\n> file.write(self.prepareLogMessage(template, logMessage))\n> file.close()\n> - print (\"Perforce submit template written as %s. \"\n> - + \"Please review/edit and then use p4 submit -i< %s to submit directly!\"\n> - % (fileName, fileName))\n> + if gitConfig(\"git-p4.changeOnSubmit\"):\n> + print (\"Perforce submit template written as %s. \"\n> + + \"Please review/edit and then use p4 change -i< %s to create changelist!\"\n> + % (fileName, fileName))\n> + else:\n> + print (\"Perforce submit template written as %s. \"\n> + + \"Please review/edit and then use p4 submit -i< %s to submit directly!\"\n> + % (fileName, fileName))\n>\n> def run(self, args):\n> if len(args) == 0:\n> diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt\n> index 52003ae..3a3a815 100644\n> --- a/contrib/fast-import/git-p4.txt\n> +++ b/contrib/fast-import/git-p4.txt\n> @@ -180,6 +180,16 @@ git-p4.allowSubmit\n>\n> git config [--global] git-p4.allowSubmit false\n>\n> +git-p4.changeOnSubmit\n> +\n> + git config [--global] git-p4.changeOnSubmit false\n> +\n> +Most places using p4/sourcedepot don't actually want you submit\n\nSmall typo: should be \"want you *to* submit\"\n\n> +changes directly, and changelists are used to do regression testing,\n> +batch builds and review, hence, by setting this parameter to\n> +true you acknowledge you end up creating a changelist which you\n> +must then manually commit.\n> +\n> git-p4.syncFromOrigin\n>\n> A useful setup may be that you have a periodically updated git repository\n\nRegards!\nLuke\n"},{"id":"177864","messageId":"83923897.7841.1318868319131.JavaMail.root@zimbra-prod-mbox-2.vmware.com","threadId":"28682","inReplyTo":"4E99E8D2.6020107@diamand.org","subject":"Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.","fromName":"Andrei Warkentin","fromEmail":"awarkentin@vmware.com","sentAt":"2011-10-17T16:18:39Z","receivedAt":"2011-10-17T16:18:39Z","isPatch":true,"sender":{"key":"awarkentin@vmware.com","avatar":null},"body":"Hi,\n\n----- Original Message -----\n> From: \"Luke Diamand\" \n> To: \"Andrei Warkentin\" \n> Cc: git@vger.kernel.org, gitster@pobox.com, \"Pete Wyckoff\" \n> Sent: Saturday, October 15, 2011 4:10:58 PM\n> Subject: Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.\n> \n> On 14/10/11 22:51, Andrei Warkentin wrote:\n> > Many users of p4/sd use changelists for review, regression\n> > tests and batch builds, thus changes are almost never directly\n> > submitted.\n> >\n> > This new config option lets a 'p4 change -i' run instead of\n> > the 'p4 submit -i'.\n> >\n> > Signed-off-by: Andrei Warkentin\n> > ---\n> > contrib/fast-import/git-p4 | 16 ++++++++++++----\n> > contrib/fast-import/git-p4.txt | 10 ++++++++++\n> > 2 files changed, 22 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/contrib/fast-import/git-p4\n> > b/contrib/fast-import/git-p4\n> > index 2f7b270..19c295b 100755\n> > --- a/contrib/fast-import/git-p4\n> > +++ b/contrib/fast-import/git-p4\n> > @@ -959,7 +959,10 @@ class P4Submit(Command, P4UserMap):\n> > submitTemplate =\n> > message[:message.index(separatorLine)]\n> > if self.isWindows:\n> > submitTemplate =\n> > submitTemplate.replace(\"\\r\\n\", \"\\n\")\n> > - p4_write_pipe(\"submit -i\", submitTemplate)\n> > + if gitConfig(\"git-p4.changeOnSubmit\"):\n> > + p4_write_pipe(\"change -i\", submitTemplate)\n> > + else:\n> > + p4_write_pipe(\"subadasdmit -i\",\n> > submitTemplate)\n> \n> \n> What does \"p4 subadasmit\" do? That's a new command to me!\n> \n\nAck, that's emabarrasing. How did that get there :-)?\n\nAnyway, the other suggestion I had was to create a new command\ninstead of overriding behaviour of an existing one. Of course,\ncopy-pasting P4Submit into P4Change is silly, so...\n\nHow about something like this?\n\nThe commands dict maps command name to class and optional dict passed to cmd.run(). That way 'change'\ncan really mean P4Submit with an extra parameter not to submit but to do a changelist instead. The\nreason why I initially made the config flag was because I didn't want to copy-paste P4Submit into P4Change.\n\ncommands = {\n \"debug\" : [ P4Debug, {} ]\n \"submit\" : [ P4Submit, { \"doChange\" : 0 } ]\n \"commit\" : [ P4Submit, { \"doChange\" : 0 } ]\n \"change\" : [ P4Submit, { \"doChange\" : 1 } ]\n \"sync\" : [ P4Sync, {} ],\n \"rebase\" : [ P4Rebase, {} ],\n \"clone\" : [ P4Clone, {} ],\n \"rollback\" : [ P4RollBack, {} ],\n \"branches\" : [ P4Branches, {} ]\n}\n\nThanks for the review,\nA\n"},{"id":"177874","messageId":"4E9C799E.70700@diamand.org","threadId":"28682","inReplyTo":"83923897.7841.1318868319131.JavaMail.root@zimbra-prod-mbox-2.vmware.com","subject":"Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-10-17T18:53:18Z","receivedAt":"2011-10-17T18:53:18Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 17/10/11 17:18, Andrei Warkentin wrote:\n> Hi,\n>\n> ----- Original Message -----\n> Anyway, the other suggestion I had was to create a new command\n> instead of overriding behaviour of an existing one. Of course,\n> copy-pasting P4Submit into P4Change is silly, so...\n>\n> How about something like this?\n>\n> The commands dict maps command name to class and optional dict passed to cmd.run(). That way 'change'\n> can really mean P4Submit with an extra parameter not to submit but to do a changelist instead. The\n> reason why I initially made the config flag was because I didn't want to copy-paste P4Submit into P4Change.\n>\n> commands = {\n> \"debug\" : [ P4Debug, {} ]\n> \"submit\" : [ P4Submit, { \"doChange\" : 0 } ]\n> \"commit\" : [ P4Submit, { \"doChange\" : 0 } ]\n> \"change\" : [ P4Submit, { \"doChange\" : 1 } ]\n> \"sync\" : [ P4Sync, {} ],\n> \"rebase\" : [ P4Rebase, {} ],\n> \"clone\" : [ P4Clone, {} ],\n> \"rollback\" : [ P4RollBack, {} ],\n> \"branches\" : [ P4Branches, {} ]\n> }\n>\n> Thanks for the review,\n> A\n>\n\nSounds plausible to me. The alternative would be a command line \nparameter, although that could get annoying and error prone, especially \nas you can't easily unsubmit a perforce change.\n"},{"id":"177903","messageId":"20111017223202.GA1834@arf.padd.com","threadId":"28682","inReplyTo":"4E9C799E.70700@diamand.org","subject":"Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-10-17T22:32:02Z","receivedAt":"2011-10-17T22:32:02Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"luke@diamand.org wrote on Mon, 17 Oct 2011 19:53 +0100:\n> On 17/10/11 17:18, Andrei Warkentin wrote:\n> >Hi,\n> >\n> >----- Original Message -----\n> >Anyway, the other suggestion I had was to create a new command\n> >instead of overriding behaviour of an existing one. Of course,\n> >copy-pasting P4Submit into P4Change is silly, so...\n> >\n> >How about something like this?\n> >\n> >The commands dict maps command name to class and optional dict passed to cmd.run(). That way 'change'\n> >can really mean P4Submit with an extra parameter not to submit but to do a changelist instead. The\n> >reason why I initially made the config flag was because I didn't want to copy-paste P4Submit into P4Change.\n> >\n> >commands = {\n> > \"debug\" : [ P4Debug, {} ]\n> > \"submit\" : [ P4Submit, { \"doChange\" : 0 } ]\n> > \"commit\" : [ P4Submit, { \"doChange\" : 0 } ]\n> > \"change\" : [ P4Submit, { \"doChange\" : 1 } ]\n> > \"sync\" : [ P4Sync, {} ],\n> > \"rebase\" : [ P4Rebase, {} ],\n> > \"clone\" : [ P4Clone, {} ],\n> > \"rollback\" : [ P4RollBack, {} ],\n> > \"branches\" : [ P4Branches, {} ]\n> >}\n> >\n> >Thanks for the review,\n> >A\n> >\n> \n> Sounds plausible to me. The alternative would be a command line\n> parameter, although that could get annoying and error prone,\n> especially as you can't easily unsubmit a perforce change.\n\nThis seems like a useful thing to do, but needs some care.\n\nGit can have multiple commits outstanding that touch the same\nfile, but p4 cannot really have multiple pending changes in the\nsame workspace that touch the same file.\n\nIf you call \"git-p4 change\", it would build a p4 change for each\nof those commits. If the commits happen to touch the same file,\nthe changes get rearranged as far as p4 is concerned so that all\nchanges to a given file are lumped in the first change that sees\nthe file. This is highly counterintuitive from a git mindset.\n\nThe most restrictive implementation would have to:\n\n 1. ensure no pending changes in the P4 clientPath\n 2. ensure number of commits (\"git rev-list\") is 1\n\nYou could be more permissive, allowing multiple pending changes\nif the file sets do not conflict. In that case, the first test\nwould look at the files in pending changes and allow the\noperation if they did not intersect with files in origin..master.\nThe second would make sure that each file appears in no more than\n1 commit in origin..master.\n\nAlso make sure this works with preserveUser. Not sure if an\nunsubmitted change can be handled the same way.\n\nBecause it feels like a delicate operation that could have big\nnegative consequences, this needs a few unit tests.\n\nFor the code structure, I'd like to see a proper subclass instead\nof the dictionary idea. Something like, e.g.:\n\nclass P4Submit(...):\n def __init__(self, change_only=0)\n\t...\n\tself.change_only = change_only\n\nclass P4Change(P4Submit):\n def __init__(self):\n\tP4Submit.__init__(self, change_only=1)\n\nSorry this is looking so difficult now.\n\n\t\t-- Pete\n"},{"id":"177905","messageId":"1987300386.27372.1318891051633.JavaMail.root@zimbra-prod-mbox-2.vmware.com","threadId":"28682","inReplyTo":"20111017223202.GA1834@arf.padd.com","subject":"Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.","fromName":"Andrei Warkentin","fromEmail":"awarkentin@vmware.com","sentAt":"2011-10-17T22:37:31Z","receivedAt":"2011-10-17T22:37:31Z","isPatch":true,"sender":{"key":"awarkentin@vmware.com","avatar":null},"body":"Hi,\n\n----- Original Message -----\n> From: \"Pete Wyckoff\" \n> To: \"Andrei Warkentin\" \n> Cc: git@vger.kernel.org, \"Andrei Warkentin\" , \"Tor Arvid Lund\" , \"Luke\n> Diamand\" , gitster@pobox.com\n> Sent: Monday, October 17, 2011 6:32:02 PM\n> Subject: Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.\n> \n> Git can have multiple commits outstanding that touch the same\n> file, but p4 cannot really have multiple pending changes in the\n> same workspace that touch the same file.\n> \n> If you call \"git-p4 change\", it would build a p4 change for each\n> of those commits. If the commits happen to touch the same file,\n> the changes get rearranged as far as p4 is concerned so that all\n> changes to a given file are lumped in the first change that sees\n> the file. This is highly counterintuitive from a git mindset.\n> \n> The most restrictive implementation would have to:\n> \n> 1. ensure no pending changes in the P4 clientPath\n> 2. ensure number of commits (\"git rev-list\") is 1\n> \n> You could be more permissive, allowing multiple pending changes\n> if the file sets do not conflict. In that case, the first test\n> would look at the files in pending changes and allow the\n> operation if they did not intersect with files in origin..master.\n> The second would make sure that each file appears in no more than\n> 1 commit in origin..master.\n\nHmmm...I see. I'll think some more about it, then!\n\n> \n> Also make sure this works with preserveUser. Not sure if an\n> unsubmitted change can be handled the same way.\n> \n> Because it feels like a delicate operation that could have big\n> negative consequences, this needs a few unit tests.\n> \n> For the code structure, I'd like to see a proper subclass instead\n> of the dictionary idea. Something like, e.g.:\n> \n> class P4Submit(...):\n> def __init__(self, change_only=0)\n> \t...\n> \tself.change_only = change_only\n> \n> class P4Change(P4Submit):\n> def __init__(self):\n> \tP4Submit.__init__(self, change_only=1)\n> \n> Sorry this is looking so difficult now.\n\nNo problem!\n\nA\n"}]}