[webkit-reviews] review granted: [Bug 27323] Better support for non-Cygwin SVN on Windows : [Attachment 32899] Fixes, part 2

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has granted Peter Kasting
<pkasting at google.com>'s request for review:
Bug 27323: Better support for non-Cygwin SVN on Windows
https://bugs.webkit.org/show_bug.cgi?id=27323

Attachment 32899: Fixes, part 2
https://bugs.webkit.org/attachment.cgi?id=32899&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
>      my $path = '.';
>      my $parent = '../';
>      my $devnull = File::Spec->devnull();
> -    my $exitCode;
> +    my $repositoryRoot;
>      while (1) {
> -	   $exitCode = system("svn info $path 2> $devnull > $devnull")/256;
> -	   last if $exitCode;
> +	   my $thisRoot;
> +	   open INFO, "svn info '$path' |" or die;
> +	   while (<INFO>) {
> +	       if (/^Repository Root: (.+)/) {
> +		   $thisRoot = $1;
> +	       }
> +	   }
> +	   close INFO;
> +
> +	   # It's possible (e.g. for developers of some ports) to have a WebKit

> +	   # checkout in a subdirectory of another checkout.  So abort if the
> +	   # repository root suddenly changes.
> +	   last if !$thisRoot;
> +	   if (!$repositoryRoot) {
> +	       $repositoryRoot = $thisRoot;
> +	   }
> +	   last if $thisRoot ne $repositoryRoot;
> +
>	   $last = $path;
>	   $path = $parent . $path;
>      }

This change looks fine.  I notice another change that I missed in Bug 26999
that could be made to make the code a bit more Windows- (or non-UNIX-)
friendly:

-      my $parent = '../';
+      my $parent = '..';

-	   $path = $parent . $path;
+	   $path = File::Spec->catdir($parent, $path);

Feel free to make this change if you'd like, but it's not necessary.

r=me