git-p4 and keyword expansion

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/

Hi,

On Mon, Sep 8, 2008 at 6:25 AM, dhruva [off-list ref] 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-

On Mon, Sep 8, 2008 at 10:27 AM, Tor Arvid Lund [off-list ref] wrote:

Hi,

On Mon, Sep 8, 2008 at 6:25 AM, dhruva [off-list ref] wrote:

quoted

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-

Quoting dhruva [off-list ref]:

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.