Revision: 19565
Author: [email protected]
Date: Wed Feb 26 15:13:31 2014 UTC
Log: Refactoring: Deprecate optparse in push and merge scripts.
- Deprecate optparse with argparse
- The tests include now options parsing by default: each test specifies the
command-line args to parse rather than the options directly
This CL is split off from https://codereview.chromium.org/173983002/
[email protected]
Review URL: https://codereview.chromium.org/181583002
http://code.google.com/p/v8/source/detail?r=19565
Modified:
/branches/bleeding_edge/tools/push-to-trunk/auto_roll.py
/branches/bleeding_edge/tools/push-to-trunk/merge_to_branch.py
/branches/bleeding_edge/tools/push-to-trunk/push_to_trunk.py
/branches/bleeding_edge/tools/push-to-trunk/test_scripts.py
=======================================
--- /branches/bleeding_edge/tools/push-to-trunk/auto_roll.py Thu Feb 20
16:39:41 2014 UTC
+++ /branches/bleeding_edge/tools/push-to-trunk/auto_roll.py Wed Feb 26
15:13:31 2014 UTC
@@ -26,8 +26,8 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+import argparse
import json
-import optparse
import os
import re
import sys
@@ -180,28 +180,28 @@
def BuildOptions():
- result = optparse.OptionParser()
- result.add_option("-a", "--author", dest="a",
- help=("Specify the author email used for rietveld."))
- result.add_option("-c", "--chromium", dest="c",
- help=("Specify the path to your Chromium src/ "
- "directory to automate the V8 roll."))
- result.add_option("-p", "--push",
- help="Push to trunk if possible. Dry run if
unspecified.",
- default=False, action="store_true")
- result.add_option("-r", "--reviewer",
- help=("Specify the account name to be used for
reviews."))
- result.add_option("-s", "--step", dest="s",
- help="Specify the step where to start work. Default:
0.",
- default=0, type="int")
- result.add_option("--status-password",
- help="A file with the password to the status app.")
- return result
+ parser = argparse.ArgumentParser()
+ parser.add_argument("-a", "--author", dest="a",
+ help="The author email used for rietveld.")
+ parser.add_argument("-c", "--chromium", dest="c",
+ help=("The path to your Chromium src/ directory to "
+ "automate the V8 roll."))
+ parser.add_argument("-p", "--push",
+ help="Push to trunk if possible. Dry run if
unspecified.",
+ default=False, action="store_true")
+ parser.add_argument("-r", "--reviewer",
+ help="The account name to be used for reviews.")
+ parser.add_argument("-s", "--step", dest="s",
+ help="Specify the step where to start work. Default:
0.",
+ default=0, type=int)
+ parser.add_argument("--status-password",
+ help="A file with the password to the status app.")
+ return parser
def Main():
parser = BuildOptions()
- (options, args) = parser.parse_args()
+ options = parser.parse_args()
if not options.a or not options.c or not options.reviewer:
print "You need to specify author, chromium src location and reviewer."
parser.print_help()
=======================================
--- /branches/bleeding_edge/tools/push-to-trunk/merge_to_branch.py Thu Feb
20 16:39:41 2014 UTC
+++ /branches/bleeding_edge/tools/push-to-trunk/merge_to_branch.py Wed Feb
26 15:13:31 2014 UTC
@@ -26,8 +26,8 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+import argparse
from collections import OrderedDict
-import optparse
import sys
from common_includes import *
@@ -51,7 +51,7 @@
class MergeToBranchOptions(CommonOptions):
- def __init__(self, options, args):
+ def __init__(self, options):
super(MergeToBranchOptions, self).__init__(options, True)
self.requires_editor = True
self.wait_for_lgtm = True
@@ -60,7 +60,8 @@
self.revert = getattr(options, "r", False)
self.revert_bleeding_edge = getattr(options, "revert_bleeding_edge",
False)
self.patch = getattr(options, "p", "")
- self.args = args
+ self.branch = options.branch
+ self.revisions = options.revisions
class Preparation(Step):
@@ -77,9 +78,8 @@
self.InitialEnvironmentChecks()
if self._options.revert_bleeding_edge:
self["merge_to_branch"] = "bleeding_edge"
- elif self._options.args[0]:
- self["merge_to_branch"] = self._options.args[0]
- self._options.args = self._options.args[1:]
+ elif self._options.branch:
+ self["merge_to_branch"] = self._options.branch
else:
self.Die("Please specify a branch to merge to")
@@ -99,7 +99,8 @@
MESSAGE = "Search for corresponding architecture ports."
def RunStep(self):
- self["full_revision_list"] =
list(OrderedDict.fromkeys(self._options.args))
+ self["full_revision_list"] = list(OrderedDict.fromkeys(
+ self._options.revisions))
port_revision_list = []
for revision in self["full_revision_list"]:
# Search for commits which matches the "Port rXXX" pattern.
@@ -310,53 +311,53 @@
def BuildOptions():
- result = optparse.OptionParser()
- result.set_usage("""%prog [OPTIONS]... [BRANCH] [REVISION]...
-
-Performs the necessary steps to merge revisions from bleeding_edge
-to other branches, including trunk.""")
- result.add_option("-f",
- help="Delete sentinel file.",
- default=False, action="store_true")
- result.add_option("-m", "--message",
- help="Specify a commit message for the patch.")
- result.add_option("-r", "--revert",
- help="Revert specified patches.",
- default=False, action="store_true")
- result.add_option("-R", "--revert-bleeding-edge",
- help="Revert specified patches from bleeding edge.",
- default=False, action="store_true")
- result.add_option("-p", "--patch", dest="p",
- help="Specify a patch file to apply as part of the
merge.")
- result.add_option("-s", "--step", dest="s",
- help="Specify the step where to start work. Default:
0.",
- default=0, type="int")
- return result
+ parser = argparse.ArgumentParser(
+ description=("Performs the necessary steps to merge revisions from "
+ "bleeding_edge to other branches, including trunk."))
+ group = parser.add_mutually_exclusive_group(required=True)
+ group.add_argument("--branch", help="The branch to merge to.")
+ group.add_argument("-R", "--revert-bleeding-edge",
+ help="Revert specified patches from bleeding edge.",
+ default=False, action="store_true")
+ parser.add_argument("revisions", nargs="*",
+ help="The revisions to merge.")
+ parser.add_argument("-a", "--author", default="",
+ help="The author email used for rietveld.")
+ parser.add_argument("-f",
+ help="Delete sentinel file.",
+ default=False, action="store_true")
+ parser.add_argument("-m", "--message",
+ help="A commit message for the patch.")
+ parser.add_argument("-r", "--revert",
+ help="Revert specified patches.",
+ default=False, action="store_true")
+ parser.add_argument("-p", "--patch", dest="p",
+ help="A patch file to apply as part of the merge.")
+ parser.add_argument("-s", "--step", dest="s",
+ help="The step where to start work. Default: 0.",
+ default=0, type=int)
+ return parser
-def ProcessOptions(options, args):
- revert_from_bleeding_edge = 1 if options.revert_bleeding_edge else 0
- min_exp_args = 2 - revert_from_bleeding_edge
- if len(args) < min_exp_args:
- if not options.p:
+def ProcessOptions(options):
+ # TODO(machenbach): Add a test that covers revert from bleeding_edge
+ if len(options.revisions) < 1:
+ if not options.patch:
print "Either a patch file or revision numbers must be specified"
return False
if not options.message:
print "You must specify a merge comment if no patches are specified"
return False
- if options.s < 0:
- print "Bad step number %d" % options.s
- return False
return True
def Main():
parser = BuildOptions()
- (options, args) = parser.parse_args()
- if not ProcessOptions(options, args):
+ options = parser.parse_args()
+ if not ProcessOptions(options):
parser.print_help()
return 1
- RunMergeToBranch(CONFIG, MergeToBranchOptions(options, args))
+ RunMergeToBranch(CONFIG, MergeToBranchOptions(options))
if __name__ == "__main__":
sys.exit(Main())
=======================================
--- /branches/bleeding_edge/tools/push-to-trunk/push_to_trunk.py Thu Feb 20
16:39:41 2014 UTC
+++ /branches/bleeding_edge/tools/push-to-trunk/push_to_trunk.py Wed Feb 26
15:13:31 2014 UTC
@@ -26,7 +26,7 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-import optparse
+import argparse
import sys
import tempfile
import urllib2
@@ -545,32 +545,31 @@
def BuildOptions():
- result = optparse.OptionParser()
- result.add_option("-a", "--author", dest="a",
- help=("Specify the author email used for rietveld."))
- result.add_option("-b", "--last-bleeding-edge", dest="b",
- help=("Manually specify the git commit ID of the last "
- "bleeding edge revision that was pushed to
trunk. "
- "This is used for the auto-generated ChangeLog "
- "entry."))
- result.add_option("-c", "--chromium", dest="c",
- help=("Specify the path to your Chromium src/ "
- "directory to automate the V8 roll."))
- result.add_option("-f", "--force", dest="f",
- help="Don't prompt the user.",
- default=False, action="store_true")
- result.add_option("-l", "--last-push", dest="l",
- help=("Manually specify the git commit ID "
- "of the last push to trunk."))
- result.add_option("-m", "--manual", dest="m",
- help="Prompt the user at every important step.",
- default=False, action="store_true")
- result.add_option("-r", "--reviewer",
- help=("Specify the account name to be used for
reviews."))
- result.add_option("-s", "--step", dest="s",
- help="Specify the step where to start work. Default:
0.",
- default=0, type="int")
- return result
+ parser = argparse.ArgumentParser()
+ group = parser.add_mutually_exclusive_group()
+ group.add_argument("-f", "--force", dest="f",
+ help="Don't prompt the user.",
+ default=False, action="store_true")
+ group.add_argument("-m", "--manual", dest="m",
+ help="Prompt the user at every important step.",
+ default=False, action="store_true")
+ parser.add_argument("-a", "--author", dest="a",
+ help="The author email used for rietveld.")
+ parser.add_argument("-b", "--last-bleeding-edge", dest="b",
+ help=("The git commit ID of the last bleeding edge "
+ "revision that was pushed to trunk. This is
used "
+ "for the auto-generated ChangeLog entry."))
+ parser.add_argument("-c", "--chromium", dest="c",
+ help=("The path to your Chromium src/ directory to "
+ "automate the V8 roll."))
+ parser.add_argument("-l", "--last-push", dest="l",
+ help="The git commit ID of the last push to trunk.")
+ parser.add_argument("-r", "--reviewer",
+ help="The account name to be used for reviews.")
+ parser.add_argument("-s", "--step", dest="s",
+ help="The step where to start work. Default: 0.",
+ default=0, type=int)
+ return parser
def ProcessOptions(options):
@@ -580,9 +579,6 @@
if not options.m and not options.reviewer:
print "A reviewer (-r) is required in (semi-)automatic mode."
return False
- if options.f and options.m:
- print "Manual and forced mode cannot be combined."
- return False
if not options.m and not options.c:
print "A chromium checkout (-c) is required in (semi-)automatic mode."
return False
@@ -594,7 +590,7 @@
def Main():
parser = BuildOptions()
- (options, args) = parser.parse_args()
+ options = parser.parse_args()
if not ProcessOptions(options):
parser.print_help()
return 1
=======================================
--- /branches/bleeding_edge/tools/push-to-trunk/test_scripts.py Thu Feb 20
16:39:41 2014 UTC
+++ /branches/bleeding_edge/tools/push-to-trunk/test_scripts.py Wed Feb 26
15:13:31 2014 UTC
@@ -64,6 +64,12 @@
TEMPORARY_PATCH_FILE: "/tmp/test-merge-to-branch-tempfile-temporary-patch",
}
+AUTO_ROLL_ARGS = [
+ "-a", "[email protected]",
+ "-c", TEST_CONFIG[CHROMIUM],
+ "-r", "[email protected]",
+]
+
def MakeOptions(s=0, l=None, f=False, m=True, r=None, c=None, a=None,
status_password=None, revert_bleeding_edge=None, p=None):
@@ -740,9 +746,11 @@
if force:
self.ExpectReadline([])
- options = MakeOptions(f=force, m=manual, a="[email protected]",
- r="[email protected]" if not manual else
None,
- c = TEST_CONFIG[CHROMIUM])
+ args = ["-a", "[email protected]", "-c", TEST_CONFIG[CHROMIUM]]
+ if force: args.append("-f")
+ if manual: args.append("-m")
+ else: args += ["-r", "[email protected]"]
+ options = push_to_trunk.BuildOptions().parse_args(args)
RunPushToTrunk(TEST_CONFIG, PushToTrunkOptions(options), self)
deps = FileToText(TEST_CONFIG[DEPS_FILE])
@@ -777,8 +785,8 @@
self.assertRaises(Exception, self.MakeStep(CheckLastPush,
state=state).Run)
def testAutoRoll(self):
- status_password = self.MakeEmptyTempFile()
- TextToFile("PW", status_password)
+ password = self.MakeEmptyTempFile()
+ TextToFile("PW", password)
TEST_CONFIG[DOT_GIT_LOCATION] = self.MakeEmptyTempFile()
TEST_CONFIG[SETTINGS_LOCATION] = "~/.doesnotexist"
@@ -807,8 +815,9 @@
["svn find-rev push_hash", "65"],
])
- auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(
- MakeOptions(status_password=status_password)), self)
+ options = auto_roll.BuildOptions().parse_args(
+ AUTO_ROLL_ARGS + ["--status-password", password])
+ auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(options), self)
state = json.loads(FileToText("%s-state.json"
% TEST_CONFIG[PERSISTFILE_BASENAME]))
@@ -829,8 +838,9 @@
["svn fetch", ""],
])
+ options = auto_roll.BuildOptions().parse_args(AUTO_ROLL_ARGS)
def RunAutoRoll():
- auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(MakeOptions()),
self)
+ auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(options), self)
self.assertRaises(Exception, RunAutoRoll)
def testAutoRollStoppedByTreeStatus(self):
@@ -848,8 +858,9 @@
["svn fetch", ""],
])
+ options = auto_roll.BuildOptions().parse_args(AUTO_ROLL_ARGS)
def RunAutoRoll():
- auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(MakeOptions()),
self)
+ auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(options), self)
self.assertRaises(Exception, RunAutoRoll)
def testMergeToBranch(self):
@@ -966,21 +977,22 @@
"LGTM", # Enter LGTM for V8 CL.
])
- options = MakeOptions(p=extra_patch, f=True)
# r12345 and r34567 are patches. r23456 (included) and r45678 are the
MIPS
# ports of r12345. r56789 is the MIPS port of r34567.
- args = ["trunk", "12345", "23456", "34567"]
- self.assertTrue(merge_to_branch.ProcessOptions(options, args))
+ args = ["-f", "-p", extra_patch, "--branch", "trunk", "12345", "23456",
+ "34567"]
+ options = merge_to_branch.BuildOptions().parse_args(args)
+ self.assertTrue(merge_to_branch.ProcessOptions(options))
# The first run of the script stops because of the svn being down.
self.assertRaises(GitFailedException,
lambda: RunMergeToBranch(TEST_CONFIG,
- MergeToBranchOptions(options, args),
+ MergeToBranchOptions(options),
self))
# Test that state recovery after restarting the script works.
options.s = 3
- RunMergeToBranch(TEST_CONFIG, MergeToBranchOptions(options, args),
self)
+ RunMergeToBranch(TEST_CONFIG, MergeToBranchOptions(options), self)
class SystemTest(unittest.TestCase):
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
