Changeset 45026 in webkit


Ignore:
Timestamp:
Jun 23, 2009 6:31:28 PM (15 years ago)
Author:
eric@webkit.org
Message:

2009-06-23 Eric Seidel <​eric@webkit.org>

Reviewed by Dave Levin.

Make SCM.run_command smarter, and make all previous
os.system and subprocess.popen use SCM.run_command instead.
​https://bugs.webkit.org/show_bug.cgi?id=26666

This makes it easier to handle errors in a standard way throughout all the code.
Since this new code raises by default when the exit_code != 0,
we should prevent future problems of bugzilla-tool continuing after
a git or svn command failed.

  • Scripts/modules/scm.py:
Location:
trunk/WebKitTools
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKitTools/ChangeLog

    r45012 r45026  
     12009-06-23  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Dave Levin.
     4
     5        Make SCM.run_command smarter, and make all previous
     6        os.system and subprocess.popen use SCM.run_command instead.
     7        https://bugs.webkit.org/show_bug.cgi?id=26666
     8
     9        This makes it easier to handle errors in a standard way throughout all the code.
     10        Since this new code raises by default when the exit_code != 0,
     11        we should prevent future problems of bugzilla-tool continuing after
     12        a git or svn command failed.
     13
     14        * Scripts/modules/scm.py:
     15
    1162009-06-23  Joe Mason  <joe.mason@torchmobile.com>
    217

  • trunk/WebKitTools/Scripts/bugzilla-tool

    r44979 r45026  
    329329            description = commit_lines[0]
    330330            comment_text = "\n".join(commit_lines[1:])
    331        
    332             comment_text += "\n---\n"
     331            comment_text += "---\n"
    333332            comment_text += tool.scm().files_changed_summary_for_commit(commit_id)
    334333       

  • trunk/WebKitTools/Scripts/modules/scm.py

    r44982 r45026  
    6060
    6161    @staticmethod
    62     def run_command(command, cwd=None):
    63         return subprocess.Popen(command, stdout=subprocess.PIPE, shell=True, cwd=cwd).communicate()[0].rstrip()
     62    def run_command(command, cwd=None, input=None, raise_on_failure=True, return_exit_code=False):
     63        stdin = subprocess.PIPE if input else None
     64        process = subprocess.Popen(command, stdout=subprocess.PIPE, stdin=stdin, shell=True, cwd=cwd)
     65        output = process.communicate(input)[0].rstrip()
     66        exit_code = process.wait()
     67        if raise_on_failure and exit_code:
     68            raise ScriptError("Failed to run " + command)
     69        if return_exit_code:
     70            return exit_code
     71        return output
    6472
    6573    def ensure_clean_working_directory(self, force):
    6674        if not force and not self.working_directory_is_clean():
    67             log("Working directory has modifications, pass --force-clean or --no-clean to continue.")
    68             os.system(self.status_command())
    69             exit(1)
     75            print self.run_command(self.status_command())
     76            error("Working directory has modifications, pass --force-clean or --no-clean to continue.")
    7077       
    7178        log("Cleaning the working directory")
    … …  
    7986            return
    8087        if not force:
    81             log("Working directory has local commits, pass --force-clean to continue.")
    82             exit(1)
     88            error("Working directory has local commits, pass --force-clean to continue.")
    8389        self.discard_local_commits()
    8490
    … …  
    9298    def run_status_and_extract_filenames(self, status_command, status_regexp):
    9399        filenames = []
    94         for line in os.popen(status_command).readlines():
     100        for line in self.run_command(status_command).splitlines():
    95101            match = re.search(status_regexp, line)
    96102            if not match:
    … …  
    171177
    172178    def working_directory_is_clean(self):
    173         diff_process = subprocess.Popen("svn diff", stdout=subprocess.PIPE, shell=True)
    174         diff_output = diff_process.communicate()[0]
    175         if diff_process.wait():
    176             log("Failed to run svn diff")
    177             return False
    178         return diff_output == ""
     179        return self.run_command("svn diff") == ""
    179180
    180181    def clean_working_directory(self):
    181         os.system("svn reset -R")
     182        self.run_command("svn reset -R")
    182183
    183184    def update_webkit(self):
    184         os.system("update-webkit")
     185        self.run_command("update-webkit")
    185186
    186187    def status_command(self):
    … …  
    204205
    205206    def commit_with_message(self, message):
    206         commit_process = subprocess.Popen('svn commit -F -', stdin=subprocess.PIPE, stdout=subprocess.PIPE, shell=True)
    207         (out, error) = commit_process.communicate(message)
    208         return_code = commit_process.wait()
    209         if return_code:
    210             log("Commit failure: %d" % return_code) # We really should handle the failure
    211         return out
     207        return self.run_command('svn commit -F -', input=message)
    212208
    213209# All git-specific logic should go here.
    … …  
    216212        SCM.__init__(self, cwd, dryrun)
    217213   
    218     @staticmethod
    219     def in_working_directory(path):
    220         return SCM.run_command("git rev-parse --is-inside-work-tree 2>&1", cwd=path) == "true"
    221 
    222     @staticmethod
    223     def find_checkout_root(path):
     214    @classmethod
     215    def in_working_directory(cls, path):
     216        return cls.run_command("git rev-parse --is-inside-work-tree 2>&1", cwd=path) == "true"
     217
     218    @classmethod
     219    def find_checkout_root(cls, path):
    224220        # "git rev-parse --show-cdup" would be another way to get to the root
    225         (checkout_root, dot_git) = os.path.split(SCM.run_command("git rev-parse --git-dir", cwd=path))
     221        (checkout_root, dot_git) = os.path.split(cls.run_command("git rev-parse --git-dir", cwd=path))
    226222        # If we were using 2.6 # checkout_root = os.path.relpath(checkout_root, path)
    227223        if not os.path.isabs(checkout_root): # Sometimes git returns relative paths
    … …  
    230226   
    231227    def discard_local_commits(self):
    232         os.system("git reset --hard trunk")
     228        self.run_command("git reset --hard trunk")
    233229   
    234230    def local_commits(self):
    … …  
    236232   
    237233    def working_directory_is_clean(self):
    238         diff_process = subprocess.Popen("git diff-index head", stdout=subprocess.PIPE, shell=True)
    239         diff_output = diff_process.communicate()[0]
    240         if diff_process.wait():
    241             log("Failed to run git diff-index head")
    242             return False
    243         return diff_output == ""
     234        return self.run_command("git diff-index head") == ""
    244235   
    245236    def clean_working_directory(self):
    246         os.system("git reset --hard head")
    247237        # Could run git clean here too, but that wouldn't match working_directory_is_clean
     238        self.run_command("git reset --hard head")
    248239   
    249240    def update_webkit(self):
    250241        # FIXME: Should probably call update-webkit, no?
    251242        log("Updating working directory")
    252         os.system("git svn rebase")
     243        self.run_command("git svn rebase")
    253244
    254245    def status_command(self):
    … …  
    276267   
    277268    def commit_locally_with_message(self, message):
    278         commit_process = subprocess.Popen('git commit -a -F -', stdin=subprocess.PIPE, shell=True)
    279         commit_process.communicate(message)
     269        self.run_command('git commit -a -F -', input=message)
    280270       
    281271    def push_local_commits_to_server(self):
    282272        if self.dryrun:
    283273            return "Dry run, no remote commit."
    284         commit_process = subprocess.Popen('git svn dcommit', stdout=subprocess.PIPE, shell=True)
    285         (out, error) = commit_process.communicate()
    286         return_code = commit_process.wait()
    287         if return_code:
    288             log("Commit failure: %d" % return_code) # We really should handle the failure
    289         return out
     274        return self.run_command('git svn dcommit')
    290275
    291276    def commit_ids_from_range_arguments(self, args, cherry_pick=False):
    … …  
    306291
    307292    def commit_message_for_commit(self, commit_id):
    308         commit_message_process = subprocess.Popen("git cat-file commit " + commit_id, stdout=subprocess.PIPE, shell=True)
    309         commit_message = commit_message_process.communicate()[0]
    310         commit_lines = commit_message.splitlines()
     293        commit_lines = self.run_command("git cat-file commit " + commit_id).splitlines()
    311294
    312295        # Skip the git headers.
    … …  
    322305
    323306    def files_changed_summary_for_commit(self, commit_id):
    324         return subprocess.Popen("git diff-tree --shortstat --no-commit-id " + commit_id, stdout=subprocess.PIPE, shell=True).communicate()[0]
     307        return self.run_command("git diff-tree --shortstat --no-commit-id " + commit_id)
Note: See TracChangeset for help on using the changeset viewer.