Re: [RFC v1] git-p4: test case for RCS keyword problem

[Luke Diamand <luke@diamand.org>]

On 10/05/11 00:00, Pete Wyckoff wrote:
> luke@diamand.org wrote on Mon, 09 May 2011 08:49 +0100:

<snip>

>>
>> You would think you could just tell p4 to not expand RCS keywords
>> in your client view, but sadly that option doesn't exist :-(
>
> One idea.  I snuck in a2b665d "convert filter: supply path to
> external driver" that lets you do:
>
>      git config filter.p4.clean git-p4-filter --clean %f
>      git config filter.p4.smudge git-p4-filter --smudge %f
>      echo "*.c filter=p4">>  .gitattributes
>
> Then your git tree can have expanded keywords too.  The script to
> clean is pretty easy; to smudge you have to ask p4 for the
> information using fstat and filelog.  My script is pretty nasty
> and full of dependencies, but I could clean it up if you think
> this is a good way to go.

Would this cause problems switching from one branch to another? If 
there's a file with $Date:$ or $Revision:$ then every time you do "git 
checkout other_branch" or even just "git stash" git would have to patch 
up all files tagged with ktext (tens of thousands in my case). If you 
didn't you'd be right back at square one.

At that point it would be easier just to submit a one-time change to 
Perforce that simply zapped every RCS keyword in every file (probably 
resulting in all kinds of pain for perforce users though).

My feeling is that there's no way to elegantly handle RCS keywords. 
They're a really nasty horrible hangover from the dark ages, and if 
you've got them in your code base you're going to have to accept that 
anything that tries to deal with them is going to be ugly.


> We'd need to discover text+k files at clone and sync time
> and maintain the .gitattributes accordingly.  Filtering all
> files would be wrong, and slow.
>
> Dhruva's approach has the downside of always including RCS lines
> in every commit when the file changes in p4.
>
>> This isn't a fix, it's just a test case that shows the problem,
>> and doesn't even try to test the whole-file deletion case.
>>
>> I'm hoping someone will suggest a good way to handle this.
>>
>> Otherwise, I've got a possible scheme that involves spotting the
>> failure to apply the patch, patching up RCS keywords in the
>> p4 client shadow and then trying again. It's not pretty but it seems
>> like it ought to work. My current version doesn't handle deletion,
>> and zaps *all* RCS keywords rather than just the ones zapped in git;
>> more work is needed before I can submit it.
>
> I'm a little hazy on how you would identify a patch failure as
> due to an RCS keyword, but maybe it's possible.  The deleted line
> case is surely hard.

My existing patch is pretty crude - if "git apply --check" fails then it 
parses the output, finds the files causing the problem, checks if 
they're +ktext, and then zaps every RCS keyword in the file in the P4 
shadow repo, replacing them with just $Keyword:$.

Amazingly this seems to work, since git then sees the text it is 
expecting and is happy, while untouched RCS keywords go back to $KW:$ 
which P4 is quite happy to re-expand!

I haven't tried to handle the case where the entire file is deleted. 
This is a bit harder - the file won't be opened for edit so I imagine 
you have to do some kind of 'chmod +w' :-(

>
> Curious what you think of switching to having expanded keywords
> in the repo, but using smudge/clean instead.

One of the many things I like about git is that by default it never 
messes with the contents of my files. I've spent too much of my life 
trawling through diffs full of changes that are down to RCS keywords!

>
> Maybe hang onto this test case until we figure out how we want
> to deal with it.

Another option I wondered about is to modify git apply so that it spots 
RCS keywords and ignores them? This would then mimic what p4 itself does.

And another option is to just admit that RCS keywords are stupid and 
can't be sorted out properly, put some explanation into git-p4.txt to 
explain the problem to avoid having these threads pop up, and then 
everyone just has to zap them in Perforce.

Regards!
Luke