threads / patch / 32014

patchgit p4: RCS expansion should not span newlines

Subject: [PATCH] git p4: RCS expansion should not span newlines

## tl;dr

2 messages between Nov 4, 2012 and Nov 8, 2012. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Pete Wyckoff· Nov 4, 2012, 22:04 UTC · lore

This bug was introduced in cb585a9 (git-p4: keyword flattening fixes, 2011-10-16). The newline character is indeed special, and $File$ expansions should not try to match across multiple lines.

Based-on-patch-by: Chris Goard <cgoard@gmail.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
cgoard@gmail.com wrote on Mon, 16 Jul 2012 12:49 -0700:
Show 21 quoted lines
> Hi. Noticed an apparent bug in git-p4 related to RCS keyword
> expansion. Some files in our Perforce repository have malformed RCS
> keywords, e.g. "$Revision:" without a closing $. Perforce doesn't
> expand these, obviously, but when a change to this file is imported
> into git, everything up to the next $, on another line much later in
> the file, is deleted. Seems to be due to multi-line matching of RCS
> keywords. One fix is:
> 
> diff --git a/git-p4.py b/git-p4.py
> index f895a24..ae7b431 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -215,7 +215,7 @@ def p4_keywords_regexp_for_type(base, type_mods):
>          pattern = r"""
>              \$              # Starts with a dollar, followed by...
>              (%s)            # one of the keywords, followed by...
> -            (:[^$]+)?       # possibly an old expansion, followed by...
> +            (:[^\n$]+)?     # possibly an old expansion, followed by...
>              \$              # another dollar
>              """ % kwords
>          return pattern
Chris,

I finally got around to building a test-case for this. The bug has been in since around 1.7.7, so I won't call this exactly urgent, even though it is blatantly buggy as is.

Will try to get it into the next release.
Thanks,
		-- Pete
 git-p4.py             |  2 +-
 t/t9810-git-p4-rcs.sh | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)
Show changes to 2 files +20 −1

git-p4.py, t/t9810-git-p4-rcs.sh

diff --git a/git-p4.py b/git-p4.py
index 882b1bb..7d6c928 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -227,7 +227,7 @@ def p4_keywords_regexp_for_type(base, type_mods):
         pattern = r"""
             \$              # Starts with a dollar, followed by...
             (%s)            # one of the keywords, followed by...
-            (:[^$]+)?       # possibly an old expansion, followed by...
+            (:[^$\n]+)?     # possibly an old expansion, followed by...
             \$              # another dollar
             """ % kwords
         return pattern
diff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh
index fe30ad8..0c2fc3e 100755
--- a/t/t9810-git-p4-rcs.sh
+++ b/t/t9810-git-p4-rcs.sh
@@ -155,6 +155,25 @@ test_expect_success 'cleanup after failure' '
 	)
 '
 
+# perl $File:: bug check
+test_expect_success 'ktext expansion should not expand multi-line $File::' '
+	(
+		cd "$cli" &&
+		cat >lv.pm <<-\EOF
+		my $wanted = sub { my $f = $File::Find::name;
+				    if ( -f && $f =~ /foo/ ) {
+		EOF
+		p4 add -t ktext lv.pm &&
+		p4 submit -d "lv.pm"
+	) &&
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		test_cmp "$cli/lv.pm" lv.pm
+	)
+'
+
 #
 # Do not scrub anything but +k or +ko files.  Sneak a change into
 # the cli file so that submit will get a conflict.  Make sure that
-- 
1.7.12.1.457.g468b3ef
Jeff King· Nov 8, 2012, 17:47 UTC · re: Pete Wyckoff · lore

Re: [PATCH] git p4: RCS expansion should not span newlines

On Sun, Nov 04, 2012 at 05:04:02PM -0500, Pete Wyckoff wrote:
Show 7 quoted lines
> This bug was introduced in cb585a9 (git-p4: keyword
> flattening fixes, 2011-10-16).  The newline character
> is indeed special, and $File$ expansions should not try
> to match across multiple lines.
> 
> Based-on-patch-by: Chris Goard <cgoard@gmail.com>
> Signed-off-by: Pete Wyckoff <pw@padd.com>
Thanks, I'll queue this for 'maint'. Seems obviously correct to me.
-Peff

← back to recent threads