Attachment 103323 Details for Bug 65893 – Patch

[patch] Patch

bug-65893-20110808180406.patch (text/plain), 12.91 KB, created by Adam Barth on 2011-08-08 18:04:07 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Adam Barth

Created: 2011-08-08 18:04:07 PDT

Size: 12.91 KB

patch

obsolete

>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 92659)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2011-08-08  Adam Barth  <abarth@webkit.org>
>+
>+        Chromium Windows bots can't figure out what SVN revision they're running
>+        https://bugs.webkit.org/show_bug.cgi?id=65893
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The comment in _engage_awesome_windows_hacks explains why we're making
>+        this change.  It's ugly and rediculous, but this approach seems better
>+        than using shell=True when calling popen.
>+
>+        * Scripts/webkitpy/common/checkout/scm/svn.py:
>+        * Scripts/webkitpy/layout_tests/port/chromium_win.py:
>+
> 2011-08-08  Adam Barth  <abarth@webkit.org>
> 
>         Remove deduplicate-tests
>Index: Tools/Scripts/webkitpy/common/checkout/scm/svn.py
>===================================================================
>--- Tools/Scripts/webkitpy/common/checkout/scm/svn.py	(revision 92655)
>+++ Tools/Scripts/webkitpy/common/checkout/scm/svn.py	(working copy)
>@@ -68,6 +68,8 @@ class SVN(SCM, SVNRepository):
>     svn_server_host = "svn.webkit.org"
>     svn_server_realm = "<http://svn.webkit.org:80> Mac OS Forge"
> 
>+    executable_name = "svn"
>+
>     _svn_metadata_files = frozenset(['.svn', '_svn'])
> 
>     def __init__(self, cwd, patch_directories, executive=None):
>@@ -93,7 +95,7 @@ class SVN(SCM, SVNRepository):
> 
>     @classmethod
>     def value_from_svn_info(cls, path, field_name):
>-        svn_info_args = ['svn', 'info']
>+        svn_info_args = [cls.executable_name, 'info']
>         # FIXME: This method should use a passed in executive or be made an instance method and use self._executive.
>         info_output = Executive().run_command(svn_info_args, cwd=path).rstrip()
>         match = re.search("^%s: (?P<value>.+)$" % field_name, info_output, re.MULTILINE)
>@@ -123,17 +125,17 @@ class SVN(SCM, SVNRepository):
> 
>     @memoized
>     def svn_version(self):
>-        return self.run(['svn', '--version', '--quiet'])
>+        return self.run([self.executable_name, '--version', '--quiet'])
> 
>     def working_directory_is_clean(self):
>-        return self.run(["svn", "diff"], cwd=self.checkout_root, decode_output=False) == ""
>+        return self.run([self.executable_name, "diff"], cwd=self.checkout_root, decode_output=False) == ""
> 
>     def clean_working_directory(self):
>         # Make sure there are no locks lying around from a previously aborted svn invocation.
>         # This is slightly dangerous, as it's possible the user is running another svn process
>         # on this checkout at the same time.  However, it's much more likely that we're running
>         # under windows and svn just sucks (or the user interrupted svn and it failed to clean up).
>-        self.run(["svn", "cleanup"], cwd=self.checkout_root)
>+        self.run([self.executable_name, "cleanup"], cwd=self.checkout_root)
> 
>         # svn revert -R is not as awesome as git reset --hard.
>         # It will leave added files around, causing later svn update
>@@ -142,7 +144,7 @@ class SVN(SCM, SVNRepository):
>         added_files = reversed(sorted(self.added_files()))
>         # added_files() returns directories for SVN, we walk the files in reverse path
>         # length order so that we remove files before we try to remove the directories.
>-        self.run(["svn", "revert", "-R", "."], cwd=self.checkout_root)
>+        self.run([self.executable_name, "revert", "-R", "."], cwd=self.checkout_root)
>         for path in added_files:
>             # This is robust against cwd != self.checkout_root
>             absolute_path = self.absolute_path(path)
>@@ -153,7 +155,7 @@ class SVN(SCM, SVNRepository):
>                 os.remove(absolute_path)
> 
>     def status_command(self):
>-        return ['svn', 'status']
>+        return [self.executable_name, 'status']
> 
>     def _status_regexp(self, expected_types):
>         field_count = 6 if self.svn_version() > "1.6" else 5
>@@ -171,7 +173,7 @@ class SVN(SCM, SVNRepository):
> 
>     def add(self, path, return_exit_code=False):
>         self._add_parent_directories(os.path.dirname(os.path.abspath(path)))
>-        return self.run(["svn", "add", path], return_exit_code=return_exit_code)
>+        return self.run([self.executable_name, "add", path], return_exit_code=return_exit_code)
> 
>     def _delete_parent_directories(self, path):
>         if not self.in_working_directory(path):
>@@ -186,15 +188,15 @@ class SVN(SCM, SVNRepository):
>     def delete(self, path):
>         abs_path = os.path.abspath(path)
>         parent, base = os.path.split(abs_path)
>-        result = self.run(["svn", "delete", "--force", base], cwd=parent)
>+        result = self.run([self.executable_name, "delete", "--force", base], cwd=parent)
>         self._delete_parent_directories(os.path.dirname(abs_path))
>         return result
> 
>     def exists(self, path):
>-        return not self.run(["svn", "info", path], return_exit_code=True, decode_output=False)
>+        return not self.run([self.executable_name, "info", path], return_exit_code=True, decode_output=False)
> 
>     def changed_files(self, git_commit=None):
>-        status_command = ["svn", "status"]
>+        status_command = [self.executable_name, "status"]
>         status_command.extend(self._patch_directories)
>         # ACDMR: Addded, Conflicted, Deleted, Modified or Replaced
>         return self.run_status_and_extract_filenames(status_command, self._status_regexp("ACDMR"))
>@@ -202,13 +204,13 @@ class SVN(SCM, SVNRepository):
>     def changed_files_for_revision(self, revision):
>         # As far as I can tell svn diff --summarize output looks just like svn status output.
>         # No file contents printed, thus utf-8 auto-decoding in self.run is fine.
>-        status_command = ["svn", "diff", "--summarize", "-c", revision]
>+        status_command = [self.executable_name, "diff", "--summarize", "-c", revision]
>         return self.run_status_and_extract_filenames(status_command, self._status_regexp("ACDMR"))
> 
>     def revisions_changing_file(self, path, limit=5):
>         revisions = []
>         # svn log will exit(1) (and thus self.run will raise) if the path does not exist.
>-        log_command = ['svn', 'log', '--quiet', '--limit=%s' % limit, path]
>+        log_command = [self.executable_name, 'log', '--quiet', '--limit=%s' % limit, path]
>         for line in self.run(log_command, cwd=self.checkout_root).splitlines():
>             match = re.search('^r(?P<revision>\d+) ', line)
>             if not match:
>@@ -233,9 +235,6 @@ class SVN(SCM, SVNRepository):
>         return "svn"
> 
>     def head_svn_revision(self):
>-        _log.debug('Temporary logging to debug bot...')
>-        _log.debug(self.checkout_root)
>-        _log.debug(os.getcwd())
>         return self.value_from_svn_info(self.checkout_root, 'Revision')
> 
>     # FIXME: This method should be on Checkout.
>@@ -252,17 +251,17 @@ class SVN(SCM, SVNRepository):
>             decode_output=False)
> 
>     def committer_email_for_revision(self, revision):
>-        return self.run(["svn", "propget", "svn:author", "--revprop", "-r", revision]).rstrip()
>+        return self.run([self.executable_name, "propget", "svn:author", "--revprop", "-r", revision]).rstrip()
> 
>     def contents_at_revision(self, path, revision):
>         """Returns a byte array (str()) containing the contents
>         of path @ revision in the repository."""
>         remote_path = "%s/%s" % (self._repository_url(), path)
>-        return self.run(["svn", "cat", "-r", revision, remote_path], decode_output=False)
>+        return self.run([self.executable_name, "cat", "-r", revision, remote_path], decode_output=False)
> 
>     def diff_for_revision(self, revision):
>         # FIXME: This should probably use cwd=self.checkout_root
>-        return self.run(['svn', 'diff', '-c', revision])
>+        return self.run([self.executable_name, 'diff', '-c', revision])
> 
>     def _bogus_dir_name(self):
>         if sys.platform.startswith("win"):
>@@ -291,7 +290,7 @@ class SVN(SCM, SVNRepository):
>     def diff_for_file(self, path, log=None):
>         self._setup_bogus_dir(log)
>         try:
>-            args = ['svn', 'diff']
>+            args = [self.executable_name, 'diff']
>             if self._bogus_dir:
>                 args += ['--config-dir', self._bogus_dir]
>             args.append(path)
>@@ -300,14 +299,14 @@ class SVN(SCM, SVNRepository):
>             self._teardown_bogus_dir(log)
> 
>     def show_head(self, path):
>-        return self.run(['svn', 'cat', '-r', 'BASE', path], decode_output=False)
>+        return self.run([self.executable_name, 'cat', '-r', 'BASE', path], decode_output=False)
> 
>     def _repository_url(self):
>         return self.value_from_svn_info(self.checkout_root, 'URL')
> 
>     def apply_reverse_diff(self, revision):
>         # '-c -revision' applies the inverse diff of 'revision'
>-        svn_merge_args = ['svn', 'merge', '--non-interactive', '-c', '-%s' % revision, self._repository_url()]
>+        svn_merge_args = [self.executable_name, 'merge', '--non-interactive', '-c', '-%s' % revision, self._repository_url()]
>         log("WARNING: svn merge has been known to take more than 10 minutes to complete.  It is recommended you use git for rollouts.")
>         log("Running '%s'" % " ".join(svn_merge_args))
>         # FIXME: Should this use cwd=self.checkout_root?
>@@ -315,11 +314,11 @@ class SVN(SCM, SVNRepository):
> 
>     def revert_files(self, file_paths):
>         # FIXME: This should probably use cwd=self.checkout_root.
>-        self.run(['svn', 'revert'] + file_paths)
>+        self.run([self.executable_name, 'revert'] + file_paths)
> 
>     def commit_with_message(self, message, username=None, password=None, git_commit=None, force_squash=False, changed_files=None):
>         # git-commit and force are not used by SVN.
>-        svn_commit_args = ["svn", "commit"]
>+        svn_commit_args = [self.executable_name, "commit"]
> 
>         if not username and not self.has_authorization_for_realm(self.svn_server_realm):
>             raise AuthenticationError(self.svn_server_host)
>@@ -341,7 +340,7 @@ class SVN(SCM, SVNRepository):
> 
>     def svn_commit_log(self, svn_revision):
>         svn_revision = self.strip_r_from_svn_revision(svn_revision)
>-        return self.run(['svn', 'log', '--non-interactive', '--revision', svn_revision])
>+        return self.run([self.executable_name, 'log', '--non-interactive', '--revision', svn_revision])
> 
>     def last_svn_commit_log(self):
>         # BASE is the checkout revision, HEAD is the remote repository revision
>@@ -350,8 +349,8 @@ class SVN(SCM, SVNRepository):
> 
>     def propset(self, pname, pvalue, path):
>         dir, base = os.path.split(path)
>-        return self.run(['svn', 'pset', pname, pvalue, base], cwd=dir)
>+        return self.run([self.executable_name, 'pset', pname, pvalue, base], cwd=dir)
> 
>     def propget(self, pname, path):
>         dir, base = os.path.split(path)
>-        return self.run(['svn', 'pget', pname, base], cwd=dir).encode('utf-8').rstrip("\n")
>+        return self.run([self.executable_name, 'pget', pname, base], cwd=dir).encode('utf-8').rstrip("\n")
>Index: Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py
>===================================================================
>--- Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py	(revision 92655)
>+++ Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py	(working copy)
>@@ -97,6 +97,29 @@ class ChromiumWinPort(chromium.ChromiumP
>             self._version = port_name[port_name.index('-win-') + len('-win-'):]
>             assert self._version in self.SUPPORTED_VERSIONS, "%s is not in %s" % (self._version, self.SUPPORTED_VERSIONS)
>         self._operating_system = 'win'
>+        self._engage_awesome_windows_hacks()
>+
>+    def _engage_awesome_windows_hacks(self):
>+        try:
>+            self._executive.run_command(['svn', 'help'])
>+        except OSError, e:
>+            try:
>+                self._executive.run_command(['svn.bat', 'help'])
>+                # Chromium Win uses the depot_tools package, which contains a number
>+                # of development tools, including Python and svn. Instead of using a
>+                # real svn executable, depot_tools indirects via a batch file, called
>+                # svn.bat. This batch file allows depot_tools to auto-update the real
>+                # svn executable, which is contained in a subdirectory.
>+                #
>+                # That's all fine and good, except that subprocess.popen can detect
>+                # the difference between a real svn executable and batch file when we
>+                # don't provide use shell=True. Rather than use shell=True on Windows,
>+                # We hack the svn.bat name into the SVN class.
>+                _log.debug('Engaging svn.bat Windows hack.')
>+                from webkitpy.common.checkout.scm.svn import SVN
>+                SVN.executable_name = 'svn.bat'
>+            except OSError, e:
>+                _log.debug('Failed to engage svn.bat Windows hack.')
> 
>     def setup_environ_for_server(self, server_name=None):
>         env = chromium.ChromiumPort.setup_environ_for_server(self)

Flags:

eric: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 65893: 103322 | 103323