Re: [PATCH v3] git-p4: add option to preserve user names
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 7, 2011, 22:22 UTC
- Message-ID
- <7viptmup53.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20110506235912.GA11842@arf.padd.com>
Pete Wyckoff <pw@padd.com> writes:
Show 17 quoted lines
> luke@diamand.org wrote on Fri, 06 May 2011 06:25 +0100: >> On 06/05/11 06:07, Junio C Hamano wrote: >> >Luke Diamand<luke@diamand.org> writes: >> > >> >>This is version 3 of my patch. >> > >> >The previous one from Apr 21st is already on "next" with Ack from Pete. >> >> Ah, sorry. >> >> Should I submit a patch against that then? > > Yes, your changes look good and fix bugs. > > To the diff from v2 to v3: > > Acked-by: Pete Wyckoff <pw@padd.com>
So the only thing lacking at this point is the commit log message?
I am not sure if the "actual user is luke" message you give when (and only when) preserveUser is used is a good "reminder". Isn't it that the user needs reminder when the user should have used but forgot to use this option, not the other way around like your patch does?
I suspect that the message would show an unexpected name only when the new codepath is buggy or the P4 changes the code is interacting are formatted in ways that the new codepath is not expecting (well, they amount to the same thing after all, no?), and having such a message may prevent users from submitting the changeset under an incorrect name, but at that point what recourse do they have?
It looks to me that the message is not helping the users, even though it may help as a debugging aid for git-p4 developers.
-- >8 -- Subject: git-p4 --preserve-user: finishing touches
The earlier round unnecessarily updated the user field that is already correct.
Add a message with the P4 user name used in the submit template to remind the user when this option is given.
Form the change spec string correctly, without relying on the way Python happens to order the dictionary contents.
Signed-off-by: Luke Diamand <luke@diamand.org> Acked-by: Pete Wyckoff <pw@padd.com> --- contrib/fast-import/git-p4 | 17 +++++++++++++---- 1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4 index 36e3d87..6018507 100755 --- a/contrib/fast-import/git-p4 +++ b/contrib/fast-import/git-p4 @@ -688,12 +688,18 @@ class P4Submit(Command, P4UserMap): die("Could not get changelist number for last submit - cannot patch up user details") def modifyChangelistUser(self, changelist, newUser): - # fixup the user field of a changelist after it has been submitted. + # fixup the user field of a single changelist after it has been submitted. changes = p4CmdList("change -o %s" % changelist) + input = '' for c in changes: if c.has_key('User'): + if c['User'] == newUser: + # nothing to do here + return c['User'] = newUser - input = marshal.dumps(changes[0]) + + input = input + marshal.dumps(c) + result = p4CmdList("change -f -i", stdin=input) for r in result: if r.has_key('code'): @@ -703,11 +709,10 @@ class P4Submit(Command, P4UserMap): print("Updated user field for changelist %s to %s" % (changelist, newUser)) return die("Could not modify user field of changelist %s to %s" % (changelist, newUser)) - def canChangeChangelists(self): # check to see if we have p4 admin or super-user permissions, either of # which are required to modify changelists. - results = p4CmdList("-G protects %s" % self.depotPath) + results = p4CmdList("protects %s" % self.depotPath) for r in results: if r.has_key('perm'): if r['perm'] == 'admin': @@ -865,6 +870,10 @@ class P4Submit(Command, P4UserMap): if self.interactive: submitTemplate = self.prepareLogMessage(template, logMessage) + + if self.preserveUser: + submitTemplate = submitTemplate + ("\n######## Actual user %s, modified after commit\n" % p4User) + if os.environ.has_key("P4DIFF"): del(os.environ["P4DIFF"]) diff = ""