From: machenbach@chromium.org Date: Wed, 19 Feb 2014 11:56:48 +0000 (+0000) Subject: Refactor persisting state in push and merge scripts. X-Git-Tag: upstream/4.7.83~10621 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a56d0da6238b0f4b20ccd83bcfdc777654c0ce9b;p=platform%2Fupstream%2Fv8.git Refactor persisting state in push and merge scripts. - The backed state dict is now persisted and restored in the step template as a json file - All explicit persist/restore calls are removed - Added testing an unexpected script failure + restart with state recovery to the merge-to-branch test - This CL is not changing external behavior of the scripts BUG= R=ulan@chromium.org Review URL: https://codereview.chromium.org/170583002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19478 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/tools/push-to-trunk/auto_roll.py b/tools/push-to-trunk/auto_roll.py index 086f5a8..d147f86 100755 --- a/tools/push-to-trunk/auto_roll.py +++ b/tools/push-to-trunk/auto_roll.py @@ -83,10 +83,10 @@ class CheckTreeStatus(Step): def RunStep(self): status_url = "https://v8-status.appspot.com/current?format=json" status_json = self.ReadURL(status_url, wait_plan=[5, 20, 300, 300]) - message = json.loads(status_json)["message"] - if re.search(r"nopush|no push", message, flags=re.I): - self.Die("Push to trunk disabled by tree state: %s" % message) - self.Persist("tree_message", message) + self["tree_message"] = json.loads(status_json)["message"] + if re.search(r"nopush|no push", self["tree_message"], flags=re.I): + self.Die("Push to trunk disabled by tree state: %s" + % self["tree_message"]) class FetchLatestRevision(Step): @@ -97,18 +97,17 @@ class FetchLatestRevision(Step): match = re.match(r"^r(\d+) ", log) if not match: self.Die("Could not extract current svn revision from log.") - self.Persist("latest", match.group(1)) + self["latest"] = match.group(1) class CheckLastPush(Step): MESSAGE = "Checking last V8 push to trunk." def RunStep(self): - self.RestoreIfUnset("latest") log = self.Git("svn log -1 --oneline ChangeLog").strip() match = re.match(r"^r(\d+) \| Prepare push to trunk", log) if match: - latest = int(self._state["latest"]) + latest = int(self["latest"]) last_push = int(match.group(1)) # TODO(machebach): This metric counts all revisions. It could be # improved by counting only the revisions on bleeding_edge. @@ -124,7 +123,7 @@ class FetchLKGR(Step): def RunStep(self): lkgr_url = "https://v8-status.appspot.com/lkgr" # Retry several times since app engine might have issues. - self.Persist("lkgr", self.ReadURL(lkgr_url, wait_plan=[5, 20, 300, 300])) + self["lkgr"] = self.ReadURL(lkgr_url, wait_plan=[5, 20, 300, 300]) class PushToTrunk(Step): @@ -145,11 +144,8 @@ class PushToTrunk(Step): wait_plan=[5, 20]) def RunStep(self): - self.RestoreIfUnset("latest") - self.RestoreIfUnset("lkgr") - self.RestoreIfUnset("tree_message") - latest = int(self._state["latest"]) - lkgr = int(self._state["lkgr"]) + latest = int(self["latest"]) + lkgr = int(self["lkgr"]) if latest == lkgr: print "ToT (r%d) is clean. Pushing to trunk." % latest self.PushTreeStatus("Tree is closed (preparing to push)") @@ -165,7 +161,7 @@ class PushToTrunk(Step): self._options.c), self._side_effect_handler) finally: - self.PushTreeStatus(self._state["tree_message"]) + self.PushTreeStatus(self["tree_message"]) else: print("ToT (r%d) is ahead of the LKGR (r%d). Skipping push to trunk." % (latest, lkgr)) diff --git a/tools/push-to-trunk/common_includes.py b/tools/push-to-trunk/common_includes.py index f1d5e50..e5f5c59 100644 --- a/tools/push-to-trunk/common_includes.py +++ b/tools/push-to-trunk/common_includes.py @@ -27,6 +27,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import datetime +import json import os import re import subprocess @@ -246,17 +247,35 @@ class Step(object): assert self._side_effect_handler is not None assert isinstance(options, CommonOptions) + def __getitem__(self, key): + # Convenience method to allow direct [] access on step classes for + # manipulating the backed state dict. + return self._state[key] + + def __setitem__(self, key, value): + # Convenience method to allow direct [] access on step classes for + # manipulating the backed state dict. + self._state[key] = value + def Config(self, key): return self._config[key] def Run(self): - if self._requires: - self.RestoreIfUnset(self._requires) - if not self._state[self._requires]: - return + # Restore state. + state_file = "%s-state.json" % self._config[PERSISTFILE_BASENAME] + if not self._state and os.path.exists(state_file): + self._state.update(json.loads(FileToText(state_file))) + + # Skip step if requirement is not met. + if self._requires and not self._state.get(self._requires): + return + print ">>> Step %d: %s" % (self._number, self._text) self.RunStep() + # Persist state. + TextToFile(json.dumps(self._state), state_file) + def RunStep(self): raise NotImplementedError @@ -349,19 +368,6 @@ class Step(object): msg = "Can't continue. Please delete branch %s and try again." % name self.Die(msg) - def Persist(self, var, value): - value = value or "__EMPTY__" - TextToFile(value, "%s-%s" % (self._config[PERSISTFILE_BASENAME], var)) - - def Restore(self, var): - value = FileToText("%s-%s" % (self._config[PERSISTFILE_BASENAME], var)) - value = value or self.Die("Variable '%s' could not be restored." % var) - return "" if value == "__EMPTY__" else value - - def RestoreIfUnset(self, var_name): - if self._state.get(var_name) is None: - self._state[var_name] = self.Restore(var_name) - def InitialEnvironmentChecks(self): # Cancel if this is not a git checkout. if not os.path.exists(self._config[DOT_GIT_LOCATION]): @@ -378,14 +384,13 @@ class Step(object): self.Die("Workspace is not clean. Please commit or undo your changes.") # Persist current branch. - current_branch = "" + self["current_branch"] = "" git_result = self.Git("status -s -b -uno").strip() for line in git_result.splitlines(): match = re.match(r"^## (.+)", line) if match: - current_branch = match.group(1) + self["current_branch"] = match.group(1) break - self.Persist("current_branch", current_branch) # Fetch unfetched revisions. if self.Git("svn fetch") is None: @@ -393,8 +398,7 @@ class Step(object): def PrepareBranch(self): # Get ahold of a safe temporary branch and check it out. - self.RestoreIfUnset("current_branch") - if self._state["current_branch"] != self._config[TEMP_BRANCH]: + if self["current_branch"] != self._config[TEMP_BRANCH]: self.DeleteBranch(self._config[TEMP_BRANCH]) self.Git("checkout -b %s" % self._config[TEMP_BRANCH]) @@ -402,11 +406,10 @@ class Step(object): self.DeleteBranch(self._config[BRANCHNAME]) def CommonCleanup(self): - self.RestoreIfUnset("current_branch") - self.Git("checkout -f %s" % self._state["current_branch"]) - if self._config[TEMP_BRANCH] != self._state["current_branch"]: + self.Git("checkout -f %s" % self["current_branch"]) + if self._config[TEMP_BRANCH] != self["current_branch"]: self.Git("branch -D %s" % self._config[TEMP_BRANCH]) - if self._config[BRANCHNAME] != self._state["current_branch"]: + if self._config[BRANCHNAME] != self["current_branch"]: self.Git("branch -D %s" % self._config[BRANCHNAME]) # Clean up all temporary files. @@ -417,8 +420,7 @@ class Step(object): match = re.match(r"^#define %s\s+(\d*)" % def_name, line) if match: value = match.group(1) - self.Persist("%s%s" % (prefix, var_name), value) - self._state["%s%s" % (prefix, var_name)] = value + self["%s%s" % (prefix, var_name)] = value for line in LinesInFile(self._config[VERSION_FILE]): for (var_name, def_name) in [("major", "MAJOR_VERSION"), ("minor", "MINOR_VERSION"), @@ -426,10 +428,6 @@ class Step(object): ("patch", "PATCH_LEVEL")]: ReadAndPersist(var_name, def_name) - def RestoreVersionIfUnset(self, prefix=""): - for v in ["major", "minor", "build", "patch"]: - self.RestoreIfUnset("%s%s" % (prefix, v)) - def WaitForLGTM(self): print ("Please wait for an LGTM, then type \"LGTM\" to commit " "your change. (If you need to iterate on the patch or double check " @@ -509,6 +507,9 @@ def RunScript(step_classes, config, options, side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER): + state_file = "%s-state.json" % config[PERSISTFILE_BASENAME] + if options.s == 0 and os.path.exists(state_file): + os.remove(state_file) state = {} steps = [] for (number, step_class) in enumerate(step_classes): diff --git a/tools/push-to-trunk/merge_to_branch.py b/tools/push-to-trunk/merge_to_branch.py index 7d5f53f..8e126b1 100755 --- a/tools/push-to-trunk/merge_to_branch.py +++ b/tools/push-to-trunk/merge_to_branch.py @@ -76,9 +76,9 @@ class Preparation(Step): self.InitialEnvironmentChecks() if self._options.revert_bleeding_edge: - self.Persist("merge_to_branch", "bleeding_edge") + self["merge_to_branch"] = "bleeding_edge" elif self._options.args[0]: - self.Persist("merge_to_branch", self._options.args[0]) + self["merge_to_branch"] = self._options.args[0] self._options.args = self._options.args[1:] else: self.Die("Please specify a branch to merge to") @@ -91,9 +91,8 @@ class CreateBranch(Step): MESSAGE = "Create a fresh branch for the patch." def RunStep(self): - self.RestoreIfUnset("merge_to_branch") args = "checkout -b %s svn/%s" % (self.Config(BRANCHNAME), - self._state["merge_to_branch"]) + self["merge_to_branch"]) if self.Git(args) is None: self.die("Creating branch %s failed." % self.Config(BRANCHNAME)) @@ -102,9 +101,9 @@ class SearchArchitecturePorts(Step): MESSAGE = "Search for corresponding architecture ports." def RunStep(self): - full_revision_list = list(OrderedDict.fromkeys(self._options.args)) + self["full_revision_list"] = list(OrderedDict.fromkeys(self._options.args)) port_revision_list = [] - for revision in full_revision_list: + for revision in self["full_revision_list"]: # Search for commits which matches the "Port rXXX" pattern. args = ("log svn/bleeding_edge --reverse " "--format=%%H --grep=\"Port r%d\"" % int(revision)) @@ -117,7 +116,7 @@ class SearchArchitecturePorts(Step): revision_title = self.Git("log -1 --format=%%s %s" % git_hash) # Is this revision included in the original revision list? - if svn_revision in full_revision_list: + if svn_revision in self["full_revision_list"]: print("Found port of r%s -> r%s (already included): %s" % (revision, svn_revision, revision_title)) else: @@ -130,76 +129,61 @@ class SearchArchitecturePorts(Step): if self.Confirm("Automatically add corresponding ports (%s)?" % ", ".join(port_revision_list)): #: 'y': Add ports to revision list. - full_revision_list.extend(port_revision_list) - self.Persist("full_revision_list", ",".join(full_revision_list)) + self["full_revision_list"].extend(port_revision_list) class FindGitRevisions(Step): MESSAGE = "Find the git revisions associated with the patches." def RunStep(self): - self.RestoreIfUnset("full_revision_list") - self.RestoreIfUnset("merge_to_branch") - full_revision_list = self._state["full_revision_list"].split(",") - patch_commit_hashes = [] - for revision in full_revision_list: + self["patch_commit_hashes"] = [] + for revision in self["full_revision_list"]: next_hash = self.Git("svn find-rev \"r%s\" svn/bleeding_edge" % revision) if not next_hash: self.Die("Cannot determine git hash for r%s" % revision) - patch_commit_hashes.append(next_hash) + self["patch_commit_hashes"].append(next_hash) # Stringify: [123, 234] -> "r123, r234" - revision_list = ", ".join(map(lambda s: "r%s" % s, full_revision_list)) + self["revision_list"] = ", ".join(map(lambda s: "r%s" % s, + self["full_revision_list"])) - if not revision_list: + if not self["revision_list"]: self.Die("Revision list is empty.") if self._options.revert: if not self._options.revert_bleeding_edge: - new_commit_msg = ("Rollback of %s in %s branch." - % (revision_list, self._state["merge_to_branch"])) + self["new_commit_msg"] = ("Rollback of %s in %s branch." + % (self["revision_list"], self["merge_to_branch"])) else: - new_commit_msg = "Revert %s ." % revision_list + self["new_commit_msg"] = "Revert %s." % self["revision_list"] else: - new_commit_msg = ("Merged %s into %s branch." - % (revision_list, self._state["merge_to_branch"])) - new_commit_msg += "\n\n" + self["new_commit_msg"] = ("Merged %s into %s branch." + % (self["revision_list"], self["merge_to_branch"])) + self["new_commit_msg"] += "\n\n" - for commit_hash in patch_commit_hashes: + for commit_hash in self["patch_commit_hashes"]: patch_merge_desc = self.Git("log -1 --format=%%s %s" % commit_hash) - new_commit_msg += "%s\n\n" % patch_merge_desc.strip() + self["new_commit_msg"] += "%s\n\n" % patch_merge_desc.strip() bugs = [] - for commit_hash in patch_commit_hashes: + for commit_hash in self["patch_commit_hashes"]: msg = self.Git("log -1 %s" % commit_hash) for bug in re.findall(r"^[ \t]*BUG[ \t]*=[ \t]*(.*?)[ \t]*$", msg, re.M): bugs.extend(map(lambda s: s.strip(), bug.split(","))) bug_aggregate = ",".join(sorted(bugs)) if bug_aggregate: - new_commit_msg += "BUG=%s\nLOG=N\n" % bug_aggregate - TextToFile(new_commit_msg, self.Config(COMMITMSG_FILE)) - self.Persist("new_commit_msg", new_commit_msg) - self.Persist("revision_list", revision_list) - self._state["patch_commit_hashes"] = patch_commit_hashes - self.Persist("patch_commit_hashes_list", " ".join(patch_commit_hashes)) + self["new_commit_msg"] += "BUG=%s\nLOG=N\n" % bug_aggregate + TextToFile(self["new_commit_msg"], self.Config(COMMITMSG_FILE)) class ApplyPatches(Step): MESSAGE = "Apply patches for selected revisions." def RunStep(self): - self.RestoreIfUnset("merge_to_branch") - self.RestoreIfUnset("patch_commit_hashes_list") - patch_commit_hashes = self._state.get("patch_commit_hashes") - if not patch_commit_hashes: - patch_commit_hashes = ( - self._state.get("patch_commit_hashes_list").strip().split(" ")) - if not patch_commit_hashes and not options.patch: - self.Die("Variable patch_commit_hashes could not be restored.") - for commit_hash in patch_commit_hashes: + for commit_hash in self["patch_commit_hashes"]: print("Applying patch for %s to %s..." - % (commit_hash, self._state["merge_to_branch"])) + % (commit_hash, self["merge_to_branch"])) patch = self.Git("log -1 -p %s" % commit_hash) TextToFile(patch, self.Config(TEMPORARY_PATCH_FILE)) self.ApplyPatch(self.Config(TEMPORARY_PATCH_FILE), self._options.revert) @@ -223,8 +207,7 @@ class IncrementVersion(Step): def RunStep(self): if self._options.revert_bleeding_edge: return - self.RestoreIfUnset("patch") - new_patch = str(int(self._state["patch"]) + 1) + new_patch = str(int(self["patch"]) + 1) if self.Confirm("Automatically increment PATCH_LEVEL? (Saying 'n' will " "fire up your EDITOR on %s so you can make arbitrary " "changes. When you're done, save the file and exit your " @@ -251,7 +234,6 @@ class CommitRepository(Step): MESSAGE = "Commit to the repository." def RunStep(self): - self.RestoreIfUnset("merge_to_branch") if self.Git("checkout %s" % self.Config(BRANCHNAME)) is None: self.Die("Cannot ensure that the current branch is %s" % self.Config(BRANCHNAME)) @@ -270,18 +252,16 @@ class PrepareSVN(Step): def RunStep(self): if self._options.revert_bleeding_edge: return - self.RestoreIfUnset("new_commit_msg") - self.RestoreIfUnset("merge_to_branch") if self.Git("svn fetch") is None: self.Die("'git svn fetch' failed.") args = ("log -1 --format=%%H --grep=\"%s\" svn/%s" - % (self._state["new_commit_msg"], self._state["merge_to_branch"])) + % (self["new_commit_msg"], self["merge_to_branch"])) commit_hash = self.Git(args).strip() if not commit_hash: self.Die("Unable to map git commit to svn revision.") - svn_revision = self.Git("svn find-rev %s" % commit_hash).strip() - print "subversion revision number is r%s" % svn_revision - self.Persist("svn_revision", svn_revision) + self["svn_revision"] = self.Git( + "svn find-rev %s" % commit_hash).strip() + print "subversion revision number is r%s" % self["svn_revision"] class TagRevision(Step): @@ -290,45 +270,34 @@ class TagRevision(Step): def RunStep(self): if self._options.revert_bleeding_edge: return - self.RestoreVersionIfUnset("new_") - self.RestoreIfUnset("svn_revision") - self.RestoreIfUnset("merge_to_branch") - ver = "%s.%s.%s.%s" % (self._state["new_major"], - self._state["new_minor"], - self._state["new_build"], - self._state["new_patch"]) - print "Creating tag svn/tags/%s" % ver - if self._state["merge_to_branch"] == "trunk": - to_url = "trunk" + self["version"] = "%s.%s.%s.%s" % (self["new_major"], + self["new_minor"], + self["new_build"], + self["new_patch"]) + print "Creating tag svn/tags/%s" % self["version"] + if self["merge_to_branch"] == "trunk": + self["to_url"] = "trunk" else: - to_url = "branches/%s" % self._state["merge_to_branch"] + self["to_url"] = "branches/%s" % self["merge_to_branch"] self.SVN("copy -r %s https://v8.googlecode.com/svn/%s " "https://v8.googlecode.com/svn/tags/%s -m " "\"Tagging version %s\"" - % (self._state["svn_revision"], to_url, ver, ver)) - self.Persist("to_url", to_url) + % (self["svn_revision"], self["to_url"], + self["version"], self["version"])) class CleanUp(Step): MESSAGE = "Cleanup." def RunStep(self): - self.RestoreIfUnset("svn_revision") - self.RestoreIfUnset("to_url") - self.RestoreIfUnset("revision_list") - self.RestoreVersionIfUnset("new_") - ver = "%s.%s.%s.%s" % (self._state["new_major"], - self._state["new_minor"], - self._state["new_build"], - self._state["new_patch"]) self.CommonCleanup() if not self._options.revert_bleeding_edge: print "*** SUMMARY ***" - print "version: %s" % ver - print "branch: %s" % self._state["to_url"] - print "svn revision: %s" % self._state["svn_revision"] - if self._state["revision_list"]: - print "patches: %s" % self._state["revision_list"] + print "version: %s" % self["version"] + print "branch: %s" % self["to_url"] + print "svn revision: %s" % self["svn_revision"] + if self["revision_list"]: + print "patches: %s" % self["revision_list"] def RunMergeToBranch(config, diff --git a/tools/push-to-trunk/push_to_trunk.py b/tools/push-to-trunk/push_to_trunk.py index 71a037c..fc8399a 100755 --- a/tools/push-to-trunk/push_to_trunk.py +++ b/tools/push-to-trunk/push_to_trunk.py @@ -109,8 +109,7 @@ class DetectLastPush(Step): break args = "log -1 --format=%H %s^ ChangeLog" % last_push last_push = self.Git(args).strip() - self.Persist("last_push", last_push) - self._state["last_push"] = last_push + self["last_push"] = last_push class PrepareChangeLog(Step): @@ -124,7 +123,8 @@ class PrepareChangeLog(Step): match = re.search(r"^Review URL: https://codereview\.chromium\.org/(\d+)$", body, flags=re.M) if match: - cl_url = "https://codereview.chromium.org/%s/description" % match.group(1) + cl_url = ("https://codereview.chromium.org/%s/description" + % match.group(1)) try: # Fetch from Rietveld but only retry once with one second delay since # there might be many revisions. @@ -134,20 +134,17 @@ class PrepareChangeLog(Step): return body def RunStep(self): - self.RestoreIfUnset("last_push") - # These version numbers are used again later for the trunk commit. self.ReadAndPersistVersion() - - date = self.GetDate() - self.Persist("date", date) - output = "%s: Version %s.%s.%s\n\n" % (date, - self._state["major"], - self._state["minor"], - self._state["build"]) + self["date"] = self.GetDate() + self["version"] = "%s.%s.%s" % (self["major"], + self["minor"], + self["build"]) + output = "%s: Version %s\n\n" % (self["date"], + self["version"]) TextToFile(output, self.Config(CHANGELOG_ENTRY_FILE)) - args = "log %s..HEAD --format=%%H" % self._state["last_push"] + args = "log %s..HEAD --format=%%H" % self["last_push"] commits = self.Git(args).strip() # Cache raw commit messages. @@ -208,8 +205,7 @@ class IncrementVersion(Step): MESSAGE = "Increment version number." def RunStep(self): - self.RestoreIfUnset("build") - new_build = str(int(self._state["build"]) + 1) + new_build = str(int(self["build"]) + 1) if self.Confirm(("Automatically increment BUILD_NUMBER? (Saying 'n' will " "fire up your EDITOR on %s so you can make arbitrary " @@ -230,12 +226,10 @@ class CommitLocal(Step): MESSAGE = "Commit to local branch." def RunStep(self): - self.RestoreVersionIfUnset("new_") - prep_commit_msg = ("Prepare push to trunk. " - "Now working on version %s.%s.%s." % (self._state["new_major"], - self._state["new_minor"], - self._state["new_build"])) - self.Persist("prep_commit_msg", prep_commit_msg) + self["prep_commit_msg"] = ("Prepare push to trunk. " + "Now working on version %s.%s.%s." % (self["new_major"], + self["new_minor"], + self["new_build"])) # Include optional TBR only in the git command. The persisted commit # message is used for finding the commit again later. @@ -243,7 +237,8 @@ class CommitLocal(Step): review = "\n\nTBR=%s" % self._options.reviewer else: review = "" - if self.Git("commit -a -m \"%s%s\"" % (prep_commit_msg, review)) is None: + if self.Git("commit -a -m \"%s%s\"" + % (self["prep_commit_msg"], review)) is None: self.Die("'git commit -a' failed.") @@ -273,10 +268,8 @@ class StragglerCommits(Step): if self.Git("svn fetch") is None: self.Die("'git svn fetch' failed.") self.Git("checkout svn/bleeding_edge") - self.RestoreIfUnset("prep_commit_msg") - args = "log -1 --format=%%H --grep=\"%s\"" % self._state["prep_commit_msg"] - prepare_commit_hash = self.Git(args).strip() - self.Persist("prepare_commit_hash", prepare_commit_hash) + args = "log -1 --format=%%H --grep=\"%s\"" % self["prep_commit_msg"] + self["prepare_commit_hash"] = self.Git(args).strip() class SquashCommits(Step): @@ -285,24 +278,22 @@ class SquashCommits(Step): def RunStep(self): # Instead of relying on "git rebase -i", we'll just create a diff, because # that's easier to automate. - self.RestoreIfUnset("prepare_commit_hash") - args = "diff svn/trunk %s" % self._state["prepare_commit_hash"] + args = "diff svn/trunk %s" % self["prepare_commit_hash"] TextToFile(self.Git(args), self.Config(PATCH_FILE)) # Convert the ChangeLog entry to commit message format. - self.RestoreIfUnset("date") text = FileToText(self.Config(CHANGELOG_ENTRY_FILE)) # Remove date and trailing white space. - text = re.sub(r"^%s: " % self._state["date"], "", text.rstrip()) + text = re.sub(r"^%s: " % self["date"], "", text.rstrip()) # Retrieve svn revision for showing the used bleeding edge revision in the # commit message. - args = "svn find-rev %s" % self._state["prepare_commit_hash"] - svn_revision = self.Git(args).strip() - self.Persist("svn_revision", svn_revision) + args = "svn find-rev %s" % self["prepare_commit_hash"] + self["svn_revision"] = self.Git(args).strip() text = MSub(r"^(Version \d+\.\d+\.\d+)$", - "\\1 (based on bleeding_edge revision r%s)" % svn_revision, + ("\\1 (based on bleeding_edge revision r%s)" + % self["svn_revision"]), text) # Remove indentation and merge paragraphs into single long lines, keeping @@ -339,15 +330,14 @@ class SetVersion(Step): MESSAGE = "Set correct version for trunk." def RunStep(self): - self.RestoreVersionIfUnset() output = "" for line in FileToText(self.Config(VERSION_FILE)).splitlines(): if line.startswith("#define MAJOR_VERSION"): - line = re.sub("\d+$", self._state["major"], line) + line = re.sub("\d+$", self["major"], line) elif line.startswith("#define MINOR_VERSION"): - line = re.sub("\d+$", self._state["minor"], line) + line = re.sub("\d+$", self["minor"], line) elif line.startswith("#define BUILD_NUMBER"): - line = re.sub("\d+$", self._state["build"], line) + line = re.sub("\d+$", self["build"], line) elif line.startswith("#define PATCH_LEVEL"): line = re.sub("\d+$", "0", line) elif line.startswith("#define IS_CANDIDATE_VERSION"): @@ -386,30 +376,26 @@ class CommitSVN(Step): result = filter(lambda x: re.search(r"^Committed r[0-9]+", x), result.splitlines()) if len(result) > 0: - trunk_revision = re.sub(r"^Committed r([0-9]+)", r"\1", result[0]) + self["trunk_revision"] = re.sub(r"^Committed r([0-9]+)", r"\1",result[0]) # Sometimes grepping for the revision fails. No idea why. If you figure # out why it is flaky, please do fix it properly. - if not trunk_revision: + if not self["trunk_revision"]: print("Sorry, grepping for the SVN revision failed. Please look for it " "in the last command's output above and provide it manually (just " "the number, without the leading \"r\").") self.DieNoManualMode("Can't prompt in forced mode.") - while not trunk_revision: + while not self["trunk_revision"]: print "> ", - trunk_revision = self.ReadLine() - self.Persist("trunk_revision", trunk_revision) + self["trunk_revision"] = self.ReadLine() class TagRevision(Step): MESSAGE = "Tag the new revision." def RunStep(self): - self.RestoreVersionIfUnset() - ver = "%s.%s.%s" % (self._state["major"], - self._state["minor"], - self._state["build"]) - if self.Git("svn tag %s -m \"Tagging version %s\"" % (ver, ver), + if self.Git(("svn tag %s -m \"Tagging version %s\"" + % (self["version"], self["version"])), retry_on=lambda x: x is None) is None: self.Die("'git svn tag' failed.") @@ -418,16 +404,15 @@ class CheckChromium(Step): MESSAGE = "Ask for chromium checkout." def Run(self): - chrome_path = self._options.c - if not chrome_path: + self["chrome_path"] = self._options.c + if not self["chrome_path"]: self.DieNoManualMode("Please specify the path to a Chromium checkout in " "forced mode.") print ("Do you have a \"NewGit\" Chromium checkout and want " "this script to automate creation of the roll CL? If yes, enter the " "path to (and including) the \"src\" directory here, otherwise just " "press : "), - chrome_path = self.ReadLine() - self.Persist("chrome_path", chrome_path) + self["chrome_path"] = self.ReadLine() class SwitchChromium(Step): @@ -435,9 +420,8 @@ class SwitchChromium(Step): REQUIRES = "chrome_path" def RunStep(self): - v8_path = os.getcwd() - self.Persist("v8_path", v8_path) - os.chdir(self._state["chrome_path"]) + self["v8_path"] = os.getcwd() + os.chdir(self["chrome_path"]) self.InitialEnvironmentChecks() # Check for a clean workdir. if self.Git("status -s -uno").strip() != "": @@ -452,14 +436,13 @@ class UpdateChromiumCheckout(Step): REQUIRES = "chrome_path" def RunStep(self): - os.chdir(self._state["chrome_path"]) + os.chdir(self["chrome_path"]) if self.Git("checkout master") is None: self.Die("'git checkout master' failed.") if self.Git("pull") is None: self.Die("'git pull' failed, please try again.") - self.RestoreIfUnset("trunk_revision") - args = "checkout -b v8-roll-%s" % self._state["trunk_revision"] + args = "checkout -b v8-roll-%s" % self["trunk_revision"] if self.Git(args) is None: self.Die("Failed to checkout a new branch.") @@ -469,20 +452,15 @@ class UploadCL(Step): REQUIRES = "chrome_path" def RunStep(self): - os.chdir(self._state["chrome_path"]) + os.chdir(self["chrome_path"]) # Patch DEPS file. - self.RestoreIfUnset("trunk_revision") deps = FileToText(self.Config(DEPS_FILE)) deps = re.sub("(?<=\"v8_revision\": \")([0-9]+)(?=\")", - self._state["trunk_revision"], + self["trunk_revision"], deps) TextToFile(deps, self.Config(DEPS_FILE)) - self.RestoreVersionIfUnset() - ver = "%s.%s.%s" % (self._state["major"], - self._state["minor"], - self._state["build"]) if self._options.reviewer: print "Using account %s for review." % self._options.reviewer rev = self._options.reviewer @@ -490,10 +468,9 @@ class UploadCL(Step): print "Please enter the email address of a reviewer for the roll CL: ", self.DieNoManualMode("A reviewer must be specified in forced mode.") rev = self.ReadLine() - self.RestoreIfUnset("svn_revision") args = ("commit -am \"Update V8 to version %s " "(based on bleeding_edge revision r%s).\n\nTBR=%s\"" - % (ver, self._state["svn_revision"], rev)) + % (self["version"], self["svn_revision"], rev)) if self.Git(args) is None: self.Die("'git commit' failed.") author_option = self._options.author @@ -510,33 +487,27 @@ class SwitchV8(Step): REQUIRES = "chrome_path" def RunStep(self): - self.RestoreIfUnset("v8_path") - os.chdir(self._state["v8_path"]) + os.chdir(self["v8_path"]) class CleanUp(Step): MESSAGE = "Done!" def RunStep(self): - self.RestoreVersionIfUnset() - ver = "%s.%s.%s" % (self._state["major"], - self._state["minor"], - self._state["build"]) - self.RestoreIfUnset("trunk_revision") - self.RestoreIfUnset("chrome_path") - - if self._state["chrome_path"]: + if self["chrome_path"]: print("Congratulations, you have successfully created the trunk " "revision %s and rolled it into Chromium. Please don't forget to " - "update the v8rel spreadsheet:" % ver) + "update the v8rel spreadsheet:" % self["version"]) else: print("Congratulations, you have successfully created the trunk " "revision %s. Please don't forget to roll this new version into " - "Chromium, and to update the v8rel spreadsheet:" % ver) - print "%s\ttrunk\t%s" % (ver, self._state["trunk_revision"]) + "Chromium, and to update the v8rel spreadsheet:" + % self["version"]) + print "%s\ttrunk\t%s" % (self["version"], + self["trunk_revision"]) self.CommonCleanup() - if self.Config(TRUNKBRANCH) != self._state["current_branch"]: + if self.Config(TRUNKBRANCH) != self["current_branch"]: self.Git("branch -D %s" % self.Config(TRUNKBRANCH)) diff --git a/tools/push-to-trunk/test_scripts.py b/tools/push-to-trunk/test_scripts.py index 242efba..668ced6 100644 --- a/tools/push-to-trunk/test_scripts.py +++ b/tools/push-to-trunk/test_scripts.py @@ -298,6 +298,7 @@ class ScriptTest(unittest.TestCase): def MakeStep(self, step_class=Step, state=None, options=None): """Convenience wrapper.""" options = options or CommonOptions(MakeOptions()) + state = state if state is not None else self._state return MakeStep(step_class=step_class, number=0, state=state, config=TEST_CONFIG, options=options, side_effect_handler=self) @@ -356,6 +357,7 @@ class ScriptTest(unittest.TestCase): self._rl_mock = SimpleMock("readline") self._url_mock = SimpleMock("readurl") self._tmp_files = [] + self._state = {} def tearDown(self): Command("rm", "-rf %s*" % TEST_CONFIG[PERSISTFILE_BASENAME]) @@ -369,12 +371,6 @@ class ScriptTest(unittest.TestCase): self._rl_mock.AssertFinished() self._url_mock.AssertFinished() - def testPersistRestore(self): - self.MakeStep().Persist("test1", "") - self.assertEquals("", self.MakeStep().Restore("test1")) - self.MakeStep().Persist("test2", "AB123") - self.assertEquals("AB123", self.MakeStep().Restore("test2")) - def testGitOrig(self): self.assertTrue(Command("git", "--version").startswith("git version")) @@ -396,7 +392,7 @@ class ScriptTest(unittest.TestCase): self.ExpectReadline(["Y"]) self.MakeStep().CommonPrepare() self.MakeStep().PrepareBranch() - self.assertEquals("some_branch", self.MakeStep().Restore("current_branch")) + self.assertEquals("some_branch", self._state["current_branch"]) def testCommonPrepareNoConfirm(self): self.ExpectGit([ @@ -408,7 +404,7 @@ class ScriptTest(unittest.TestCase): self.ExpectReadline(["n"]) self.MakeStep().CommonPrepare() self.assertRaises(Exception, self.MakeStep().PrepareBranch) - self.assertEquals("some_branch", self.MakeStep().Restore("current_branch")) + self.assertEquals("some_branch", self._state["current_branch"]) def testCommonPrepareDeleteBranchFailure(self): self.ExpectGit([ @@ -421,7 +417,7 @@ class ScriptTest(unittest.TestCase): self.ExpectReadline(["Y"]) self.MakeStep().CommonPrepare() self.assertRaises(Exception, self.MakeStep().PrepareBranch) - self.assertEquals("some_branch", self.MakeStep().Restore("current_branch")) + self.assertEquals("some_branch", self._state["current_branch"]) def testInitialEnvironmentChecks(self): TEST_CONFIG[DOT_GIT_LOCATION] = self.MakeEmptyTempFile() @@ -432,14 +428,10 @@ class ScriptTest(unittest.TestCase): TEST_CONFIG[VERSION_FILE] = self.MakeTempVersionFile() step = self.MakeStep() step.ReadAndPersistVersion() - self.assertEquals("3", self.MakeStep().Restore("major")) - self.assertEquals("22", self.MakeStep().Restore("minor")) - self.assertEquals("5", self.MakeStep().Restore("build")) - self.assertEquals("0", self.MakeStep().Restore("patch")) - self.assertEquals("3", step._state["major"]) - self.assertEquals("22", step._state["minor"]) - self.assertEquals("5", step._state["build"]) - self.assertEquals("0", step._state["patch"]) + self.assertEquals("3", step["major"]) + self.assertEquals("22", step["minor"]) + self.assertEquals("5", step["build"]) + self.assertEquals("0", step["patch"]) def testRegex(self): self.assertEqual("(issue 321)", @@ -490,7 +482,7 @@ class ScriptTest(unittest.TestCase): "Title\n\nBUG=456\nLOG=N\n\n"], ]) - self.MakeStep().Persist("last_push", "1234") + self._state["last_push"] = "1234" self.MakeStep(PrepareChangeLog).Run() actual_cl = FileToText(TEST_CONFIG[CHANGELOG_ENTRY_FILE]) @@ -522,10 +514,10 @@ class ScriptTest(unittest.TestCase): #""" self.assertEquals(expected_cl, actual_cl) - self.assertEquals("3", self.MakeStep().Restore("major")) - self.assertEquals("22", self.MakeStep().Restore("minor")) - self.assertEquals("5", self.MakeStep().Restore("build")) - self.assertEquals("0", self.MakeStep().Restore("patch")) + self.assertEquals("3", self._state["major"]) + self.assertEquals("22", self._state["minor"]) + self.assertEquals("5", self._state["build"]) + self.assertEquals("0", self._state["patch"]) def testEditChangeLog(self): TEST_CONFIG[CHANGELOG_ENTRY_FILE] = self.MakeEmptyTempFile() @@ -545,7 +537,7 @@ class ScriptTest(unittest.TestCase): def testIncrementVersion(self): TEST_CONFIG[VERSION_FILE] = self.MakeTempVersionFile() - self.MakeStep().Persist("build", "5") + self._state["build"] = "5" self.ExpectReadline([ "Y", # Increment build number. @@ -553,10 +545,10 @@ class ScriptTest(unittest.TestCase): self.MakeStep(IncrementVersion).Run() - self.assertEquals("3", self.MakeStep().Restore("new_major")) - self.assertEquals("22", self.MakeStep().Restore("new_minor")) - self.assertEquals("6", self.MakeStep().Restore("new_build")) - self.assertEquals("0", self.MakeStep().Restore("new_patch")) + self.assertEquals("3", self._state["new_major"]) + self.assertEquals("22", self._state["new_minor"]) + self.assertEquals("6", self._state["new_build"]) + self.assertEquals("0", self._state["new_patch"]) def testLastChangeLogEntries(self): TEST_CONFIG[CHANGELOG_FILE] = self.MakeEmptyTempFile() @@ -584,8 +576,8 @@ class ScriptTest(unittest.TestCase): ["svn find-rev hash1", "123455\n"], ]) - self.MakeStep().Persist("prepare_commit_hash", "hash1") - self.MakeStep().Persist("date", "1999-11-11") + self._state["prepare_commit_hash"] = "hash1" + self._state["date"] = "1999-11-11" self.MakeStep(SquashCommits).Run() self.assertEquals(FileToText(TEST_CONFIG[COMMITMSG_FILE]), expected_msg) @@ -810,8 +802,11 @@ Performance and stability improvements on all platforms.""", commit) auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions( MakeOptions(status_password=status_password)), self) - self.assertEquals("100", self.MakeStep().Restore("lkgr")) - self.assertEquals("100", self.MakeStep().Restore("latest")) + state = json.loads(FileToText("%s-state.json" + % TEST_CONFIG[PERSISTFILE_BASENAME])) + + self.assertEquals("100", state["lkgr"]) + self.assertEquals("100", state["latest"]) def testAutoRollStoppedBySettings(self): TEST_CONFIG[DOT_GIT_LOCATION] = self.MakeEmptyTempFile() @@ -906,6 +901,10 @@ LOG=N ["svn find-rev hash3 svn/bleeding_edge", "56789"], ["log -1 --format=%s hash3", "Title3"], ["svn find-rev \"r12345\" svn/bleeding_edge", "hash4"], + # Simulate svn being down which stops the script. + ["svn find-rev \"r23456\" svn/bleeding_edge", None], + # Restart script in the failing step. + ["svn find-rev \"r12345\" svn/bleeding_edge", "hash4"], ["svn find-rev \"r23456\" svn/bleeding_edge", "hash2"], ["svn find-rev \"r34567\" svn/bleeding_edge", "hash3"], ["svn find-rev \"r45678\" svn/bleeding_edge", "hash1"], @@ -964,6 +963,15 @@ LOG=N # ports of r12345. r56789 is the MIPS port of r34567. args = ["trunk", "12345", "23456", "34567"] self.assertTrue(merge_to_branch.ProcessOptions(options, args)) + + # The first run of the script stops because of the svn being down. + self.assertRaises(Exception, + lambda: RunMergeToBranch(TEST_CONFIG, + MergeToBranchOptions(options, args), + self)) + + # Test that state recovery after restarting the script works. + options.s = 3 RunMergeToBranch(TEST_CONFIG, MergeToBranchOptions(options, args), self)