Changeset 57439 in webkit


Ignore:
Timestamp:
Apr 10, 2010 10:27:11 PM (14 years ago)
Author:
eric@webkit.org
Message:

2010-04-10 Eric Seidel <​eric@webkit.org>

Reviewed by Adam Barth.

WinEWS bot fails to svn update because scm.clean_working_directory leaves files around
​https://bugs.webkit.org/show_bug.cgi?id=37401

The Git-based bots don't have this trouble because
Git.clean_working_directory fully removes files that were
marked as "add". SVN.clean_working_directory previously just
called "svn revert" which would leave added files in the
working directory untracked. This patch makes
SVN.clean_working_directory function more like
Git.clean_working_directory by removing added files after revert.

  • Scripts/webkitpy/common/checkout/scm.py:
    • Add SCM.absolute_path for easy conversion between repository-relative paths and absolute paths.
    • Add SCM.add and SCM.added_files
    • Make SVN.clean_working_directory remove any added_files after svn revert.
    • The new unit tests found a bug in Git.status_command, change to use git diff --name-status instead.
  • Scripts/webkitpy/common/checkout/scm_unittest.py:
    • Add tests for added code.
Location:
trunk/WebKitTools
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKitTools/ChangeLog

    r57437 r57439  
     12010-04-10  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        WinEWS bot fails to svn update because scm.clean_working_directory leaves files around
     6        https://bugs.webkit.org/show_bug.cgi?id=37401
     7
     8        The Git-based bots don't have this trouble because
     9        Git.clean_working_directory fully removes files that were
     10        marked as "add".  SVN.clean_working_directory previously just
     11        called "svn revert" which would leave added files in the
     12        working directory untracked.  This patch makes
     13        SVN.clean_working_directory function more like
     14        Git.clean_working_directory by removing added files after revert.
     15
     16        * Scripts/webkitpy/common/checkout/scm.py:
     17         - Add SCM.absolute_path for easy conversion between
     18           repository-relative paths and absolute paths.
     19         - Add SCM.add and SCM.added_files
     20         - Make SVN.clean_working_directory remove any added_files after svn revert.
     21         - The new unit tests found a bug in Git.status_command, change to use git diff --name-status instead.
     22        * Scripts/webkitpy/common/checkout/scm_unittest.py:
     23         - Add tests for added code.
     24
    1252010-04-10  Adam Barth  <abarth@webkit.org>
    226

  • trunk/WebKitTools/Scripts/webkitpy/common/checkout/scm.py

    r57361 r57439  
    101101        self.dryrun = False
    102102
     103    # SCM always returns repository relative path, but sometimes we need
     104    # absolute paths to pass to rm, etc.
     105    def absolute_path(self, repository_relative_path):
     106        return os.path.join(self.checkout_root, repository_relative_path)
     107
     108    # FIXME: This belongs in Checkout, not SCM.
    103109    def scripts_directory(self):
    104110        return os.path.join(self.checkout_root, "WebKitTools", "Scripts")
    105111
     112    # FIXME: This belongs in Checkout, not SCM.
    106113    def script_path(self, script_name):
    107114        return os.path.join(self.scripts_directory(), script_name)
    … …  
    169176        raise NotImplementedError, "subclasses must implement"
    170177
     178    def add(self, path):
     179        raise NotImplementedError, "subclasses must implement"
     180
    171181    def changed_files(self):
    172182        raise NotImplementedError, "subclasses must implement"
    173183
    174184    def changed_files_for_revision(self):
     185        raise NotImplementedError, "subclasses must implement"
     186
     187    def added_files(self):
    175188        raise NotImplementedError, "subclasses must implement"
    176189
    … …  
    296309
    297310    def working_directory_is_clean(self):
    298         # FIXME: Should this use cwd=self.checkout_root?
    299         return run_command(['svn', 'diff']) == ""
     311        return run_command(["svn", "diff"], cwd=self.checkout_root) == ""
    300312
    301313    def clean_working_directory(self):
    302         # FIXME: Shouldn't this use cwd=self.checkout_root?
    303         run_command(['svn', 'revert', '-R', '.'])
     314        # svn revert -R is not as awesome as git reset --hard.
     315        # It will leave added files around, causing later svn update
     316        # calls to fail on the bots.  We make this mirror git reset --hard
     317        # by deleting any added files as well.
     318        added_files = reversed(sorted(self.added_files()))
     319        # added_files() returns directories for SVN, we walk the files in reverse path
     320        # length order so that we remove files before we try to remove the directories.
     321        run_command(["svn", "revert", "-R", "."], cwd=self.checkout_root)
     322        for path in added_files:
     323            # This is robust against cwd != self.checkout_root
     324            absolute_path = self.absolute_path(path)
     325            # Completely lame that there is no easy way to remove both types with one call.
     326            if os.path.isdir(path):
     327                os.rmdir(absolute_path)
     328            else:
     329                os.remove(absolute_path)
    304330
    305331    def status_command(self):
    … …  
    309335        field_count = 6 if self.svn_version() > "1.6" else 5
    310336        return "^(?P<status>[%s]).{%s} (?P<filename>.+)$" % (expected_types, field_count)
     337
     338    def add(self, path):
     339        # path is assumed to be cwd relative?
     340        run_command(["svn", "add", path])
    311341
    312342    def changed_files(self):
    … …  
    320350    def conflicted_files(self):
    321351        return self.run_status_and_extract_filenames(self.status_command(), self._status_regexp("C"))
     352
     353    def added_files(self):
     354        return self.run_status_and_extract_filenames(self.status_command(), self._status_regexp("A"))
    322355
    323356    @staticmethod
    … …  
    433466
    434467    def status_command(self):
    435         return ['git', 'status']
     468        # git status returns non-zero when there are changes, so we use git diff name --name-status HEAD instead.
     469        return ["git", "diff", "--name-status", "HEAD"]
    436470
    437471    def _status_regexp(self, expected_types):
    438472        return '^(?P<status>[%s])\t(?P<filename>.+)$' % expected_types
     473
     474    def add(self, path):
     475        # path is assumed to be cwd relative?
     476        run_command(["git", "add", path])
    439477
    440478    def changed_files(self):
    … …  
    455493        status_command = ['git', 'diff', '--name-status', '-C', '-M', '--diff-filter=U']
    456494        return self.run_status_and_extract_filenames(status_command, self._status_regexp("U"))
     495
     496    def added_files(self):
     497        return self.run_status_and_extract_filenames(self.status_command(), self._status_regexp("A"))
    457498
    458499    @staticmethod

  • trunk/WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py

    r57361 r57439  
    224224        os.chdir(old_cwd)
    225225
     226    def _shared_test_added_files(self):
     227        write_into_file_at_path("test_file", "changed content")
     228        self.assertEqual(self.scm.added_files(), [])
     229
     230        write_into_file_at_path("added_file", "new stuff")
     231        self.scm.add("added_file")
     232
     233        os.mkdir("added_dir")
     234        write_into_file_at_path("added_dir/added_file2", "new stuff")
     235        self.scm.add("added_dir")
     236
     237        # SVN reports directory changes, Git does not.
     238        added_files = self.scm.added_files()
     239        if "added_dir" in added_files:
     240            added_files.remove("added_dir")
     241        self.assertEqual(added_files, ["added_dir/added_file2", "added_file"])
     242
     243        # Test also to make sure clean_working_directory removes added files
     244        self.scm.clean_working_directory()
     245        self.assertEqual(self.scm.added_files(), [])
     246        self.assertFalse(os.path.exists("added_file"))
     247        self.assertFalse(os.path.exists("added_dir"))
     248
    226249    def _shared_test_changed_files_for_revision(self):
    227250        # SVN reports directory changes, Git does not.
    … …  
    558581        self._shared_test_changed_files_for_revision()
    559582
     583    def test_added_files(self):
     584        self._shared_test_added_files()
     585
    560586    def test_contents_at_revision(self):
    561587        self._shared_test_contents_at_revision()
    … …  
    747773        self._shared_test_contents_at_revision()
    748774
     775    def test_added_files(self):
     776        self._shared_test_added_files()
     777
    749778    def test_committer_email_for_revision(self):
    750779        self._shared_test_committer_email_for_revision()
Note: See TracChangeset for help on using the changeset viewer.