From: Douglas Bagnall Date: Wed, 7 Sep 2022 03:34:23 +0000 (+1200) Subject: samba-tool: separate ._run() from command resolution X-Git-Tag: talloc-2.4.0~1128 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9ec0863ff244494ba1b3fdc1b2d3be38e195e146;p=thirdparty%2Fsamba.git samba-tool: separate ._run() from command resolution Prior to this commit, in super-commands, the first half of the _run() is resolving what sub-command to run, and the second half is working out what to print if that failed. Some issues with that are: * it looks a little bit complicated. * the tests can't use the tool's resolution code, because it runs immediately, while the tests first want to fiddle with self.outf and so on. * it makes it harder to subclass and override the resolution code, so instead we do strange things like where we subclass dict as in main.py. So we split it into ._resolve() and ._run(). There are a few tests that break. We mark these as flapping, rather than knownfail, so as to avoid going into extremely fine-grain filters for tests that will be fixed within a few commits. Signed-off-by: Douglas Bagnall Reviewed-by: Andrew Bartlett --- diff --git a/python/samba/netcmd/__init__.py b/python/samba/netcmd/__init__.py index 21738cf51fb..a2ad180b2c3 100644 --- a/python/samba/netcmd/__init__.py +++ b/python/samba/netcmd/__init__.py @@ -158,9 +158,15 @@ class Command(object): def message(self, text): self.outf.write(text + "\n") + + def _resolve(self, path, *argv): + """This is a leaf node, the command that will actually run.""" + self.command_name = path + return (self, argv) + def _run(self, *argv): - parser, optiongroups = self._create_parser(argv[0]) - opts, args = parser.parse_args(list(argv)) + parser, optiongroups = self._create_parser(self.command_name) + opts, args = parser.parse_args([self.command_name] + list(argv)) # Filter out options from option groups args = args[1:] kwargs = dict(opts.__dict__) @@ -229,43 +235,60 @@ class SuperCommand(Command): subcommands = {} - def _run(self, myname, subcommand=None, *args): - if subcommand in self.subcommands: - return self.subcommands[subcommand]._run( - "%s %s" % (myname, subcommand), *args) - elif subcommand not in [ '--help', 'help', None, '-h', '-V', '--version' ]: - print("%s: no such subcommand: %s\n" % (myname, subcommand)) - args = [] - - if subcommand == 'help': - # pass the request down - if len(args) > 0: - sub = self.subcommands.get(args[0]) - if isinstance(sub, SuperCommand): - return sub._run("%s %s" % (myname, args[0]), 'help', - *args[1:]) - elif sub is not None: - return sub._run("%s %s" % (myname, args[0]), '--help', - *args[1:]) - - subcommand = '--help' + def _resolve(self, path, *args): + """This is an internal node. We need to consume one of the args and + find the relevant child, returning an instance of that Command. + + If there are no children, this SuperCommand will be returned + and its _run() will do a --help like thing. + """ + self.command_name = path + + # We collect up certain option arguments and pass them to the + # leaf, which is why we iterate over args, though we really + # expect to return in the first iteration. + deferred_args = [] + + for i, a in enumerate(args): + if a in self.subcommands: + sub_args = args[i + 1:] + tuple(deferred_args) + sub_path = f'{path} {a}' + + sub = self.subcommands[a] + return sub._resolve(sub_path, *sub_args) + elif a in [ '--help', 'help', None, '-h', '-V', '--version' ]: + # we pass these to the leaf node. + if a == 'help': + a = '--help' + deferred_args.append(a) + continue + + # they are talking nonsense + print("%s: no such subcommand: %s\n" % (path, a), file=self.outf) + return (self, []) + + # We didn't find a subcommand, but maybe we found e.g. --version + print("%s: missing subcommand\n" % (path), file=self.outf) + return (self, deferred_args) + + def _run(self, *argv): epilog = "\nAvailable subcommands:\n" subcmds = list(self.subcommands.keys()) subcmds.sort() max_length = max([len(c) for c in subcmds]) for cmd_name in subcmds: cmd = self.subcommands[cmd_name] - if not cmd.hidden: - epilog += " %*s - %s\n" % ( - -max_length, cmd_name, cmd.short_description) - epilog += "For more help on a specific subcommand, please type: %s (-h|--help)\n" % myname - - parser, optiongroups = self._create_parser(myname, epilog=epilog) - args_list = list(args) - if subcommand: - args_list.insert(0, subcommand) - opts, args = parser.parse_args(args_list) + if cmd.hidden: + continue + epilog += " %*s - %s\n" % ( + -max_length, cmd_name, cmd.short_description) + + epilog += ("For more help on a specific subcommand, please type: " + f"{self.command_name} (-h|--help)\n") + + parser, optiongroups = self._create_parser(self.command_name, epilog=epilog) + opts, args = parser.parse_args([self.command_name] + list(argv)) parser.print_help() return -1 diff --git a/selftest/flapping.d/samba-tool b/selftest/flapping.d/samba-tool new file mode 100644 index 00000000000..f277c8e9096 --- /dev/null +++ b/selftest/flapping.d/samba-tool @@ -0,0 +1,3 @@ +^samba.tests.netcmd. +^samba.tests.blackbox.samba_dnsupdate +^samba4.ldap.password_lockout.python diff --git a/source4/scripting/bin/samba-tool b/source4/scripting/bin/samba-tool index f8a70a6b295..b5f784beff7 100755 --- a/source4/scripting/bin/samba-tool +++ b/source4/scripting/bin/samba-tool @@ -32,16 +32,10 @@ signal.signal(signal.SIGINT, signal.SIG_DFL) from samba.netcmd.main import cmd_sambatool cmd = cmd_sambatool() -subcommand = None -args = () - -if len(sys.argv) > 1: - subcommand = sys.argv[1] - if len(sys.argv) > 2: - args = sys.argv[2:] try: - retval = cmd._run("samba-tool", subcommand, *args) + command, args = cmd._resolve("samba-tool", *sys.argv[1:]) + retval = command._run(*args) except SystemExit as e: retval = e.code except Exception as e: diff --git a/source4/scripting/bin/samba_dnsupdate b/source4/scripting/bin/samba_dnsupdate index 1ce53f5dc39..0db490e21e9 100755 --- a/source4/scripting/bin/samba_dnsupdate +++ b/source4/scripting/bin/samba_dnsupdate @@ -590,7 +590,8 @@ def call_samba_tool(d, op="add", zone=None): cmd = cmd_dns() if opts.verbose: print(f'Calling samba-tool dns {op} --use-kerberos off -P {args}') - ret = cmd._run("dns", op, "--use-kerberos", "off", "-P", *args) + command, args = cmd._resolve("dns", op, "--use-kerberos", "off", "-P", *args) + ret = command._run(*args) if ret == -1: if opts.fail_immediately: sys.exit(1)