[Webkit-unassigned] [Bug 26999] bugzilla-tool/svn-apply can't handle patches made from a non-root directory

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26999


David Kilzer (ddkilzer) <ddkilzer at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32491|review?                     |review-
               Flag|                            |




--- Comment #5 from David Kilzer (ddkilzer) <ddkilzer at webkit.org>  2009-07-14 09:07:59 PDT ---
(From update of attachment 32491)
> +use webkitdirs;

It's unfortunate that we need to use the webkitdirs module since this script
could be used on any SVN working directory previously.  (Darin Adler may have
more input on this.)

> +sub chdirWebKitAndGetPrefix;

This needs empty parenthesis after the declaration (an empty prototype) to
match the style of the rest of the source.

> +    my $file = $prefix . $fileData->{path};

This should use File::Spec->catdir($prefix, $fileData->{path}) instead.  Then
you don't have to worry about the trailing '/' in chdirWebKitAndGetPrefix().

> +# Change to WebKit dir and get the difference from the current dir

This comment isn't really needed; the method name explains what it does
already.

> +sub chdirWebKitAndGetPrefix

Needs parenthesis after the method to match the style of the rest of the
source.

> +{
> +    my $before = File::Spec->rel2abs( File::Spec->curdir() );
> +    chdirWebKit();
> +    my $after = File::Spec->rel2abs( File::Spec->curdir() );
> +    $before =~ s/^$after\/?//;

I think it would be clearer to use File::Spec::abs2rel() instead of rolling
your own regex here:

my $relativePath = File::Spec->abs2rel($after, $before);

> +    return $before if $before eq '';    # Already at WebKit dir
> +    $before .= '/' if $before !~ /\/$/; # Prefix needs a trailing '/'
> +    return $before;
> +}

With the change to use File::Spec::catdir() above, and because
File::Spec::abs2rel() will either return a relative path or an empty string,
this logic goes away:

    return $relativePath;

In fact, you should just remove the $relativePath variable and return the
result directly:

    return File::Spec->abs2rel($after, $before);

r- to fix everything except the 'use webkitdirs' issue.

IMO, it would be great to have a replacement subroutine for chdirwebkit() that
got the svn URL from the "svn info" command in the current directory, then did
a "chdir .." until the current svn URL no longer matched the previous svn URL
(minus one directory), then you could chdir back into the last directory.  This
would then put you at the root of the svn working directory regardless of
whether you're working on WebKit or not.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.