git-p4 and keyword expansion

git-p4 and keyword expansion

[dhruva]

Hi,
 The git-p4 script unexpands all p4 keywords before feeding it to git fastimport. When there is a new version, it records only the diffs minus the keyword contents at it unexpands and then feeds to fastimport. When trying to submit back to perforce, applying a patch on top of the latest file in p4 with the keyword expanded fails because we have not tracked that difference. Patch applying fails and expects you to manually (out of git) to do a 'p4 submit' and get back and do 'git-p4 submit --continue'.
 Removing the keyword unexpanding code in 'git-p4' with the following patch makes it work:
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 2216cac..35c7914 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -975,10 +975,10 @@ class P4Sync(Command):
                 sys.stderr.write("p4 print fails with: %s\n" % repr(stat))
                 continue

-            if stat['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
-                text = re.sub(r'(?i)\$(Id|Header):[^$]*\$',r'$\1$', text)
-            elif stat['type'] in ('text+k', 'ktext', 'kxtext', 'unicode+k', 'bi
-                text = re.sub(r'\$(Id|Header|Author|Date|DateTime|Change|File|R
+            #if stat['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
+            #    text = re.sub(r'(?i)\$(Id|Header):[^$]*\$',r'$\1$', text)
+            #elif stat['type'] in ('text+k', 'ktext', 'kxtext', 'unicode+k', 'b
+            #    text = re.sub(r'\$(Id|Header|Author|Date|DateTime|Change|File|

             contents[stat['depotFile']] = text

Rationale:
1. The expanded keyword is stored in git repo
2. You edit the file and commit into git (the keyword does not change)
3. When a new version of the same file comes from p4 (through git-p4 sync), it will have a new keyword content. Hence, the keyword change is tracked and 'git-p4 rebase' can apply the patch with not hunk rejections
4. When comitting back to p4 through 'git-p4 submit', the change in keyword contents are tracked and there is a hunk for it. Therefore, the patch applies cleanly and submit goes through..

I am looking for feedback/suggestions on this. I am planning to use git-p4 on production level and am trying to seel to company wide (quite a large group) as an alternative to p4 (still using p4 as final centralized backend to satisfy the management).

with best regards,
dhruva


      Connect with friends all over the world. Get Yahoo! India Messenger at http://in.messenger.yahoo.com/?wm=n/

Re: git-p4 and keyword expansion

[Tor Arvid Lund]

Hi,

On Mon, Sep 8, 2008 at 6:25 AM, dhruva <dhruva@ymail.com> wrote:
> Hi,
>  The git-p4 script unexpands all p4 keywords before feeding it to git fastimport. When there is a new version, it records only the diffs minus the keyword contents at it unexpands and then feeds to fastimport. When trying to submit back to perforce, applying a patch on top of the latest file in p4 with the keyword expanded fails because we have not tracked that difference. Patch applying fails and expects you to manually (out of git) to do a 'p4 submit' and get back and do 'git-p4 submit --continue'.
>  Removing the keyword unexpanding code in 'git-p4' with the following patch makes it work:

Yes, I have also experienced this problem, and haven't yet come up
with a solution to it. Your solution seems to solve the problem with
submitting to p4, but it would also mean that files cloned from p4
would have diffs in the git database whenever the p4 headers changed.
This does not seem nice to me. Are all the keyword expansions
reproducable? I mean... could we store them in git as before
(unexpanded), and make some logic to recreate them upon submitting to
p4? That might work...

I think, at least, that although this is a nice initiative, we should
find a nicer solution.

-Tor Arvid-

Re: git-p4 and keyword expansion

[Tor Arvid Lund]

On Mon, Sep 8, 2008 at 10:27 AM, Tor Arvid Lund <torarvid@gmail.com> wrote:
> Hi,
>
> On Mon, Sep 8, 2008 at 6:25 AM, dhruva <dhruva@ymail.com> wrote:
>> Hi,
>>  The git-p4 script unexpands all p4 keywords before feeding it to git fastimport. When there is a new version, it records only the diffs minus the keyword contents at it unexpands and then feeds to fastimport. When trying to submit back to perforce, applying a patch on top of the latest file in p4 with the keyword expanded fails because we have not tracked that difference. Patch applying fails and expects you to manually (out of git) to do a 'p4 submit' and get back and do 'git-p4 submit --continue'.
>>  Removing the keyword unexpanding code in 'git-p4' with the following patch makes it work:
>
> Yes, I have also experienced this problem, and haven't yet come up
> with a solution to it. Your solution seems to solve the problem with
> submitting to p4, but it would also mean that files cloned from p4
> would have diffs in the git database whenever the p4 headers changed.

Hmm... Now that I think about it... Maybe we _want_ the extra diff
hunks... Personally I don't care much for the keyword expansions, but
since they're there in p4, and p4 diff shows a diff hunk there, maybe
we should just do the same. In which case your original patch might be
just fine. I realise I haven't had my morning coffee yet. Me
confused... :-/

-TA-

Re: git-p4 and keyword expansion

[Jing Xue]

Quoting dhruva <dhruva@ymail.com>:

> Hi,
>  The git-p4 script unexpands all p4 keywords before feeding it to  
> git fastimport. When there is a new version, it records only the  
> diffs minus the keyword contents at it unexpands and then feeds to  
> fastimport. When trying to submit back to perforce, applying a patch  
> on top of the latest file in p4 with the keyword expanded fails  
> because we have not tracked that difference.
> Patch applying fails and expects you to manually (out of git) to do  
> a 'p4 submit' and get back and do 'git-p4 submit --continue'.
>  Removing the keyword unexpanding code in 'git-p4' with the  
> following patch makes it work:

I'm not really arguing against the patch itself, but just wondering  
whether it would be a good idea to make it optional or configurable.

IIUC, there are reasons for git to discourage keyword expansion - for  
instance as discussed in this thread:

http://kerneltrap.org/mailarchive/git/2007/10/11/335112

Cheers.
-- 
Jing Xue

----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

Re: git-p4 and keyword expansion

[dhruva]

Hi,
 Nice to see some traction in this area, motivates me to work harder to solve this problem.



----- Original Message ----
> From: Tor Arvid Lund <torarvid@gmail.com>
> To: dhruva <dhruva@ymail.com>
> Cc: Git Mailing List <git@vger.kernel.org>
> Sent: Monday, 8 September, 2008 2:07:05 PM
> Subject: Re: git-p4 and keyword expansion
> 
> On Mon, Sep 8, 2008 at 10:27 AM, Tor Arvid Lund wrote:
> > Hi,
> >
> > On Mon, Sep 8, 2008 at 6:25 AM, dhruva wrote:
> >>  Removing the keyword unexpanding code in 'git-p4' with the following patch 
> makes it work:
> >
> > Yes, I have also experienced this problem, and haven't yet come up
> > with a solution to it. Your solution seems to solve the problem with
> > submitting to p4, but it would also mean that files cloned from p4
> > would have diffs in the git database whenever the p4 headers changed.
> 
> Hmm... Now that I think about it... Maybe we _want_ the extra diff
> hunks... Personally I don't care much for the keyword expansions, but
> since they're there in p4, and p4 diff shows a diff hunk there, maybe
> we should just do the same. In which case your original patch might be
> just fine. I realise I haven't had my morning coffee yet. Me
> confused... :-/
> 

I was drafting my reply and saw your response. The only area I am not sure it git-p4 handles is a rename/copy of an existing file. If that depends on heuristics based on diff, that might fail due to keyword expansions. I will look closer into the git-p4 script today.

-dhruva



      Get an email ID as yourname@ymail.com or yourname@rocketmail..com. Click here http://in.promos.yahoo.com/address

Re: git-p4 and keyword expansion

[dhruva]

Hello,



----- Original Message ----
> From: Jing Xue <jingxue@digizenstudio.com>
> To: dhruva <dhruva@ymail.com>
> Cc: GIT SCM <git@vger.kernel.org>
> Sent: Tuesday, 9 September, 2008 3:09:00 AM
> Subject: Re: git-p4 and keyword expansion
> >  Removing the keyword unexpanding code in 'git-p4' with the  
> > following patch makes it work:
> 
> I'm not really arguing against the patch itself, but just wondering  
> whether it would be a good idea to make it optional or configurable.

I feel the configuration must be set the first time only, when you clone using 'git-p4 clone'. Altering it in between will be very confusing! Ideally, the setting must be transferred when the git repo (cloned from git-p4) is cloned using standard git. Is it something possible (well, I am new to git and am exploring. Any extra information would help).

My proposal is as follows:
1. Add an extra command line argument to 'git-p4 clone' to either enable/disable keyword expansion
2. Store that information under the .git folder in a file that is copied when someone clones that repo
3. Use the stored information in future 'git-p4 sync/rebase'

> IIUC, there are reasons for git to discourage keyword expansion - for  
> instance as discussed in this thread:
> 
> http://kerneltrap.org/mailarchive/git/2007/10/11/335112

I agree that keyword expansion is bad but there is no alternative when you work with a system that has keyword expansion and you need interoperability (p4<->git)

-dhruva



      Unlimited freedom, unlimited storage. Get it now, on http://help.yahoo.com/l/in/yahoo/mail/yahoomail/tools/tools-08.html/

Re: git-p4 and keyword expansion

[Jing Xue]

On Tue, Sep 09, 2008 at 04:44:11PM +0530, dhruva wrote:
> Hello,

Hi,

> I feel the configuration must be set the first time only, when you
> clone using 'git-p4 clone'. Altering it in between will be very
> confusing!
> Ideally, the setting must be transferred when the git repo
> (cloned from git-p4) is cloned using standard git. Is it something
> possible (well, I am new to git and am exploring. Any extra
> information would help).
>
> My proposal is as follows:
> 1. Add an extra command line argument to 'git-p4 clone' to either  
> enable/disable keyword expansion
> 2. Store that information under the .git folder in a file that is  
> copied when someone clones that repo
> 3. Use the stored information in future 'git-p4 sync/rebase'

Any way to make it optional would be welcome by me.

> I agree that keyword expansion is bad but there is no alternative
> when you work with a system that has keyword expansion and you
> need interoperability (p4<->git)

A patch would only fail if it has a hunk containing a line with
keyword expansion in it. So I for one am willing to _occasionally_
have to submit to perforce manually, for the benefits of no keyword
expansion - among other things, the ability to diff between
branches without the interferences is very important for me.

But that's just my 2 cents, and I'm just another git(-p4) user.
If you do come up with a "formal" patch, you might want to
explicitly add Simon Hausmann to the To list, for he's the git-p4
author.

Cheers.
-- 
Jing

----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.