[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Fri Sep 16 06:48:02 UTC 2022


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.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Fri Sep 16 06:47:43 UTC 2022 on sn-devel-184

commit 884f105214973d0b414fdf2b3be6eaff4c75512c
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 1cf48a588fc440eba665b27cf5d8f56264d2ca51
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 7283fed0b3524cd00d256eb1a9292685e0f9b43a
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 7f9fedd744c1f5144518efbe975330ea0df1cfd0
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit b1ff59fb8b729f07836c4953a77eb710dc361f4c
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 3c5cb27885a542e0c0ba80e6c9b776859a29d2ff
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 2775d6b5d1c92aa72d02bde617927020cd8a79a2
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit ed72ec763133b3ed17a9f75bf4ae0bf0782c2967
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit b350a9c37c997eed219d22f7ae010358b620fef4
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit a64e6c9639ce9162d615fbb2f1f0349e1bd9720e
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 07cbb10dc07381df6409f12ca0b4ecb6911ce495
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Fri Sep 9 14:56:08 2022 +1200

    samba-tool visualise: use global --color
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit adf8b8b4a16493d2e0c2f33c00e7a4970b8a9c2a
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit c0d0c13670a1082427c1a62f1fd36c0e0a672a9f
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 7d4387d15dff755a57724d4df5e25b75ae5bee6b
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit baf7c5c585de35a01699a1b0e18bbb339c14afa0
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 5dd4696fb792cff37534eccb943be66cdd9e544c
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 4c623356ce547ea2dd4d9055ef9162f227d4cabd
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 4f30d06a365540aa237976f4807e23b9455e9c90
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 3119349a3f1973697980aff0a012dff92be3402a
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 53f6dbe03f7389242a6ebfaddc90bc39865b17fc
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 9983ea0ed26fb61b205b369796b26e701c546b85
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 1f60e881973ea8faffbd136971c3ae3f3dd233a5
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit eab89c8e29d77922420f345ae0198425ad0ac937
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 7af1326a58ed371209b82f561a6720df2c893849
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at 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



More information about the samba-cvs mailing list