The branch, master has been updated via 3e95c677f24 pytests:s4/dsdb/passwords: avoid unused imports via 884f1052149 pytests:s4/drs/getnc_schema: avoid unused imports via 1cf48a588fc pytests:s4/drs/repl_move: avoid unused and star imports via 7283fed0b35 pytests:s4/drs/repl_rodc: avoid unused imports via 7f9fedd744c pytests:s4/drs/linked_attributes_drs: avoid unused imports via b1ff59fb8b7 pytests:s4/drs/ridalloc_exop: avoid unused imports via 3c5cb27885a pytests: remove backwards compat workaround for python 2.6 via 2775d6b5d1c pytest: samba-tool visualize: improve a message via ed72ec76313 samba-tool: no stack trace on missing ldb tdb via b350a9c37c9 samba-tool: write ERROR in red if colour is wanted via a64e6c9639c samba-tool visualize: simplify --color-scheme calculations via 07cbb10dc07 samba-tool visualise: use global --color via adf8b8b4a16 py:colour: is_colour_wanted() can take filenames via c0d0c13670a samba-tool: --color=auto looks at stderr and stdout via 7d4387d15df samba-tool drs showrepl: use global --color option via baf7c5c585d samba-tool: save --color choice for subcommands via 5dd4696fb79 samba-tool: make --color a general option via 4c623356ce5 py:colour: colour_if_wanted() returns the result via 4f30d06a365 pytest: samba-tool visualize: fix filename via 3119349a3f1 libcli/auth/proto.h: remove unneeded path details. via 53f6dbe03f7 ldb: ldb_build_search_req() check for a talloc failure via 9983ea0ed26 s4/server: stop suggesting ntvfs in error message via 1f60e881973 libaddns: remove duplicate declaration via eab89c8e29d pytest/password_lockout: be less verbose by default via 7af1326a58e samba-tool: simplify and clarify SuperCommand._run() a little from 4f5b4bd9dfb ctdb-tests: Reformat remaining test stubs with "shfmt -w -p -i 0 -fn"
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 3e95c677f242b28eaa031ed402a28dbdc0958d9f Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 16 11:42:48 2022 +1200 pytests:s4/dsdb/passwords: avoid unused imports Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Autobuild-User(master): Andrew Bartlett <abart...@samba.org> Autobuild-Date(master): Fri Sep 16 06:47:43 UTC 2022 on sn-devel-184 commit 884f105214973d0b414fdf2b3be6eaff4c75512c Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 16 11:42:14 2022 +1200 pytests:s4/drs/getnc_schema: avoid unused imports Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 1cf48a588fc440eba665b27cf5d8f56264d2ca51 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 16 11:41:39 2022 +1200 pytests:s4/drs/repl_move: avoid unused and star imports Found the names using something like: flake8 repl_move.py | \ grep -oP "(?<=F405 ')[\w.]+" /tmp/repl_move | sort | uniq Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 7283fed0b3524cd00d256eb1a9292685e0f9b43a Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 16 11:38:40 2022 +1200 pytests:s4/drs/repl_rodc: avoid unused imports Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 7f9fedd744c1f5144518efbe975330ea0df1cfd0 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 16 11:38:08 2022 +1200 pytests:s4/drs/linked_attributes_drs: avoid unused imports Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit b1ff59fb8b729f07836c4953a77eb710dc361f4c Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 16 11:37:14 2022 +1200 pytests:s4/drs/ridalloc_exop: avoid unused imports Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 3c5cb27885a542e0c0ba80e6c9b776859a29d2ff Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 16 11:36:28 2022 +1200 pytests: remove backwards compat workaround for python 2.6 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 2775d6b5d1c92aa72d02bde617927020cd8a79a2 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Sep 14 21:12:47 2022 +1200 pytest: samba-tool visualize: improve a message Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit ed72ec763133b3ed17a9f75bf4ae0bf0782c2967 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 9 16:13:12 2022 +1200 samba-tool: no stack trace on missing ldb tdb Now, in a testenv, if you forget to use '-s st/ad_dc/etc/smb.conf', you only see this: $ bin/samba-tool user rename dsadsa ldb: Unable to open tdb '$HERE/st/client/private/secrets.ldb': No such file or directory ldb: Failed to connect to '$HERE/st/client/private/secrets.ldb' with backend 'tdb': Unable to open tdb '$HERE/st/client/private/secrets.ldb': No such file or directory Could not find machine account in secrets database: Failed to fetch machine account password from secrets.ldb: Could not open secrets.ldb and failed to open $HERE/st/client/private/secrets.tdb: NT_STATUS_CANT_ACCESS_DOMAIN_INFO ltdb: tdb($HERE/st/client/private/sam.ldb): tdb_open_ex: could not open file $HERE/st/client/private/sam.ldb: No such file or directory Unable to open tdb '$HERE/st/client/private/sam.ldb': No such file or directory Failed to connect to 'tdb://$HERE/st/client/private/sam.ldb' with backend 'tdb': Unable to open tdb '$HERE/st/client/private/sam.ldb': No such file or directory ERROR(ldb): uncaught exception - Unable to open tdb '$HERE/st/client/private/sam.ldb': No such file or directory rather than all that AND a stack trace. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit b350a9c37c997eed219d22f7ae010358b620fef4 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 9 15:08:30 2022 +1200 samba-tool: write ERROR in red if colour is wanted Often we'll write something like ERROR: Unable to find user "potato" which can get lost in the jumble of other output. With this patch, we colour the word "ERROR" red but not the rest of the string, unless it is determined that colour is not wanted (due to one of --color=never, NO_COLOR=1, output is not a tty). We choose to redden the word "ERROR" only to maintain legibility in the actual message, while hopefully increasing the noticeability of the line. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit a64e6c9639ce9162d615fbb2f1f0349e1bd9720e Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Sep 14 18:23:16 2022 +1200 samba-tool visualize: simplify --color-scheme calculations If you ask for a --color-scheme, you are implicitly asking for --color. That was documented in --help, but not followed here. Now --color=no --color-scheme=ansi will use colour for the graph, but not for other output. This might be useful when the graph is going to a different place than everything else (`-o foo.txt > bar.txt`). Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 07cbb10dc07381df6409f12ca0b4ecb6911ce495 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 9 14:56:08 2022 +1200 samba-tool visualise: use global --color Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit adf8b8b4a16493d2e0c2f33c00e7a4970b8a9c2a Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sat Sep 10 16:55:48 2022 +1200 py:colour: is_colour_wanted() can take filenames We need this for `samba-tool visualize -o -` which means output to stdout, and which has always had a tty test for colour. Rather than continue to duplicate the full logic there, we can reuse this. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit c0d0c13670a1082427c1a62f1fd36c0e0a672a9f Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 9 15:24:29 2022 +1200 samba-tool: --color=auto looks at stderr and stdout More often than not we are using colour in stderr, but are deciding based on stdout's tty-ness. This patch changes to use both, and will affect the following situation: samba-tool 2>/tmp/errors # used to be colour, now not. of course, if you want colour, you can always samba-tool --color=yes 2>/tmp/errors Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 7d4387d15dff755a57724d4df5e25b75ae5bee6b Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 9 14:50:13 2022 +1200 samba-tool drs showrepl: use global --color option This changes the default from --color=no to --color=auto. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit baf7c5c585de35a01699a1b0e18bbb339c14afa0 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 9 14:38:18 2022 +1200 samba-tool: save --color choice for subcommands In particular, visualize needs it to decide colour for an output file that may or may not be stdout, so it needs to make its own decision for that file. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 5dd4696fb792cff37534eccb943be66cdd9e544c Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 9 14:48:29 2022 +1200 samba-tool: make --color a general option We don't put --color into options.SambaOptions because we can't handle the 'auto' case in the options module without knowing whether or not self.outf is a tty, and a) this might not be resolved and b) is fiddly to pass through. The .use_colour class flag allows samba-tool subcommands to avoid having --color, and is *also* useful in the short term for visualise and drs commands to avoid having this --color clobber their own bespoke versions (temporarily, during the transition). Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 4c623356ce547ea2dd4d9055ef9162f227d4cabd Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Sep 9 14:35:12 2022 +1200 py:colour: colour_if_wanted() returns the result Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 4f30d06a365540aa237976f4807e23b9455e9c90 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Sep 14 17:36:08 2022 +1200 pytest: samba-tool visualize: fix filename Overwriting the other file was harmless but misleading. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 3119349a3f1973697980aff0a012dff92be3402a Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Dec 17 14:34:50 2020 +1300 libcli/auth/proto.h: remove unneeded path details. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 53f6dbe03f7389242a6ebfaddc90bc39865b17fc Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Aug 31 15:42:46 2022 +1200 ldb: ldb_build_search_req() check for a talloc failure The failure in question would have to be a `talloc_strdup(dn, "")` in ldb_dn_from_ldb_val(). Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 9983ea0ed26fb61b205b369796b26e701c546b85 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Aug 17 10:12:28 2022 +1200 s4/server: stop suggesting ntvfs in error message I am not sure about the rpc proxy. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 1f60e881973ea8faffbd136971c3ae3f3dd233a5 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Jul 2 15:45:45 2021 +1200 libaddns: remove duplicate declaration Also declared on line 257, exactly the same. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit eab89c8e29d77922420f345ae0198425ad0ac937 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Sep 8 14:32:13 2022 +1200 pytest/password_lockout: be less verbose by default leaving the carefully constructed verbosity there for whoever choses to switch it on. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 7af1326a58ed371209b82f561a6720df2c893849 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Sep 7 15:41:17 2022 +1200 samba-tool: simplify and clarify SuperCommand._run() a little Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> ----------------------------------------------------------------------- Summary of changes: lib/addns/dns.h | 2 - lib/ldb/common/ldb.c | 4 ++ libcli/auth/msrpc_parse.h | 2 +- libcli/auth/proto.h | 8 ++-- python/samba/colour.py | 27 ++++++++--- python/samba/netcmd/__init__.py | 48 +++++++++++++++---- python/samba/netcmd/drs.py | 5 +- python/samba/netcmd/visualize.py | 54 +++++++++------------- python/samba/tests/__init__.py | 7 +-- python/samba/tests/samba_tool/visualize.py | 5 +- source4/dsdb/tests/python/password_lockout.py | 18 ++++---- source4/dsdb/tests/python/password_lockout_base.py | 30 +++++++----- source4/dsdb/tests/python/passwords.py | 6 +-- source4/samba/server.c | 3 +- source4/torture/drs/python/getnc_schema.py | 4 -- .../torture/drs/python/linked_attributes_drs.py | 16 +------ source4/torture/drs/python/repl_move.py | 36 ++++++++++++++- source4/torture/drs/python/repl_rodc.py | 2 - source4/torture/drs/python/ridalloc_exop.py | 3 -- 19 files changed, 163 insertions(+), 117 deletions(-) Changeset truncated at 500 lines: diff --git a/lib/addns/dns.h b/lib/addns/dns.h index de1897b6e87..685cded966b 100644 --- a/lib/addns/dns.h +++ b/lib/addns/dns.h @@ -285,8 +285,6 @@ DNS_ERROR dns_create_tsig_record(TALLOC_CTX *mem_ctx, const char *keyname, uint16_t mac_length, const uint8_t *mac, uint16_t original_id, uint16_t error, struct dns_rrec **prec); -DNS_ERROR dns_add_rrec(TALLOC_CTX *mem_ctx, struct dns_rrec *rec, - uint16_t *num_records, struct dns_rrec ***records); DNS_ERROR dns_create_update_request(TALLOC_CTX *mem_ctx, const char *domainname, const char *hostname, diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c index c2599a096b9..6145bc7e500 100644 --- a/lib/ldb/common/ldb.c +++ b/lib/ldb/common/ldb.c @@ -1467,6 +1467,10 @@ int ldb_build_search_req_ex(struct ldb_request **ret_req, req->operation = LDB_SEARCH; if (base == NULL) { req->op.search.base = ldb_dn_new(req, ldb, NULL); + if (req->op.search.base == NULL) { + ldb_oom(ldb); + return LDB_ERR_OPERATIONS_ERROR; + } } else { req->op.search.base = base; } diff --git a/libcli/auth/msrpc_parse.h b/libcli/auth/msrpc_parse.h index 47529f24a95..717dec9eb7c 100644 --- a/libcli/auth/msrpc_parse.h +++ b/libcli/auth/msrpc_parse.h @@ -30,7 +30,7 @@ * used outside this particular subsystem! */ -/* The following definitions come from /home/jeremy/src/samba/git/master/source3/../source4/../libcli/auth/msrpc_parse.c */ +/* The following definitions come from libcli/auth/msrpc_parse.c */ NTSTATUS msrpc_gen(TALLOC_CTX *mem_ctx, DATA_BLOB *blob, diff --git a/libcli/auth/proto.h b/libcli/auth/proto.h index baf57308c9f..f6ca2f1632d 100644 --- a/libcli/auth/proto.h +++ b/libcli/auth/proto.h @@ -11,7 +11,7 @@ * used outside this particular subsystem! */ -/* The following definitions come from /home/jeremy/src/samba/git/master/source3/../source4/../libcli/auth/credentials.c */ +/* The following definitions come from libcli/auth/credentials.c */ bool netlogon_creds_is_random_challenge(const struct netr_Credential *challenge); void netlogon_creds_random_challenge(struct netr_Credential *challenge); @@ -91,7 +91,7 @@ union netr_LogonLevel *netlogon_creds_shallow_copy_logon(TALLOC_CTX *mem_ctx, enum netr_LogonInfoClass level, const union netr_LogonLevel *in); -/* The following definitions come from /home/jeremy/src/samba/git/master/source3/../source4/../libcli/auth/session.c */ +/* The following definitions come from libcli/auth/session.c */ int sess_crypt_blob(DATA_BLOB *out, const DATA_BLOB *in, const DATA_BLOB *session_key, enum samba_gnutls_direction encrypt); @@ -102,7 +102,7 @@ DATA_BLOB sess_encrypt_blob(TALLOC_CTX *mem_ctx, DATA_BLOB *blob_in, const DATA_ NTSTATUS sess_decrypt_blob(TALLOC_CTX *mem_ctx, const DATA_BLOB *blob, const DATA_BLOB *session_key, DATA_BLOB *ret); -/* The following definitions come from /home/jeremy/src/samba/git/master/source3/../source4/../libcli/auth/smbencrypt.c */ +/* The following definitions come from libcli/auth/smbencrypt.c */ int SMBencrypt_hash(const uint8_t lm_hash[16], const uint8_t *c8, uint8_t p24[24]); bool SMBencrypt(const char *passwd, const uint8_t *c8, uint8_t p24[24]); @@ -275,7 +275,7 @@ WERROR decode_wkssvc_join_password_buffer(TALLOC_CTX *mem_ctx, DATA_BLOB *session_key, char **pwd); -/* The following definitions come from /home/jeremy/src/samba/git/master/source3/../source4/../libcli/auth/smbdes.c */ +/* The following definitions come from libcli/auth/smbdes.c */ int des_crypt56_gnutls(uint8_t out[8], const uint8_t in[8], const uint8_t key[7], enum samba_gnutls_direction encrypt); diff --git a/python/samba/colour.py b/python/samba/colour.py index 448b456b465..553e637d3a4 100644 --- a/python/samba/colour.py +++ b/python/samba/colour.py @@ -90,9 +90,12 @@ def xterm_256_colour(n, bg=False, bold=False): return "\033[%s%s;5;%dm" % (weight, target, int(n)) -def is_colour_wanted(stream, hint='auto'): +def is_colour_wanted(*streams, hint='auto'): """The hint is presumably a --color argument. + The streams to be considered can be file objects or file names, + with '-' being a special filename indicating stdout. + We follow the behaviour of GNU `ls` in what we accept. * `git` is stricter, accepting only {always,never,auto}. * `grep` is looser, accepting mixed case variants. @@ -116,13 +119,25 @@ def is_colour_wanted(stream, hint='auto'): # Note: per spec, we treat the empty string as if unset. return False - if (hasattr(stream, 'isatty') and stream.isatty()): - return True - return False + for stream in streams: + if isinstance(stream, str): + # This function can be passed filenames instead of file + # objects, in which case we treat '-' as stdout, and test + # that. Any other string is not regarded as a tty. + if stream != '-': + return False + import sys + stream = sys.stdout + + if not stream.isatty(): + return False + return True -def colour_if_wanted(stream, hint='auto'): - if is_colour_wanted(stream, hint): +def colour_if_wanted(*streams, hint='auto'): + wanted = is_colour_wanted(*streams, hint=hint) + if wanted: switch_colour_on() else: switch_colour_off() + return wanted diff --git a/python/samba/netcmd/__init__.py b/python/samba/netcmd/__init__.py index 874f553e910..e69da388f36 100644 --- a/python/samba/netcmd/__init__.py +++ b/python/samba/netcmd/__init__.py @@ -84,6 +84,8 @@ class Command(object): takes_optiongroups = {} hidden = False + use_colour = True + requested_colour = None raw_argv = None raw_args = None @@ -102,6 +104,15 @@ class Command(object): parser, _ = self._create_parser(prog) parser.print_usage() + def _print_error(self, msg, evalue=None, klass=None): + err = colour.c_DARK_RED("ERROR") + klass = '' if klass is None else f'({klass})' + + if evalue is None: + print(f"{err}{klass}: {msg}", file=self.errf) + else: + print(f"{err}{klass}: {msg} - {evalue}", file=self.errf) + def show_command_error(self, e): '''display a command error''' if isinstance(e, CommandError): @@ -128,20 +139,24 @@ class Command(object): elif ldb_emsg == 'LDAP client internal error: NT_STATUS_NETWORK_UNREACHABLE': print("Could not reach remote server", file=self.errf) force_traceback = False + elif ldb_emsg.startswith("Unable to open tdb "): + self._print_error(message, ldb_emsg, 'ldb') + force_traceback = False else: - self.errf.write("ERROR(ldb): %s - %s\n" % (message, ldb_emsg)) + self._print_error(message, ldb_emsg, 'ldb') + elif isinstance(inner_exception, AssertionError): - self.errf.write("ERROR(assert): %s\n" % message) + self._print_error(message, klass='assert') force_traceback = True elif isinstance(inner_exception, RuntimeError): - self.errf.write("ERROR(runtime): %s - %s\n" % (message, evalue)) + self._print_error(message, evalue, 'runtime') elif type(inner_exception) is Exception: - self.errf.write("ERROR(exception): %s - %s\n" % (message, evalue)) + self._print_error(message, evalue, 'exception') force_traceback = True elif inner_exception is None: - self.errf.write("ERROR: %s\n" % (message)) + self._print_error(message) else: - self.errf.write("ERROR(%s): %s - %s\n" % (str(etype), message, evalue)) + self._print_error(message, evalue, str(etype)) if force_traceback or samba.get_debug_level() >= 3: traceback.print_tb(etraceback, file=self.errf) @@ -158,6 +173,12 @@ class Command(object): optiongroup = self.takes_optiongroups[name] optiongroups[name] = optiongroup(parser) parser.add_option_group(optiongroups[name]) + if self.use_colour: + parser.add_option("--color", + help="use colour if available (default: auto)", + metavar="always|never|auto", + default="auto") + return parser, optiongroups def message(self, text): @@ -180,6 +201,9 @@ class Command(object): del kwargs[option.dest] kwargs.update(optiongroups) + if self.use_colour: + self.apply_colour_choice(kwargs.pop('color', 'auto')) + # Check for a min a max number of allowed arguments, whenever possible # The suffix "?" means zero or one occurence # The suffix "+" means at least one occurence @@ -225,8 +249,11 @@ class Command(object): "constants" in the colour module to be either real colours or empty strings. """ + self.requested_colour = requested try: - colour.colour_if_wanted(self.outf, requested) + colour.colour_if_wanted(self.outf, + self.errf, + hint=requested) except ValueError as e: raise CommandError(f"Unknown --color option: {requested} " "please choose from always|never|auto") @@ -279,8 +306,8 @@ class SuperCommand(Command): def _run(self, *argv): epilog = "\nAvailable subcommands:\n" - subcmds = list(self.subcommands.keys()) - subcmds.sort() + + subcmds = sorted(self.subcommands.keys()) max_length = max([len(c) for c in subcmds]) for cmd_name in subcmds: cmd = self.subcommands[cmd_name] @@ -295,6 +322,9 @@ class SuperCommand(Command): parser, optiongroups = self._create_parser(self.command_name, epilog=epilog) opts, args = parser.parse_args(list(argv)) + # note: if argv had --help, parser.parse_args() will have + # already done the .print_help() and attempted to exit with + # return code 0, so we won't get here. parser.print_help() return -1 diff --git a/python/samba/netcmd/drs.py b/python/samba/netcmd/drs.py index 52620090a64..6a4063a91d6 100644 --- a/python/samba/netcmd/drs.py +++ b/python/samba/netcmd/drs.py @@ -117,8 +117,6 @@ class cmd_drs_showrepl(Command): dest='format', action='store_const', const='classic', default=DEFAULT_SHOWREPL_FORMAT), Option("-v", "--verbose", help="Be verbose", action="store_true"), - Option("--color", help="Use colour output (yes|no|auto)", - default='no'), ] takes_args = ["DC?"] @@ -184,8 +182,7 @@ class cmd_drs_showrepl(Command): def run(self, DC=None, sambaopts=None, credopts=None, versionopts=None, format=DEFAULT_SHOWREPL_FORMAT, - verbose=False, color='no'): - self.apply_colour_choice(color) + verbose=False): self.lp = sambaopts.get_loadparm() if DC is None: DC = common.netcmd_dnsname(self.lp) diff --git a/python/samba/netcmd/visualize.py b/python/samba/netcmd/visualize.py index ea8e696ee61..7d84934c654 100644 --- a/python/samba/netcmd/visualize.py +++ b/python/samba/netcmd/visualize.py @@ -21,7 +21,6 @@ import os import sys from collections import defaultdict import subprocess - import tempfile import samba.getopt as options from samba import dsdb @@ -31,6 +30,8 @@ from samba.samdb import SamDB from samba.graph import dot_graph from samba.graph import distance_matrix, COLOUR_SETS from samba.graph import full_matrix +from samba.colour import is_colour_wanted + from ldb import SCOPE_BASE, SCOPE_SUBTREE, LdbError import time import re @@ -56,9 +57,6 @@ COMMON_OPTIONS = [ dest='format', const='distance', action='store_const'), Option("--utf8", help="Use utf-8 Unicode characters", action='store_true'), - Option("--color", help="use color (yes, no, auto)", - choices=['yes', 'no', 'auto', 'always', 'never', 'force', - 'none', 'if-tty', 'tty']), Option("--color-scheme", help=("use this colour scheme " "(implies --color=yes)"), choices=list(COLOUR_SETS.keys())), @@ -151,30 +149,27 @@ class GraphCommand(Command): subprocess.call([xdot, fn]) os.remove(fn) - def calc_distance_color_scheme(self, color, color_scheme, output): + def calc_distance_color_scheme(self, color_scheme, output): """Heuristics to work out the colour scheme for distance matrices. Returning None means no colour, otherwise it sould be a colour from graph.COLOUR_SETS""" - if color in ('no', 'never', 'none'): - return None + if color_scheme is not None: + # --color-scheme implies --color=yes for *this* purpose. + return color_scheme - if color in ('auto', 'tty', 'if-tty', None): - if os.environ.get('NO_COLOR'): - return None - if color_scheme is not None: - # --color-scheme usually implies --color=yes. - return color_scheme - if isinstance(output, str) and output != '-': - return None - if not self.outf.isatty(): - return None + if output in ('-', None): + output = self.outf - if color_scheme is None: - if '256color' in os.environ.get('TERM', ''): - return 'xterm-256color-heatmap' - return 'ansi' + want_colour = is_colour_wanted(output, hint=self.requested_colour) + if not want_colour: + return None - return color_scheme + # if we got to here, we are using colour according to the + # --color/NO_COLOR rules, but no colour scheme has been + # specified, so we choose some defaults. + if '256color' in os.environ.get('TERM', ''): + return 'xterm-256color-heatmap' + return 'ansi' def get_dnstr_site(dn): @@ -215,7 +210,7 @@ class cmd_reps(GraphCommand): def run(self, H=None, output=None, shorten_names=False, key=True, talk_to_remote=False, sambaopts=None, credopts=None, versionopts=None, - mode='self', partition=None, color=None, color_scheme=None, + mode='self', partition=None, color_scheme=None, utf8=None, format=None, xdot=False): # We use the KCC libraries in readonly mode to get the # replication graph. @@ -311,8 +306,7 @@ class cmd_reps(GraphCommand): # interpretation and presentation. if self.calc_output_format(format, output) == 'distance': - color_scheme = self.calc_distance_color_scheme(color, - color_scheme, + color_scheme = self.calc_distance_color_scheme(color_scheme, output) header_strings = { 'from': "RepsFrom objects for %s", @@ -419,7 +413,7 @@ class cmd_ntdsconn(GraphCommand): def run(self, H=None, output=None, shorten_names=False, key=True, talk_to_remote=False, sambaopts=None, credopts=None, versionopts=None, - color=None, color_scheme=None, + color_scheme=None, utf8=None, format=None, importldif=None, xdot=False): @@ -496,8 +490,7 @@ class cmd_ntdsconn(GraphCommand): vertices, rodc_status = zip(*sorted(vertices)) if self.calc_output_format(format, output) == 'distance': - color_scheme = self.calc_distance_color_scheme(color, - color_scheme, + color_scheme = self.calc_distance_color_scheme(color_scheme, output) colours = COLOUR_SETS[color_scheme] c_header = colours.get('header', '') @@ -654,7 +647,7 @@ class cmd_uptodateness(GraphCommand): def run(self, H=None, output=None, shorten_names=False, key=True, talk_to_remote=False, sambaopts=None, credopts=None, versionopts=None, - color=None, color_scheme=None, + color_scheme=None, utf8=False, format=None, importldif=None, xdot=False, partition=None, max_digits=3): if not talk_to_remote: @@ -671,8 +664,7 @@ class cmd_uptodateness(GraphCommand): partition = get_partition(self.samdb, partition) short_partitions, long_partitions = get_partition_maps(self.samdb) - color_scheme = self.calc_distance_color_scheme(color, - color_scheme, + color_scheme = self.calc_distance_color_scheme(color_scheme, output) for part_name, part_dn in short_partitions.items(): diff --git a/python/samba/tests/__init__.py b/python/samba/tests/__init__.py index e27dd2ff5a5..1a8576137c5 100644 --- a/python/samba/tests/__init__.py +++ b/python/samba/tests/__init__.py @@ -51,11 +51,8 @@ import samba.ndr import samba.dcerpc.dcerpc import samba.dcerpc.epmapper -try: - from unittest import SkipTest -except ImportError: - class SkipTest(Exception): - """Test skipped.""" +from unittest import SkipTest + BINDIR = os.path.abspath(os.path.join(os.path.dirname(__file__), "../../../../bin")) diff --git a/python/samba/tests/samba_tool/visualize.py b/python/samba/tests/samba_tool/visualize.py index f4323725b75..16585268d33 100644 --- a/python/samba/tests/samba_tool/visualize.py +++ b/python/samba/tests/samba_tool/visualize.py @@ -126,7 +126,8 @@ class SambaToolVisualizeLdif(SambaToolCmdTest): '-H', self.dburl, '-S', *args) self.assertCmdSuccess(result, out, err) - self.assertTrue(colour_re.search(out)) + self.assertTrue(colour_re.search(out), + f"'{' '.join(args)}' should be colour") uncoloured = colour_re.sub('', out) self.assertStringsEqual(monochrome, uncoloured, strip=True) @@ -312,7 +313,7 @@ class SambaToolVisualizeLdif(SambaToolCmdTest): self.assertStringsEqual(color_no, expected, strip=True) - color_yes_file = os.path.join(self.tempdir, 'color-no') + color_yes_file = os.path.join(self.tempdir, 'color-yes') result, out, err = self.runsubcmd("visualize", "ntdsconn", '-H', self.dburl, '--color=yes', '-S', diff --git a/source4/dsdb/tests/python/password_lockout.py b/source4/dsdb/tests/python/password_lockout.py index 325c0cfdd01..1f7e1e1a487 100755 --- a/source4/dsdb/tests/python/password_lockout.py +++ b/source4/dsdb/tests/python/password_lockout.py @@ -195,11 +195,11 @@ userAccountControl: %d if use_kerberos == MUST_USE_KERBEROS: logoncount_relation = 'greater' lastlogon_relation = 'greater' - print("Performs a password cleartext change operation on 'userPassword' using Kerberos") + self.debug("Performs a password cleartext change operation on 'userPassword' using Kerberos") else: logoncount_relation = 'equal' lastlogon_relation = 'equal' - print("Performs a password cleartext change operation on 'userPassword' using NTLMSSP") + self.debug("Performs a password cleartext change operation on 'userPassword' using NTLMSSP") if initial_lastlogon_relation is not None: lastlogon_relation = initial_lastlogon_relation @@ -293,7 +293,7 @@ userPassword: thatsAcomplPASS2 msDSUserAccountControlComputed=0) badPasswordTime = int(res[0]["badPasswordTime"][0]) - print("two failed password change") + self.debug("two failed password change") # Wrong old password try: @@ -696,7 +696,7 @@ userPassword: thatsAcomplPASS2XYZ for i in range(lockout_threshold): badPwdCount = i + 1 try: - print("Trying bad password, attempt #%u" % badPwdCount) + self.debug("Trying bad password, attempt #%u" % badPwdCount) net.change_password(newpassword=new_password, username=creds.get_username(), oldpassword="bad-password") @@ -730,7 +730,7 @@ userPassword: thatsAcomplPASS2XYZ # good or a bad password now for password in (creds.get_password(), "bad-password"): try: - print("Trying password %s" % password) + self.debug("Trying password %s" % password) net.change_password(newpassword=new_password, username=creds.get_username(), oldpassword=password) @@ -930,7 +930,7 @@ userPassword: thatsAcomplPASS2XYZ with self.assertRaises( NTSTATUSError, msg='Invalid SAMR change_password accepted') as err: - print(f'Trying correct password, attempt #{i}') + self.debug(f'Trying correct password, attempt #{i}') net.change_password(newpassword=new_password, username=username, oldpassword=creds.get_password()) @@ -1024,7 +1024,7 @@ userPassword: {new_password} -- Samba Shared Repository