Changeset 57361 in webkit
- Timestamp:
- Apr 9, 2010 2:01:32 PM (14 years ago)
- Location:
- trunk/WebKitTools
- Files:
-
- 4 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/WebKitTools/ChangeLog
r57339 r57361 1 2010-04-09 Eric Seidel <eric@webkit.org> 2 3 Reviewed by Adam Barth. 4 5 webkit-patch attached my patch to the wrong bug 6 https://bugs.webkit.org/show_bug.cgi?id=37015 7 8 The problem here is that SVN was violating SCM's implicit 9 contract of always returning paths relative to the repository root. 10 That can easily be fixed by telling SVN that the CWD is the repository root. 11 12 When fixing this I realized there are a large number of places in SCM.py where 13 we want to consider explicitly passing self.checkout_root as the CWD. 14 That would allow scm methods to be executed even when the CWD is not inside 15 the scm tree at all, and would also make sure (in the case of SVN) that paths 16 returned are relative to the root. Git (almost always) returns paths relative 17 to the repository root. 18 19 * Scripts/webkitpy/common/checkout/scm.py: 20 - Explicitly pass self.checkout_root as cwd in run_status_and_extract_filenames 21 - Add a ton of FIXMEs about the need to go back and decide which methods require cwd=self.checkout_root 22 and which do not. We'll probably add a helper function to scm (likely SCM._run) which 23 always passes cwd=self.checkout_root to Executive.run_command 24 * Scripts/webkitpy/common/checkout/scm_unittest.py: 25 - Add a test for this change. 26 * Scripts/webkitpy/tool/commands/upload.py: 27 - Removed the explicit os.chdir to the repository root, since scm.py methods 28 should be robust against the cwd not being equal to the root. 29 1 30 2010-04-09 Adam Roben <aroben@apple.com> 2 31 -
trunk/WebKitTools/Scripts/webkitpy/common/checkout/scm.py
r57091 r57361 33 33 import re 34 34 35 # FIXME: Instead of using run_command directly, most places in this 36 # class would rather use an SCM.run method which automatically set 37 # cwd=self.checkout_root. 35 38 from webkitpy.common.system.executive import Executive, run_command, ScriptError 36 39 from webkitpy.common.system.user import User … … 91 94 92 95 96 # SCM methods are expected to return paths relative to self.checkout_root. 93 97 class SCM: 94 98 def __init__(self, cwd): … … 105 109 def ensure_clean_working_directory(self, force_clean): 106 110 if not force_clean and not self.working_directory_is_clean(): 111 # FIXME: Shouldn't this use cwd=self.checkout_root? 107 112 print run_command(self.status_command(), error_handler=Executive.ignore_error) 108 113 raise ScriptError(message="Working directory has modifications, pass --force-clean or --no-clean to continue.") … … 123 128 def run_status_and_extract_filenames(self, status_command, status_regexp): 124 129 filenames = [] 125 for line in run_command(status_command).splitlines(): 130 # We run with cwd=self.checkout_root so that returned-paths are root-relative. 131 for line in run_command(status_command, cwd=self.checkout_root).splitlines(): 126 132 match = re.search(status_regexp, line) 127 133 if not match: … … 290 296 291 297 def working_directory_is_clean(self): 298 # FIXME: Should this use cwd=self.checkout_root? 292 299 return run_command(['svn', 'diff']) == "" 293 300 294 301 def clean_working_directory(self): 302 # FIXME: Shouldn't this use cwd=self.checkout_root? 295 303 run_command(['svn', 'revert', '-R', '.']) 296 304 … … 331 339 332 340 def diff_for_revision(self, revision): 341 # FIXME: This should probably use cwd=self.checkout_root 333 342 return run_command(['svn', 'diff', '-c', str(revision)]) 334 343 … … 341 350 log("WARNING: svn merge has been known to take more than 10 minutes to complete. It is recommended you use git for rollouts.") 342 351 log("Running '%s'" % " ".join(svn_merge_args)) 352 # FIXME: Should this use cwd=self.checkout_root? 343 353 run_command(svn_merge_args) 344 354 345 355 def revert_files(self, file_paths): 356 # FIXME: This should probably use cwd=self.checkout_root. 346 357 run_command(['svn', 'revert'] + file_paths) 347 358 … … 358 369 svn_commit_args.extend(["--username", username]) 359 370 svn_commit_args.extend(["-m", message]) 371 # FIXME: Should this use cwd=self.checkout_root? 360 372 return run_command(svn_commit_args, error_handler=commit_error_handler) 361 373 … … 389 401 @classmethod 390 402 def read_git_config(cls, key): 403 # FIXME: This should probably use cwd=self.checkout_root. 391 404 return run_command(["git", "config", key], 392 405 error_handler=Executive.ignore_error).rstrip('\n') … … 397 410 398 411 def discard_local_commits(self): 412 # FIXME: This should probably use cwd=self.checkout_root 399 413 run_command(['git', 'reset', '--hard', self.svn_branch_name()]) 400 414 401 415 def local_commits(self): 416 # FIXME: This should probably use cwd=self.checkout_root 402 417 return run_command(['git', 'log', '--pretty=oneline', 'HEAD...' + self.svn_branch_name()]).splitlines() 403 418 … … 406 421 407 422 def working_directory_is_clean(self): 423 # FIXME: This should probably use cwd=self.checkout_root 408 424 return run_command(['git', 'diff', 'HEAD', '--name-only']) == "" 409 425 410 426 def clean_working_directory(self): 427 # FIXME: These should probably use cwd=self.checkout_root. 411 428 # Could run git clean here too, but that wouldn't match working_directory_is_clean 412 429 run_command(['git', 'reset', '--hard', 'HEAD']) … … 447 464 448 465 def create_patch(self): 466 # FIXME: This should probably use cwd=self.checkout_root 449 467 return run_command(['git', 'diff', '--binary', 'HEAD']) 450 468 451 469 @classmethod 452 470 def git_commit_from_svn_revision(cls, revision): 471 # FIXME: This should probably use cwd=self.checkout_root 453 472 git_commit = run_command(['git', 'svn', 'find-rev', 'r%s' % revision]).rstrip() 454 473 # git svn find-rev always exits 0, even when the revision is not found. -
trunk/WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py
r57091 r57361 70 70 # Exists to share svn repository creation code between the git and svn tests 71 71 class SVNTestRepository: 72 @staticmethod 73 def _setup_test_commits(test_object): 72 @classmethod 73 def _svn_add(cls, path): 74 run_command(["svn", "add", path]) 75 76 @classmethod 77 def _svn_commit(cls, message): 78 run_command(["svn", "commit", "--quiet", "--message", message]) 79 80 @classmethod 81 def _setup_test_commits(cls, test_object): 74 82 # Add some test commits 75 83 os.chdir(test_object.svn_checkout_path) 76 test_file = open('test_file', 'w') 77 test_file.write("test1") 78 test_file.flush() 79 80 run_command(['svn', 'add', 'test_file']) 81 run_command(['svn', 'commit', '--quiet', '--message', 'initial commit']) 82 83 test_file.write("test2") 84 test_file.flush() 85 86 run_command(['svn', 'commit', '--quiet', '--message', 'second commit']) 87 88 test_file.write("test3\n") 89 test_file.flush() 84 85 write_into_file_at_path("test_file", "test1") 86 cls._svn_add("test_file") 87 cls._svn_commit("initial commit") 88 89 write_into_file_at_path("test_file", "test1test2") 90 # This used to be the last commit, but doing so broke 91 # GitTest.test_apply_git_patch which use the inverse diff of the last commit. 92 # svn-apply fails to remove directories in Git, see: 93 # https://bugs.webkit.org/show_bug.cgi?id=34871 94 os.mkdir("test_dir") 95 # Slash should always be the right path separator since we use cygwin on Windows. 96 test_file3_path = "test_dir/test_file3" 97 write_into_file_at_path(test_file3_path, "third file") 98 cls._svn_add("test_dir") 99 cls._svn_commit("second commit") 100 101 write_into_file_at_path("test_file", "test1test2test3\n") 90 102 write_into_file_at_path("test_file2", "second file") 91 run_command(['svn', 'add', 'test_file2']) 92 93 run_command(['svn', 'commit', '--quiet', '--message', 'third commit']) 94 95 test_file.write("test4\n") 96 test_file.close() 97 98 run_command(['svn', 'commit', '--quiet', '--message', 'fourth commit']) 103 cls._svn_add("test_file2") 104 cls._svn_commit("third commit") 105 106 write_into_file_at_path("test_file", "test1test2test3\ntest4\n") 107 cls._svn_commit("fourth commit") 99 108 100 109 # svn does not seem to update after commit as I would expect. … … 204 213 self.assertEqual(self.scm.svn_revision_from_commit_text(commit_text), '0') 205 214 215 def _shared_test_changed_files(self): 216 write_into_file_at_path("test_file", "changed content") 217 self.assertEqual(self.scm.changed_files(), ["test_file"]) 218 write_into_file_at_path("test_dir/test_file3", "new stuff") 219 self.assertEqual(self.scm.changed_files(), ["test_dir/test_file3", "test_file"]) 220 old_cwd = os.getcwd() 221 os.chdir("test_dir") 222 # Validate that changed_files does not change with our cwd, see bug 37015. 223 self.assertEqual(self.scm.changed_files(), ["test_dir/test_file3", "test_file"]) 224 os.chdir(old_cwd) 225 206 226 def _shared_test_changed_files_for_revision(self): 207 self.assertEqual(self.scm.changed_files_for_revision(2), ["test_file"]) 227 # SVN reports directory changes, Git does not. 228 changed_files = self.scm.changed_files_for_revision(2) 229 if "test_dir" in changed_files: 230 changed_files.remove("test_dir") 231 self.assertEqual(changed_files, ["test_dir/test_file3", "test_file"]) 208 232 self.assertEqual(sorted(self.scm.changed_files_for_revision(3)), sorted(["test_file", "test_file2"])) # Git and SVN return different orders. 209 233 self.assertEqual(self.scm.changed_files_for_revision(4), ["test_file"]) … … 428 452 429 453 def test_create_patch_is_full_patch(self): 430 test_dir_path = os.path.join(self.svn_checkout_path, 'test_dir')454 test_dir_path = os.path.join(self.svn_checkout_path, "test_dir2") 431 455 os.mkdir(test_dir_path) 432 456 test_file_path = os.path.join(test_dir_path, 'test_file2') 433 457 write_into_file_at_path(test_file_path, 'test content') 434 run_command(['svn', 'add', 'test_dir '])458 run_command(['svn', 'add', 'test_dir2']) 435 459 436 460 # create_patch depends on 'svn-create-patch', so make a dummy version. … … 528 552 self._shared_test_svn_apply_git_patch() 529 553 554 def test_changed_files(self): 555 self._shared_test_changed_files() 556 530 557 def test_changed_files_for_revision(self): 531 558 self._shared_test_changed_files_for_revision() … … 656 683 def test_apply_git_patch(self): 657 684 scm = detect_scm_system(self.git_checkout_path) 685 # We carefullly pick a diff which does not have a directory addition 686 # as currently svn-apply will error out when trying to remove directories 687 # in Git: https://bugs.webkit.org/show_bug.cgi?id=34871 658 688 patch = self._create_patch(run_command(['git', 'diff', 'HEAD..HEAD^'])) 659 689 self._setup_webkittools_scripts_symlink(scm) … … 708 738 self.assertEqual(patch_from_local_commit, patch_since_local_commit) 709 739 740 def test_changed_files(self): 741 self._shared_test_changed_files() 742 710 743 def test_changed_files_for_revision(self): 711 744 self._shared_test_changed_files_for_revision() -
trunk/WebKitTools/Scripts/webkitpy/tool/commands/upload.py
r56863 r57361 53 53 54 54 def execute(self, options, args, tool): 55 os.chdir(tool.scm().checkout_root) 55 # This command is a useful test to make sure commit_message_for_this_commit 56 # always returns the right value regardless of the current working directory. 56 57 print "%s" % tool.checkout().commit_message_for_this_commit().message() 57 58
Note: See TracChangeset
for help on using the changeset viewer.