Diff
Modified: trunk/Tools/ChangeLog (139711 => 139712)
--- trunk/Tools/ChangeLog 2013-01-15 05:15:31 UTC (rev 139711)
+++ trunk/Tools/ChangeLog 2013-01-15 05:16:20 UTC (rev 139712)
@@ -1,3 +1,19 @@
+2013-01-14 Tim 'mithro' Ansell <[email protected]>
+
+ Changing clean_working_directory/clean_local_commits and related
+ functions to have consistent naming.
+
+ https://bugs.webkit.org/show_bug.cgi?id=104198
+
+ Reviewed by Adam Barth.
+
+ * Scripts/webkitpy/common/checkout/scm/git.py:
+ * Scripts/webkitpy/common/checkout/scm/scm.py:
+ * Scripts/webkitpy/common/checkout/scm/scm_mock.py:
+ * Scripts/webkitpy/common/checkout/scm/scm_unittest.py:
+ * Scripts/webkitpy/tool/steps/cleanworkingdirectory.py:
+ * Scripts/webkitpy/tool/steps/cleanworkingdirectory_unittest.py:
+
2013-01-14 Alan Cutter <[email protected]>
Fix EWS GCE build scripts to detect which zone is available
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/scm/git.py (139711 => 139712)
--- trunk/Tools/Scripts/webkitpy/common/checkout/scm/git.py 2013-01-15 05:15:31 UTC (rev 139711)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/scm/git.py 2013-01-15 05:16:20 UTC (rev 139712)
@@ -48,11 +48,11 @@
class AmbiguousCommitError(Exception):
- def __init__(self, num_local_commits, working_directory_is_clean):
+ def __init__(self, num_local_commits, has_working_directory_changes):
Exception.__init__(self, "Found %s local commits and the working directory is %s" % (
- num_local_commits, ["not clean", "clean"][working_directory_is_clean]))
+ num_local_commits, ["clean", "not clean"][has_working_directory_changes]))
self.num_local_commits = num_local_commits
- self.working_directory_is_clean = working_directory_is_clean
+ self.has_working_directory_changes = has_working_directory_changes
class Git(SCM, SVNRepository):
@@ -148,13 +148,13 @@
def rebase_in_progress(self):
return self._filesystem.exists(self.absolute_path(self._filesystem.join('.git', 'rebase-apply')))
- def working_directory_is_clean(self):
- return self._run_git(['diff', self.remote_merge_base(), '--no-renames', '--name-only']) == ""
+ def has_working_directory_changes(self):
+ return self._run_git(['diff', 'HEAD', '--no-renames', '--name-only']) != ""
- def clean_working_directory(self):
- # Could run git clean here too, but that wouldn't match working_directory_is_clean
- self._run_git(['reset', '--hard', self.remote_merge_base()])
- # Aborting rebase even though this does not match working_directory_is_clean
+ def discard_working_directory_changes(self):
+ # Could run git clean here too, but that wouldn't match subversion
+ self._run_git(['reset', 'HEAD', '--hard'])
+ # Aborting rebase even though this does not match subversion
if self.rebase_in_progress():
self._run_git(['rebase', '--abort'])
@@ -336,30 +336,30 @@
def revert_files(self, file_paths):
self._run_git(['checkout', 'HEAD'] + file_paths)
- def _assert_can_squash(self, working_directory_is_clean):
+ def _assert_can_squash(self, has_working_directory_changes):
squash = Git.read_git_config('webkit-patch.commit-should-always-squash', cwd=self.checkout_root)
should_squash = squash and squash.lower() == "true"
if not should_squash:
# Only warn if there are actually multiple commits to squash.
num_local_commits = len(self.local_commits())
- if num_local_commits > 1 or (num_local_commits > 0 and not working_directory_is_clean):
- raise AmbiguousCommitError(num_local_commits, working_directory_is_clean)
+ if num_local_commits > 1 or (num_local_commits > 0 and has_working_directory_changes):
+ raise AmbiguousCommitError(num_local_commits, has_working_directory_changes)
def commit_with_message(self, message, username=None, password=None, git_commit=None, force_squash=False, changed_files=None):
# Username is ignored during Git commits.
- working_directory_is_clean = self.working_directory_is_clean()
+ has_working_directory_changes = self.has_working_directory_changes()
if git_commit:
# Special-case HEAD.. to mean working-copy changes only.
if git_commit.upper() == 'HEAD..':
- if working_directory_is_clean:
+ if not has_working_directory_changes:
raise ScriptError(message="The working copy is not modified. --git-commit=HEAD.. only commits working copy changes.")
self.commit_locally_with_message(message)
return self._commit_on_branch(message, 'HEAD', username=username, password=password)
# Need working directory changes to be committed so we can checkout the merge branch.
- if not working_directory_is_clean:
+ if has_working_directory_changes:
# FIXME: webkit-patch land will modify the ChangeLogs to correct the reviewer.
# That will modify the working-copy and cause us to hit this error.
# The ChangeLog modification could be made to modify the existing local commit.
@@ -367,7 +367,7 @@
return self._commit_on_branch(message, git_commit, username=username, password=password)
if not force_squash:
- self._assert_can_squash(working_directory_is_clean)
+ self._assert_can_squash(has_working_directory_changes)
self._run_git(['reset', '--soft', self.remote_merge_base()])
self.commit_locally_with_message(message)
return self.push_local_commits_to_server(username=username, password=password)
@@ -408,7 +408,7 @@
commit_succeeded = False
finally:
# And then swap back to the original branch and clean up.
- self.clean_working_directory()
+ self.discard_working_directory_changes()
self._run_git(['checkout', '-q', branch_name])
self.delete_branch(MERGE_BRANCH_NAME)
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm.py (139711 => 139712)
--- trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm.py 2013-01-15 05:15:31 UTC (rev 139711)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm.py 2013-01-15 05:16:20 UTC (rev 139712)
@@ -90,26 +90,6 @@
def script_path(self, script_name):
return self._filesystem.join(self.scripts_directory(), script_name)
- def ensure_clean_working_directory(self, force_clean):
- if self.working_directory_is_clean():
- return
- if not force_clean:
- print self.run(self.status_command(), error_handler=Executive.ignore_error, cwd=self.checkout_root)
- raise ScriptError(message="Working directory has modifications, pass --force-clean or --no-clean to continue.")
- _log.info("Cleaning working directory")
- self.clean_working_directory()
-
- def ensure_no_local_commits(self, force):
- if not self.supports_local_commits():
- return
- commits = self.local_commits()
- if not len(commits):
- return
- if not force:
- _log.error("Working directory has local commits, pass --force-clean to continue.")
- sys.exit(1)
- self.discard_local_commits()
-
def run_status_and_extract_filenames(self, status_command, status_regexp):
filenames = []
# We run with cwd=self.checkout_root so that returned-paths are root-relative.
@@ -147,12 +127,6 @@
def commit_success_regexp():
SCM._subclass_must_implement()
- def working_directory_is_clean(self):
- self._subclass_must_implement()
-
- def clean_working_directory(self):
- self._subclass_must_implement()
-
def status_command(self):
self._subclass_must_implement()
@@ -231,12 +205,28 @@
def svn_blame(self, path):
self._subclass_must_implement()
+ def has_working_directory_changes(self):
+ self._subclass_must_implement()
+
+ def discard_working_directory_changes(self):
+ self._subclass_must_implement()
+
+ #--------------------------------------------------------------------------
# Subclasses must indicate if they support local commits,
# but the SCM baseclass will only call local_commits methods when this is true.
@staticmethod
def supports_local_commits():
SCM._subclass_must_implement()
+ def local_commits(self):
+ return []
+
+ def has_local_commits(self):
+ return len(self.local_commits()) > 0
+
+ def discard_local_commits(self):
+ return
+
def remote_merge_base(self):
SCM._subclass_must_implement()
@@ -244,8 +234,12 @@
_log.error("Your source control manager does not support local commits.")
sys.exit(1)
- def discard_local_commits(self):
- pass
+ def local_changes_exist(self):
+ return (self.supports_local_commits() and self.has_local_commits()) or self.has_working_directory_changes()
- def local_commits(self):
- return []
+ def discard_local_changes(self):
+ if self.has_working_directory_changes():
+ self.discard_working_directory_changes()
+
+ if self.has_local_commits():
+ self.discard_local_commits()
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm_mock.py (139711 => 139712)
--- trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm_mock.py 2013-01-15 05:15:31 UTC (rev 139711)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm_mock.py 2013-01-15 05:16:20 UTC (rev 139712)
@@ -46,15 +46,24 @@
if return_exit_code:
return 0
- def ensure_clean_working_directory(self, force_clean):
+ def has_working_directory_changes(self):
+ return False
+
+ def discard_working_directory_changes(self):
pass
def supports_local_commits(self):
return True
- def ensure_no_local_commits(self, force_clean):
+ def has_local_commits(self):
+ return False
+
+ def discard_local_commits(self):
pass
+ def discard_local_changes(self):
+ pass
+
def exists(self, path):
# TestRealMain.test_real_main (and several other rebaseline tests) are sensitive to this return value.
# We should make those tests more robust, but for now we just return True always (since no test needs otherwise).
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py (139711 => 139712)
--- trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py 2013-01-15 05:15:31 UTC (rev 139711)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py 2013-01-15 05:16:20 UTC (rev 139712)
@@ -329,8 +329,8 @@
added_files.remove("added_dir")
self.assertItemsEqual(added_files, ["added_dir/added_file2", "added_file", "added_file3", "added_file4"])
- # Test also to make sure clean_working_directory removes added files
- self.scm.clean_working_directory()
+ # Test also to make sure discard_working_directory_changes removes added files
+ self.scm.discard_working_directory_changes()
self.assertItemsEqual(self.scm.added_files(), [])
self.assertFalse(os.path.exists("added_file"))
self.assertFalse(os.path.exists("added_file3"))
@@ -889,7 +889,7 @@
write_into_file_at_path(svn_root_lock_path, "", "utf-8")
# webkit-patch uses a Checkout object and runs update-webkit, just use svn update here.
self.assertRaises(ScriptError, run_command, ['svn', 'update'])
- self.scm.clean_working_directory()
+ self.scm.discard_working_directory_changes()
self.assertFalse(os.path.exists(svn_root_lock_path))
run_command(['svn', 'update']) # Should succeed and not raise.
@@ -1111,11 +1111,11 @@
self.assertTrue(self.scm.rebase_in_progress())
# Make sure our cleanup works.
- self.scm.clean_working_directory()
+ self.scm.discard_working_directory_changes()
self.assertFalse(self.scm.rebase_in_progress())
# Make sure cleanup doesn't throw when no rebase is in progress.
- self.scm.clean_working_directory()
+ self.scm.discard_working_directory_changes()
def test_commitish_parsing(self):
# Multiple revisions are cherry-picked.
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/scm/svn.py (139711 => 139712)
--- trunk/Tools/Scripts/webkitpy/common/checkout/scm/svn.py 2013-01-15 05:15:31 UTC (rev 139711)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/scm/svn.py 2013-01-15 05:16:20 UTC (rev 139712)
@@ -138,10 +138,11 @@
def svn_version(self):
return self._run_svn(['--version', '--quiet'])
- def working_directory_is_clean(self):
- return self._run_svn(["diff"], cwd=self.checkout_root, decode_output=False) == ""
+ def has_working_directory_changes(self):
+ # FIXME: What about files which are not committed yet?
+ return self._run_svn(["diff"], cwd=self.checkout_root, decode_output=False) != ""
- def clean_working_directory(self):
+ def discard_working_directory_changes(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
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py (139711 => 139712)
--- trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py 2013-01-15 05:15:31 UTC (rev 139711)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py 2013-01-15 05:16:20 UTC (rev 139712)
@@ -111,7 +111,7 @@
return ", ".join(target_nicks)
def _update_working_copy(self, tool):
- tool.scm().ensure_clean_working_directory(force_clean=True)
+ tool.scm().discard_local_changes()
tool.executive.run_and_throw_if_fail(tool.deprecated_port().update_webkit_command(), quiet=True, cwd=tool.scm().checkout_root)
def execute(self, nick, args, tool, sheriff):
Modified: trunk/Tools/Scripts/webkitpy/tool/steps/cleanworkingdirectory.py (139711 => 139712)
--- trunk/Tools/Scripts/webkitpy/tool/steps/cleanworkingdirectory.py 2013-01-15 05:15:31 UTC (rev 139711)
+++ trunk/Tools/Scripts/webkitpy/tool/steps/cleanworkingdirectory.py 2013-01-15 05:16:20 UTC (rev 139712)
@@ -28,6 +28,7 @@
from webkitpy.tool.steps.abstractstep import AbstractStep
from webkitpy.tool.steps.options import Options
+from webkitpy.common.system.executive import ScriptError
class CleanWorkingDirectory(AbstractStep):
@@ -46,5 +47,9 @@
if not self._options.clean:
return
if not self._allow_local_commits:
- self._tool.scm().ensure_no_local_commits(self._options.force_clean)
- self._tool.scm().ensure_clean_working_directory(force_clean=self._options.force_clean)
+ if self._tool.scm().has_local_commits() and not self._options.force_clean:
+ raise ScriptError("Repository has local commits, pass --force-clean to continue.")
+ self._tool.scm().discard_local_commits()
+ if self._tool.scm().has_working_directory_changes() and not self._options.force_clean:
+ raise ScriptError("Working directory has changes, pass --force-clean to continue.")
+ self._tool.scm().discard_working_directory_changes()
Modified: trunk/Tools/Scripts/webkitpy/tool/steps/cleanworkingdirectory_unittest.py (139711 => 139712)
--- trunk/Tools/Scripts/webkitpy/tool/steps/cleanworkingdirectory_unittest.py 2013-01-15 05:15:31 UTC (rev 139711)
+++ trunk/Tools/Scripts/webkitpy/tool/steps/cleanworkingdirectory_unittest.py 2013-01-15 05:16:20 UTC (rev 139712)
@@ -31,22 +31,60 @@
from webkitpy.thirdparty.mock import Mock
from webkitpy.tool.mocktool import MockOptions, MockTool
from webkitpy.tool.steps.cleanworkingdirectory import CleanWorkingDirectory
+from webkitpy.common.system.executive import ScriptError
class CleanWorkingDirectoryTest(unittest.TestCase):
- def test_run(self):
+ def test_run_working_directory_changes_no_force(self):
tool = MockTool()
tool._scm = Mock()
- tool._scm.checkout_root = '/mock-checkout'
step = CleanWorkingDirectory(tool, MockOptions(clean=True, force_clean=False))
+ tool._scm.has_working_directory_changes = lambda: True
+ self.assertRaises(ScriptError, step.run, {})
+ self.assertEqual(tool._scm.discard_local_commits.call_count, 0)
+ self.assertEqual(tool._scm.discard_working_directory_changes.call_count, 0)
+
+ def test_run_local_commits_no_force(self):
+ tool = MockTool()
+ tool._scm = Mock()
+ step = CleanWorkingDirectory(tool, MockOptions(clean=True, force_clean=False))
+ tool._scm.has_local_commits = lambda: True
+ self.assertRaises(ScriptError, step.run, {})
+ self.assertEqual(tool._scm.discard_local_commits.call_count, 0)
+ self.assertEqual(tool._scm.discard_working_directory_changes.call_count, 0)
+
+ def test_run_working_directory_changes_force(self):
+ tool = MockTool()
+ tool._scm = Mock()
+ step = CleanWorkingDirectory(tool, MockOptions(clean=True, force_clean=True))
+ tool._scm.has_working_directory_changes = lambda: True
step.run({})
- self.assertEqual(tool._scm.ensure_no_local_commits.call_count, 1)
- self.assertEqual(tool._scm.ensure_clean_working_directory.call_count, 1)
+ self.assertEqual(tool._scm.discard_local_commits.call_count, 1)
+ self.assertEqual(tool._scm.discard_working_directory_changes.call_count, 1)
+ def test_run_local_commits_force(self):
+ tool = MockTool()
+ tool._scm = Mock()
+ step = CleanWorkingDirectory(tool, MockOptions(clean=True, force_clean=True))
+ tool._scm.has_local_commits = lambda: True
+ step.run({})
+ self.assertEqual(tool._scm.discard_local_commits.call_count, 1)
+ self.assertEqual(tool._scm.discard_working_directory_changes.call_count, 1)
+
+ def test_run_no_local_changes(self):
+ tool = MockTool()
+ tool._scm = Mock()
+ step = CleanWorkingDirectory(tool, MockOptions(clean=True, force_clean=False))
+ tool._scm.has_working_directory_changes = lambda: False
+ tool._scm.has_local_commits = lambda: False
+ step.run({})
+ self.assertEqual(tool._scm.discard_local_commits.call_count, 1)
+ self.assertEqual(tool._scm.discard_working_directory_changes.call_count, 1)
+
def test_no_clean(self):
tool = MockTool()
tool._scm = Mock()
step = CleanWorkingDirectory(tool, MockOptions(clean=False))
step.run({})
- self.assertEqual(tool._scm.ensure_no_local_commits.call_count, 0)
- self.assertEqual(tool._scm.ensure_clean_working_directory.call_count, 0)
+ self.assertEqual(tool._scm.discard_local_commits.call_count, 0)
+ self.assertEqual(tool._scm.discard_working_directory_changes.call_count, 0)
