Bug 74469 - svn scripts cannot handle files with @ symbols
Summary: svn scripts cannot handle files with @ symbols
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-13 17:42 PST by Stephanie Lewis
Modified: 2011-12-15 17:09 PST (History)
3 users (show)

See Also:


Attachments
patch (17.04 KB, patch)
2011-12-13 17:52 PST, Stephanie Lewis
mrowe: review-
Details | Formatted Diff | Diff
second patch (16.05 KB, patch)
2011-12-13 18:34 PST, Stephanie Lewis
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephanie Lewis 2011-12-13 17:42:07 PST
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.
Comment 1 Stephanie Lewis 2011-12-13 17:52:37 PST
Created attachment 119122 [details]
patch
Comment 2 Mark Rowe (bdash) 2011-12-13 17:59:32 PST
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.
Comment 3 Stephanie Lewis 2011-12-13 18:34:53 PST
Created attachment 119133 [details]
second patch
Comment 4 Adam Roben (:aroben) 2011-12-14 19:07:43 PST
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.
Comment 5 Stephanie Lewis 2011-12-15 17:09:45 PST
Committed http://trac.webkit.org/changeset/103002