mirror of
https://gerrit.googlesource.com/git-repo
synced 2024-12-21 07:16:21 +00:00
split out cli validation from execution
A common pattern in our subcommands is to verify the arguments & options before executing things. For some subcommands, that check stage is quite long which makes the execution function even bigger. Lets split that logic out of the execute phase so it's easier to manage these. This is most noticeable in the sync subcommand whose Execute func is quite large, and the option checking makes up ~15% of it. The manifest command's Execute can be simplified significantly as the optparse configuration always sets output_file to a string. Change-Id: I7097847ff040e831345e63de6b467ee17609990e Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/234834 Reviewed-by: David Pursehouse <dpursehouse@collab.net> Tested-by: Mike Frysinger <vapier@google.com>
This commit is contained in:
parent
3fc157285c
commit
ae6cb08ae5
10
command.py
10
command.py
@ -98,6 +98,16 @@ class Command(object):
|
|||||||
self.OptionParser.print_usage()
|
self.OptionParser.print_usage()
|
||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
|
|
||||||
|
def ValidateOptions(self, opt, args):
|
||||||
|
"""Validate the user options & arguments before executing.
|
||||||
|
|
||||||
|
This is meant to help break the code up into logical steps. Some tips:
|
||||||
|
* Use self.OptionParser.error to display CLI related errors.
|
||||||
|
* Adjust opt member defaults as makes sense.
|
||||||
|
* Adjust the args list, but do so inplace so the caller sees updates.
|
||||||
|
* Try to avoid updating self state. Leave that to Execute.
|
||||||
|
"""
|
||||||
|
|
||||||
def Execute(self, opt, args):
|
def Execute(self, opt, args):
|
||||||
"""Perform the action, after option parsing is complete.
|
"""Perform the action, after option parsing is complete.
|
||||||
"""
|
"""
|
||||||
|
1
main.py
1
main.py
@ -197,6 +197,7 @@ class _Repo(object):
|
|||||||
cmd_event = cmd.event_log.Add(name, event_log.TASK_COMMAND, start)
|
cmd_event = cmd.event_log.Add(name, event_log.TASK_COMMAND, start)
|
||||||
cmd.event_log.SetParent(cmd_event)
|
cmd.event_log.SetParent(cmd_event)
|
||||||
try:
|
try:
|
||||||
|
cmd.ValidateOptions(copts, cargs)
|
||||||
result = cmd.Execute(copts, cargs)
|
result = cmd.Execute(copts, cargs)
|
||||||
except (DownloadError, ManifestInvalidRevisionError,
|
except (DownloadError, ManifestInvalidRevisionError,
|
||||||
NoManifestException) as e:
|
NoManifestException) as e:
|
||||||
|
@ -37,19 +37,19 @@ It is equivalent to "git branch -D <branchname>".
|
|||||||
dest='all', action='store_true',
|
dest='all', action='store_true',
|
||||||
help='delete all branches in all projects')
|
help='delete all branches in all projects')
|
||||||
|
|
||||||
def Execute(self, opt, args):
|
def ValidateOptions(self, opt, args):
|
||||||
if not opt.all and not args:
|
if not opt.all and not args:
|
||||||
self.Usage()
|
self.Usage()
|
||||||
|
|
||||||
if not opt.all:
|
if not opt.all:
|
||||||
nb = args[0]
|
nb = args[0]
|
||||||
if not git.check_ref_format('heads/%s' % nb):
|
if not git.check_ref_format('heads/%s' % nb):
|
||||||
print("error: '%s' is not a valid name" % nb, file=sys.stderr)
|
self.OptionParser.error("'%s' is not a valid branch name" % nb)
|
||||||
sys.exit(1)
|
|
||||||
else:
|
else:
|
||||||
args.insert(0,None)
|
args.insert(0, "'All local branches'")
|
||||||
nb = "'All local branches'"
|
|
||||||
|
|
||||||
|
def Execute(self, opt, args):
|
||||||
|
nb = args[0]
|
||||||
err = defaultdict(list)
|
err = defaultdict(list)
|
||||||
success = defaultdict(list)
|
success = defaultdict(list)
|
||||||
all_projects = self.GetProjects(args[1:])
|
all_projects = self.GetProjects(args[1:])
|
||||||
|
@ -34,10 +34,11 @@ The command is equivalent to:
|
|||||||
repo forall [<project>...] -c git checkout <branchname>
|
repo forall [<project>...] -c git checkout <branchname>
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def Execute(self, opt, args):
|
def ValidateOptions(self, opt, args):
|
||||||
if not args:
|
if not args:
|
||||||
self.Usage()
|
self.Usage()
|
||||||
|
|
||||||
|
def Execute(self, opt, args):
|
||||||
nb = args[0]
|
nb = args[0]
|
||||||
err = []
|
err = []
|
||||||
success = []
|
success = []
|
||||||
|
@ -37,10 +37,11 @@ change id will be added.
|
|||||||
def _Options(self, p):
|
def _Options(self, p):
|
||||||
pass
|
pass
|
||||||
|
|
||||||
def Execute(self, opt, args):
|
def ValidateOptions(self, opt, args):
|
||||||
if len(args) != 1:
|
if len(args) != 1:
|
||||||
self.Usage()
|
self.Usage()
|
||||||
|
|
||||||
|
def Execute(self, opt, args):
|
||||||
reference = args[0]
|
reference = args[0]
|
||||||
|
|
||||||
p = GitCommand(None,
|
p = GitCommand(None,
|
||||||
|
@ -176,10 +176,11 @@ synced and their revisions won't be found.
|
|||||||
self.printText(log)
|
self.printText(log)
|
||||||
self.out.nl()
|
self.out.nl()
|
||||||
|
|
||||||
def Execute(self, opt, args):
|
def ValidateOptions(self, opt, args):
|
||||||
if not args or len(args) > 2:
|
if not args or len(args) > 2:
|
||||||
self.Usage()
|
self.OptionParser.error('missing manifests to diff')
|
||||||
|
|
||||||
|
def Execute(self, opt, args):
|
||||||
self.out = _Coloring(self.manifest.globalConfig)
|
self.out = _Coloring(self.manifest.globalConfig)
|
||||||
self.printText = self.out.nofmt_printer('text')
|
self.printText = self.out.nofmt_printer('text')
|
||||||
if opt.color:
|
if opt.color:
|
||||||
|
@ -177,10 +177,11 @@ without iterating through the remaining projects.
|
|||||||
'worktree': project.worktree,
|
'worktree': project.worktree,
|
||||||
}
|
}
|
||||||
|
|
||||||
def Execute(self, opt, args):
|
def ValidateOptions(self, opt, args):
|
||||||
if not opt.command:
|
if not opt.command:
|
||||||
self.Usage()
|
self.Usage()
|
||||||
|
|
||||||
|
def Execute(self, opt, args):
|
||||||
cmd = [opt.command[0]]
|
cmd = [opt.command[0]]
|
||||||
|
|
||||||
shell = True
|
shell = True
|
||||||
|
@ -436,18 +436,17 @@ to update the working directory files.
|
|||||||
print(' rm -r %s/.repo' % self.manifest.topdir)
|
print(' rm -r %s/.repo' % self.manifest.topdir)
|
||||||
print('and try again.')
|
print('and try again.')
|
||||||
|
|
||||||
def Execute(self, opt, args):
|
def ValidateOptions(self, opt, args):
|
||||||
git_require(MIN_GIT_VERSION, fail=True)
|
|
||||||
|
|
||||||
if opt.reference:
|
if opt.reference:
|
||||||
opt.reference = os.path.expanduser(opt.reference)
|
opt.reference = os.path.expanduser(opt.reference)
|
||||||
|
|
||||||
# Check this here, else manifest will be tagged "not new" and init won't be
|
# Check this here, else manifest will be tagged "not new" and init won't be
|
||||||
# possible anymore without removing the .repo/manifests directory.
|
# possible anymore without removing the .repo/manifests directory.
|
||||||
if opt.archive and opt.mirror:
|
if opt.archive and opt.mirror:
|
||||||
print('fatal: --mirror and --archive cannot be used together.',
|
self.OptionParser.error('--mirror and --archive cannot be used together.')
|
||||||
file=sys.stderr)
|
|
||||||
sys.exit(1)
|
def Execute(self, opt, args):
|
||||||
|
git_require(MIN_GIT_VERSION, fail=True)
|
||||||
|
|
||||||
self._SyncManifest(opt)
|
self._SyncManifest(opt)
|
||||||
self._LinkManifest(opt.manifest_name)
|
self._LinkManifest(opt.manifest_name)
|
||||||
|
@ -49,6 +49,10 @@ This is similar to running: repo forall -c 'echo "$REPO_PATH : $REPO_PROJECT"'.
|
|||||||
dest='path_only', action='store_true',
|
dest='path_only', action='store_true',
|
||||||
help="Display only the path of the repository")
|
help="Display only the path of the repository")
|
||||||
|
|
||||||
|
def ValidateOptions(self, opt, args):
|
||||||
|
if opt.fullpath and opt.name_only:
|
||||||
|
self.OptionParser.error('cannot combine -f and -n')
|
||||||
|
|
||||||
def Execute(self, opt, args):
|
def Execute(self, opt, args):
|
||||||
"""List all projects and the associated directories.
|
"""List all projects and the associated directories.
|
||||||
|
|
||||||
@ -60,11 +64,6 @@ This is similar to running: repo forall -c 'echo "$REPO_PATH : $REPO_PROJECT"'.
|
|||||||
opt: The options.
|
opt: The options.
|
||||||
args: Positional args. Can be a list of projects to list, or empty.
|
args: Positional args. Can be a list of projects to list, or empty.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
if opt.fullpath and opt.name_only:
|
|
||||||
print('error: cannot combine -f and -n', file=sys.stderr)
|
|
||||||
sys.exit(1)
|
|
||||||
|
|
||||||
if not opt.regex:
|
if not opt.regex:
|
||||||
projects = self.GetProjects(args, groups=opt.groups)
|
projects = self.GetProjects(args, groups=opt.groups)
|
||||||
else:
|
else:
|
||||||
|
@ -73,14 +73,9 @@ in a Git repository for use during future 'repo init' invocations.
|
|||||||
if opt.output_file != '-':
|
if opt.output_file != '-':
|
||||||
print('Saved manifest to %s' % opt.output_file, file=sys.stderr)
|
print('Saved manifest to %s' % opt.output_file, file=sys.stderr)
|
||||||
|
|
||||||
def Execute(self, opt, args):
|
def ValidateOptions(self, opt, args):
|
||||||
if args:
|
if args:
|
||||||
self.Usage()
|
self.Usage()
|
||||||
|
|
||||||
if opt.output_file is not None:
|
def Execute(self, opt, args):
|
||||||
self._Output(opt)
|
self._Output(opt)
|
||||||
return
|
|
||||||
|
|
||||||
print('error: no operation to perform', file=sys.stderr)
|
|
||||||
print('error: see repo help manifest', file=sys.stderr)
|
|
||||||
sys.exit(1)
|
|
||||||
|
@ -41,15 +41,16 @@ revision specified in the manifest.
|
|||||||
dest='all', action='store_true',
|
dest='all', action='store_true',
|
||||||
help='begin branch in all projects')
|
help='begin branch in all projects')
|
||||||
|
|
||||||
def Execute(self, opt, args):
|
def ValidateOptions(self, opt, args):
|
||||||
if not args:
|
if not args:
|
||||||
self.Usage()
|
self.Usage()
|
||||||
|
|
||||||
nb = args[0]
|
nb = args[0]
|
||||||
if not git.check_ref_format('heads/%s' % nb):
|
if not git.check_ref_format('heads/%s' % nb):
|
||||||
print("error: '%s' is not a valid name" % nb, file=sys.stderr)
|
self.OptionParser.error("'%s' is not a valid name" % nb)
|
||||||
sys.exit(1)
|
|
||||||
|
|
||||||
|
def Execute(self, opt, args):
|
||||||
|
nb = args[0]
|
||||||
err = []
|
err = []
|
||||||
projects = []
|
projects = []
|
||||||
if not opt.all:
|
if not opt.all:
|
||||||
|
@ -738,6 +738,24 @@ later is required to fix a server side protocol bug.
|
|||||||
fd.close()
|
fd.close()
|
||||||
return 0
|
return 0
|
||||||
|
|
||||||
|
def ValidateOptions(self, opt, args):
|
||||||
|
if opt.force_broken:
|
||||||
|
print('warning: -f/--force-broken is now the default behavior, and the '
|
||||||
|
'options are deprecated', file=sys.stderr)
|
||||||
|
if opt.network_only and opt.detach_head:
|
||||||
|
self.OptionParser.error('cannot combine -n and -d')
|
||||||
|
if opt.network_only and opt.local_only:
|
||||||
|
self.OptionParser.error('cannot combine -n and -l')
|
||||||
|
if opt.manifest_name and opt.smart_sync:
|
||||||
|
self.OptionParser.error('cannot combine -m and -s')
|
||||||
|
if opt.manifest_name and opt.smart_tag:
|
||||||
|
self.OptionParser.error('cannot combine -m and -t')
|
||||||
|
if opt.manifest_server_username or opt.manifest_server_password:
|
||||||
|
if not (opt.smart_sync or opt.smart_tag):
|
||||||
|
self.OptionParser.error('-u and -p may only be combined with -s or -t')
|
||||||
|
if None in [opt.manifest_server_username, opt.manifest_server_password]:
|
||||||
|
self.OptionParser.error('both -u and -p must be given')
|
||||||
|
|
||||||
def Execute(self, opt, args):
|
def Execute(self, opt, args):
|
||||||
if opt.jobs:
|
if opt.jobs:
|
||||||
self.jobs = opt.jobs
|
self.jobs = opt.jobs
|
||||||
@ -745,30 +763,6 @@ later is required to fix a server side protocol bug.
|
|||||||
soft_limit, _ = _rlimit_nofile()
|
soft_limit, _ = _rlimit_nofile()
|
||||||
self.jobs = min(self.jobs, (soft_limit - 5) // 3)
|
self.jobs = min(self.jobs, (soft_limit - 5) // 3)
|
||||||
|
|
||||||
if opt.force_broken:
|
|
||||||
print('warning: -f/--force-broken is now the default behavior, and the '
|
|
||||||
'options are deprecated', file=sys.stderr)
|
|
||||||
if opt.network_only and opt.detach_head:
|
|
||||||
print('error: cannot combine -n and -d', file=sys.stderr)
|
|
||||||
sys.exit(1)
|
|
||||||
if opt.network_only and opt.local_only:
|
|
||||||
print('error: cannot combine -n and -l', file=sys.stderr)
|
|
||||||
sys.exit(1)
|
|
||||||
if opt.manifest_name and opt.smart_sync:
|
|
||||||
print('error: cannot combine -m and -s', file=sys.stderr)
|
|
||||||
sys.exit(1)
|
|
||||||
if opt.manifest_name and opt.smart_tag:
|
|
||||||
print('error: cannot combine -m and -t', file=sys.stderr)
|
|
||||||
sys.exit(1)
|
|
||||||
if opt.manifest_server_username or opt.manifest_server_password:
|
|
||||||
if not (opt.smart_sync or opt.smart_tag):
|
|
||||||
print('error: -u and -p may only be combined with -s or -t',
|
|
||||||
file=sys.stderr)
|
|
||||||
sys.exit(1)
|
|
||||||
if None in [opt.manifest_server_username, opt.manifest_server_password]:
|
|
||||||
print('error: both -u and -p must be given', file=sys.stderr)
|
|
||||||
sys.exit(1)
|
|
||||||
|
|
||||||
if opt.manifest_name:
|
if opt.manifest_name:
|
||||||
self.manifest.Override(opt.manifest_name)
|
self.manifest.Override(opt.manifest_name)
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user