RESOLVED FIXED 74469
svn scripts cannot handle files with @ symbols
https://bugs.webkit.org/show_bug.cgi?id=74469
Summary svn scripts cannot handle files with @ symbols
Stephanie Lewis
Reported 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.
Attachments
patch (17.04 KB, patch)
2011-12-13 17:52 PST, Stephanie Lewis
mrowe: review-
second patch (16.05 KB, patch)
2011-12-13 18:34 PST, Stephanie Lewis
aroben: review+
Stephanie Lewis
Comment 1 2011-12-13 17:52:37 PST
Mark Rowe (bdash)
Comment 2 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.
Stephanie Lewis
Comment 3 2011-12-13 18:34:53 PST
Created attachment 119133 [details] second patch
Adam Roben (:aroben)
Comment 4 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.
Stephanie Lewis
Comment 5 2011-12-15 17:09:45 PST
Note You need to log in before you can comment on or make changes to this bug.