[Webkit-unassigned] [Bug 169003] Use git's -C flag when possible in VCSUtils.pm

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Feb 28 18:46:38 PST 2017


Daniel Bates <dbates at webkit.org> changed:

           What    |Removed                     |Added
 Attachment #303006|review?                     |review+
              Flags|                            |

--- Comment #3 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 303006
  --> https://bugs.webkit.org/attachment.cgi?id=303006

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

Please fix up the Git cases in VCSUtils::svn{InfoForPath, RevisionForDirectory}() to use "git -C" instead of changing directories before invoking git before landing this change.

> Tools/ChangeLog:3
> +        Use the git -C flag where appropriate to avoid changing directories.

This line should be the bug title. The bug description should be below the "Reviewed by" line.

> Tools/Scripts/VCSUtils.pm:226
> +    return system("git -C $dir rev-parse > " . File::Spec->devnull() . " 2>&1") == 0;

We should put surround the value of $dir in single quotes so as to support paths that contain spaces. I also do not see the need to abbreviate the word directory to dir. I suggest we take this opportunity to rename the local variable to $directory.

> Tools/Scripts/VCSUtils.pm:244
> +    my $output = `git -C $directory config --get svn-remote.svn.fetch 2>& 1`;

This change breaks this code when $directory contains a space character(s) since this form of system() will invoke the specified string using the shell and the shell will interpret a space character as the delimiter between command line arguments. One way to fix this is to surround $directory in single quotes.

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170301/97cb702d/attachment-0001.html>

More information about the webkit-unassigned mailing list