Simplify log file handling.

Changes:
* Remove leading and trailing double quotes from SCONS_MSDEBUG_VALUE and issue a warning.
* Catch logging module OSError and FileNotFoundError and wrap in SCons UserError.

1 files changed, 20 insertions, 169 deletions

diff --git a/SCons/Tool/MSCommon/common.py b/SCons/Tool/MSCommon/common.py
index ba43f8a..5ec92a2 100644
--- a/SCons/Tool/MSCommon/common.py
+++ b/SCons/Tool/MSCommon/common.py
@@ -33,183 +33,32 @@ import sys
 from contextlib import suppress
 from subprocess import DEVNULL, PIPE
 from pathlib import Path
-from typing import Optional, Tuple
 
+import SCons.Errors
 import SCons.Util
 import SCons.Warnings
 
 class MSVCCacheInvalidWarning(SCons.Warnings.WarningOnByDefault):
     pass
 
-class WindowsFileNameWarning(SCons.Warnings.WarningOnByDefault):
+class MSCommonLogFileWarning(SCons.Warnings.WarningOnByDefault):
     pass
 
-class _WindowsFileName:
-
-    # Known Issues:
-    # * NTFS filenames with a colon are reported as invalid (false positive)
-    #   given a file named "colonbeg:colonend.txt":
-    #     * file "colonbeg" is created and appears empty
-    #     * hidden alternate data stream file "colonbeg:colonend.txt:$DATA" is created
-    #     * use "dir /R colonbeg" to display alternate data streams of files
-
-    drive_prefix = r"([a-zA-Z][:])[\\]?(?P<suffix>.*)$"
-    re_drive_spec = re.compile(drive_prefix)
-
-    unc_prefix = r"(\\\\[^\\]+)[\\](?P<suffix>.*)$"
-    re_unc_spec = re.compile(unc_prefix)
-
-    re_illegal_chars = re.compile(
-        r'[<>:"/\\|?*\r\t\n]',
-        re.IGNORECASE
-    )
-
-    re_reserved_names = re.compile(
-        r"^(?P<reserved>CON|PRN|AUX|NUL|COM[0-9]|LPT[0-9])(\.|$)",
-        re.IGNORECASE,
-    )
-
-    @classmethod
-    def is_filename_invalid(cls, fileorig: str) -> Tuple[str, str]:
-
-        filename = fileorig
-        if not filename:
-            reason = "file name is empty"
-            return reason, filename
-
-        filename = os.fspath(filename)
-        filename = os.path.abspath(filename)
-
-        pathspec, filespec = os.path.split(filename)
-
-        if filespec and not filespec.strip():
-            reason = f"file name is whitespace only: {filespec!r}"
-            return reason, filename
-
-        suffix = ""
-
-        do_once = True
-        while do_once:
-            do_once = False
-
-            m = cls.re_drive_spec.match(filename)
-            if m:
-                # filename: r"c:\dir\filename.txt"
-                #   prefix: r"c:"
-                #   suffix: r"dir\filename.txt"
-                suffix = m.group("suffix")
-                break
-
-            m = cls.re_unc_spec.match(filename)
-            if m:
-                # filename: r"\\server\share\filename.txt"
-                #   prefix: r"\\server"
-                #   suffix: r"share\filename.txt"
-                suffix = m.group("suffix")
-                break
-
-            reason = "unrecognized path prefix"
-            return reason, filename
-
-        if suffix:
-
-            comps = suffix.split(os.path.sep)
-            for name in comps:
-
-                if not name:
-                    reason = "file name component is empty"
-                    return reason, filename
-
-                if name and not name.strip():
-                    reason = f"file name component is whitespace-only ({name!r})"
-                    return reason, filename
-
-                illegal_chars = cls.re_illegal_chars.findall(name)
-                if illegal_chars:
-                    seen_chars = ', '.join(list(
-                        {repr(s): s for s in illegal_chars}.keys()
-                    ))
-                    reason = f"file name contains illegal characters ({seen_chars})"
-                    return reason, filename
-
-                match = cls.re_reserved_names.match(name)
-                if match:
-                    reserved = match.group("reserved")
-                    reason = f"file name contains a reserved name ({reserved!r})"
-                    return reason, filename
-
-        if not os.path.exists(pathspec):
-            reason = "path to file name does not exist"
-            return reason, filename
-
-        if os.path.exists(filename) and os.path.isdir(filename):
-            reason = "file name is a directory"
-            return reason, filename
-
-        return "", filename
-
-    @classmethod
-    def check_filename_invalid(
-        cls,
-        filename: str,
-        description: Optional[str] = None
-    ) -> bool:
-        reason, abspath = cls.is_filename_invalid(filename)
-        if reason:
-            if description:
-                msg_description = description + " "
-            else:
-                msg_description = ""
-            msg = (
-                f"{msg_description}file name is invalid:\n"
-                f"  filename: {filename!r}\n"
-                f"   abspath: {abspath!r}\n"
-                f"    reason: {reason}"
-            )
-            SCons.Warnings.warn(WindowsFileNameWarning, msg)
-        return bool(reason)
-
-    @classmethod
-    def process_filename(
-        cls,
-        filename: Optional[str],
-        description: Optional[str] = None
-    ) -> Optional[str]:
-        if filename is None:
-            return filename
-        if len(filename) < 2:
-            return filename
-        if filename[0] == '"' and filename[-1] == '"':
-            fileorig = filename
-            filename = filename[1:-1]
-            if description:
-                msg_description = description + " "
-            else:
-                msg_description = ""
-            msg = (
-                f"{msg_description}file name is invalid:\n"
-                f"  filename: {fileorig!r}\n"
-                f"  modified: {filename!r}\n"
-                f"    action: leading and trailing double quotes removed"
+def _check_logfile(logfile):
+    if logfile and len(logfile) >= 2:
+        if logfile[0] == '"' and logfile[-1] == '"':
+            logfile = logfile[1:-1]
+            warn_msg = (
+                "SCONS_MSCOMMON_DEBUG value enclosed in double quotes, doubles quotes removed\n"
+                f'  original value="{logfile}"\n'
+                f"  modified value={logfile}"
             )
-            SCons.Warnings.warn(WindowsFileNameWarning, msg)
-        return filename
-
-    @classmethod
-    def get_filename_environ(
-        cls,
-        evar: str,
-    ) -> Optional[str]:
-        filename = cls.process_filename(os.environ.get(evar), description=evar)
-        return filename
+            SCons.Warnings.warn(MSCommonLogFileWarning, warn_msg)
+    return logfile
 
-check_filename_invalid = _WindowsFileName.check_filename_invalid
-get_filename_environ = _WindowsFileName.get_filename_environ
-
-_LOGFILE_EVAR = "SCONS_MSCOMMON_DEBUG"
 # SCONS_MSCOMMON_DEBUG is internal-use so undocumented:
 # set to '-' to print to console, else set to filename to log to
-LOGFILE = get_filename_environ(_LOGFILE_EVAR)
+LOGFILE = _check_logfile(os.environ.get('SCONS_MSCOMMON_DEBUG'))
 if LOGFILE:
     import logging
 
@@ -295,14 +144,16 @@ if LOGFILE:
         log_prefix = 'debug: '
         log_handler = logging.StreamHandler(sys.stdout)
     else:
-        isinvalid = check_filename_invalid(LOGFILE, description=_LOGFILE_EVAR)
         log_prefix = ''
         try:
             log_handler = logging.FileHandler(filename=LOGFILE)
-        except Exception as e:
-            if isinvalid:
-                raise e.with_traceback(None)
-            raise
+        except (OSError, FileNotFoundError) as e:
+            err_msg = (
+                f"Could not create logfile, check SCONS_MSCOMMON_DEBUG\n"
+                f"  SCONS_MSCOMMON_DEBUG={LOGFILE}\n"
+                f"  {e.__class__.__name__}: {str(e)}"
+            )
+            raise SCons.Errors.UserError(err_msg)
     log_formatter = _CustomFormatter(log_prefix)
     log_handler.setFormatter(log_formatter)
     logger = logging.getLogger(name=__name__)