svn scripts cannot handle files with @ symbols

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Stephanie Lewis]

svn scripts cannot handle files with @'s symbols.  In svn @ means look at this file at a specific revision.  We should escape them by adding a second @ at the end.  This causes svn to ignore the first.

[Stephanie Lewis]

Created attachment 119122 [details]
patch

[Mark Rowe (bdash)]

Comment on attachment 119122 [details]
patch

Please add a helper function to VCSUtils.pm that escapes a path and call that from the places that need it rather than duplicating the logic across all of these scripts.

[Stephanie Lewis]

Created attachment 119133 [details]
second patch

[Adam Roben (:aroben)]

Comment on attachment 119133 [details]
second patch

View in context: https://bugs.webkit.org/attachment.cgi?id=119133&action=review

> Tools/Scripts/VCSUtils.pm:1997
> +sub escapePath($)
> +{
> +    my ($path) = @_;
> +    $path .= "@" if $path =~ /@/;
> +    return $path;
> +}

Since this function only makes sense for Subversion, it should probably either do nothing for git or have a name that makes it clear it's Subversion-only. Maybe escapeSubversionPath?

> Tools/Scripts/prepare-ChangeLog:1340
> +        my @escapedPaths;
> +        foreach my $path (@paths) { push @escapedPaths, escapePath($path); }

You can do this more succinctly with map:

my @escapedPaths = map(escapePath, @paths);

Seems like $pathsString should become a variable that's only used in the git codepath.

> Tools/Scripts/prepare-ChangeLog:1361
> +        my @escapedFiles;
> +        foreach my $file (@files) { push @escapedFiles, escapePath($file); }
> +        my $escapedFilesString = "'" . join("' '", @escapedFiles) . "'";
> +        $command = "$SVN stat $escapedFilesString";

Same comments as above.

> Tools/Scripts/svn-apply:455
> +        open SVN, "svn rm --force '$escapedPath' |" or die "svn rm --force '$path' failed!";

You should use $escapedPath in the error message too.

[Stephanie Lewis]

Committed http://trac.webkit.org/changeset/103002