SCons: Fix `silence_msvc` implementation errors

[Repiteo]

Fixes #91883

My eyes have been opened to how futile it is to consolidate logic on this silence_msvc nonsense, so stdout is gonna be more explicitly parsed now. Namely, the decoding now happens immediately & each line is checked individually for potential bloat capture. In the interest of keeping this somewhat restrained, the checks are tailored for this repository where appropriate & warnings are given when this is the case.

[jcbrill]

Potential issues:

1. The check to see if a user is manually rerouting stdout should probably check stderr as well:

               # Process as normal if the user is manually rerouting stdout.
               for arg in args:
   -               if arg.find(">", 0, 1) != -1 or arg.find("1>", 0, 2) != -1:
   +               if arg.find(">", 0, 1) != -1 or arg.find("1>", 0, 2) != -1  or arg.find("2>", 0, 2) != -1:
                       return old_spawn(sh, escape, cmd, args, env)
   

   Otherwise, when a user is attempting to manually redirect stderr one of the following would apply:

   • 2>&1

     The user is redirecting command stderr to stdout which is silently ignored.

     Because the temporary file is appended after the argument list, the windows command line becomes:

     cl ... 2>&1 >tempfilename
     

     Anything written to stderr will not be redirected to the temporary file due to the order of the command arguments.

     The following would be required to redirect to stderr to the temporary file:

     cl ... 2>&1 >tempfilename 2>&1
     

     or

     cl ... >tempfilename 2>&1
     

     Effectively, the user is not getting what was expected.

   • 2>>myuserfile.txt or 2>myuserfile.txt

     The user is redirecting command stderr to a file which works as expected.

     However, the filter code is now writing to stderr after the command completes.

     This likely is not what a user would expect as the post-command writes to stderr are not captured in the user's redirected file.

2. The link filter check is language dependent:

   elif not is_cl and line.startswith("   Creating library"):
   

   If the language is English, the target line looks like:

   link /nologo /OUT:_build001\hello.exe _build001\hello-1.obj _build001\hello-2.obj
      Creating library _build001\hello.lib and object _build001\hello.exp
   

   If the language is German, the target line looks like:

   link /nologo /OUT:_build001\hello.exe _build001\hello-1.obj _build001\hello-2.obj
      Bibliothek "_build001\hello.lib" und Objekt "_build001\hello.exp" werden erstellt.
   

   There are a number of ways to proceed but all are likely infallible in one way or another.

   A simple-ish test would be "starts with 3 spaces and contains '.lib' and contains '.exp'".

   A more involved test involves finding the build artifact name from the link command and searching for the actual lib and exp file names in the argument. A dynamically constructed regex would be useful.

Postscript

I realize that the stream checks are similar to those in the SCons platform win32.py code.

IMHO, using arg.find() != -1 is a bit obtuse for simply checking the start of a string.

Unless I'm missing something, which is always possible, the following are equivalent:

• Using startswith is easier to read:

          for arg in args:
            if arg.startswith(">") or arg.startswith("1>") or arg.startswith("2>"):
                return old_spawn(sh, escape, cmd, args, env)
  

• Using a regular expression is more compact:

  re_redirect_stream = re.compile(r'^[12]?>')
  ...
          for arg in args:
            if re_redirect_stream.match(arg):
                return old_spawn(sh, escape, cmd, args, env)
  

[Repiteo]

Hot damn, you're on a roll with these insights! Went ahead and expanded the logic to incorporate those points & a few other improvements:

• Changed the output check to your regex, so it also checks for stderr as well.
• Link capture check changed to a simple regex that looks for starting 3 spaces & a 4th character that isn't whitespace. This is hopefully specific enough that it won't cause false positives, but even if it does…
• Captured output is now rerouted to msvc_capture.log. That way, even if false positives occur, the values captured will be preserved and readily accessible (provided you don't start another build).
• As it's outside the scope of this PR, but still a valid issue to look into: a FIXME was added above setup_msvc_auto that warns of potential overwrite issues. Didn't want it to get buried after this PR "fixes" that issue.

[jcbrill]

Looks good.

Suggestions:

• The regexes only need to be compiled once: suggest moving outside of spawn_capture function.

Optional enhancements, feel free to ignore:

• A regex ending the line with the known c/c++ suffixes could be used for the compiler. For example, assume a line ending with a c/c++ suffix is the capture line:

  re_cl_filename = re.compile('^.+\.(c|cc|cpp|cxx|c[+]{2})$', re.IGNORECASE)
  

  The regex above has not been tested.

• If the link search includes the .lib and .exp file name sequence, there would be fewer false positives. For example:

  re_link_capture = re.compile(r'\s{3}\S.+\s(?:"[^"]+.lib"|\S+.lib)\s.+\s(?:"[^"]+.exp"|\S+.exp)')
  

  Explanation:

  • 3 spaces
  • 1 non-space
  • 1 or more of characters
  • 1 space
  • either:
    • quoted string of 1 or more characters ending with .lib (e.g., "_build001\hello.lib")
    • 1 or more non-space characters ending with .lib (e.g., _build001\hello.lib)
  • 1 space
  • 1 or more characters
  • 1 space
  • either:
    • quoted string of 1 or more characters ending with .exp (e.g., "_build001\hello.exp")
    • 1 or more non-space characters ending with .exp (e.g., _build001\hello.exp)

  It could probably use some more testing. I was using something similar in my test implementation.

[Repiteo]

Did a few fresh builds & everything's working as expected!

[akien-mga]

Thanks!