Rework MSVC environment powershell configuration and description in CHANGES.txt and RELEASE.txt #4720
Conversation
Changes: * Partially restore restore original GH SCons#4717 CHANGES.txt description * Change MSVS to MSVC in RELEASE.txt * Remove extraneous two blank lines introduced in common.py when variable moved
|
Notes for updated text PR. Observations:
The runtimes with powershell 5 are an order of magnitude slower than with powershell 7 in Visual Studio 2022 VCPKG
Example SConstruct times with Powershell 5 (runner results sorted low to high in secs):
Example SConstruct times with Powershell 7 (runner results sorted low to high in secs):
|
|
Windows runner experiments measuring only elapsed time for subprocess invocation of msvc batch files. Notes:
Windows Runner ExperimentsPowershell 5 and 7 results in seconds:
|
Windows VMWare VM ExperimentsPowershell 5 and 7 results in seconds:
VM State Reverted Between Experiments
VM State Not Reverted Between Experiments
|
jcbrill
changed the title
|
Does the CHANGES.txt blurb make sense to anyone else? |
|
Makes sense, yes. I might be proposing a bit of a rewording which would affect tone and tense, but not content - if I get to it, I promise to be careful not to lose any of the message. I'm currently proofreading some Python docu PRs on request and the energy may not extend to this one. We'll see what @bdbaddog thinks. |
|
@mwichmann I starting to wonder if the implementation should be adaptive to the os environment rather than hard-coded. PS 7 is now hard-coded before PS 5 in the msvc environment. The telemetry calls are currently hard-coded to use PS 5 in the batch files which is why PS 5 was added to the msvc environment. The optional vcpkg batch files will use PS 7 or 5 based on which is found first on the os system path. The SCons behavior is only the same as the msvc batch files if PS 7 precedes PS 5 on the os system path. It does in the windows runner but it may not in every end user's system configuration. I'm wondering if the order of PS 7 and 5 in the msvc environment should be based in the order on the user's system path. Similarly, should There are 3 known PS 7 path locations and 2 known PS 5 path locations in the PSModulePath in This is a situation where invoking the msvc batch files in the minimal msvc environment could be different than when running in the os environment. Any thoughts? [Always] @REM Send Telemetry if user's VS is opted-in
if "%VSCMD_SKIP_SENDTELEMETRY%"=="" (
if "%VSCMD_DEBUG%" NEQ "" (
@echo [DEBUG:%~nx0] Sending telemetry
powershell.exe -NoProfile -Command "& {Import-Module '%~dp0\Microsoft.VisualStudio.DevShell.dll'; Send-VsDevShellTelemetry -NewInstanceType Cmd;}"
) else (
START "" /B powershell.exe -NoProfile -Command "& {if($PSVersionTable.PSVersion.Major -ge 3){Import-Module '%~dp0\Microsoft.VisualStudio.DevShell.dll'; Send-VsDevShellTelemetry -NewInstanceType Cmd; }}" > NUL
)
)[If Installed] :: Call powershell which may or may not invoke bootstrap if there's a version mismatch
SET Z_POWERSHELL_EXE=
FOR %%i IN (pwsh.exe powershell.exe) DO (
IF EXIST "%%~$PATH:i" (
SET "Z_POWERSHELL_EXE=%%~$PATH:i"
GOTO :gotpwsh
)
)IMPORTANT: Use of diff format in code fragments intended as easy method of highlighting rather than as actual additions and subtractions.
os.environ[PATH]=
+ C:\Program Files\PowerShell\7;
C:\Program Files\MongoDB\Server\5.0\bin;
C:\aliyun-cli;
C:\vcpkg;
C:\Program Files (x86)\NSIS\;
C:\tools\zstd;
C:\Program Files\Mercurial\;
C:\hostedtoolcache\windows\stack\3.5.1\x64;
C:\cabal\bin;
C:\\ghcup\bin;
C:\mingw64\bin;
C:\Program Files\dotnet;
C:\Program Files\MySQL\MySQL Server 8.0\bin;
C:\Program Files\R\R-4.4.2\bin\x64;
C:\SeleniumWebDrivers\GeckoDriver;
C:\SeleniumWebDrivers\EdgeDriver\;
C:\SeleniumWebDrivers\ChromeDriver;
C:\Program Files (x86)\sbt\bin;
C:\Program Files (x86)\GitHub CLI;
C:\Program Files\Git\bin;
C:\Program Files (x86)\pipx_bin;
C:\npm\prefix;
C:\hostedtoolcache\windows\go\1.24.2\x64\bin;
C:\hostedtoolcache\windows\Python\3.9.13\x64\Scripts;
C:\hostedtoolcache\windows\Python\3.9.13\x64;
C:\hostedtoolcache\windows\Ruby\3.0.7\x64\bin;
C:\Program Files\OpenSSL\bin;
C:\tools\kotlinc\bin;
C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\8.0.452-9\x64\bin;
C:\Program Files\ImageMagick-7.1.1-Q16-HDRI;
C:\Program Files\Microsoft SDKs\Azure\CLI2\wbin;
C:\ProgramData\kind;
C:\ProgramData\Chocolatey\bin;
C:\Windows\system32;
C:\Windows;
C:\Windows\System32\Wbem;
+ C:\Windows\System32\WindowsPowerShell\v1.0\;
C:\Windows\System32\OpenSSH\;
C:\Program Files\dotnet\;
+ C:\Program Files\PowerShell\7\;
C:\Program Files\Microsoft\Web Platform Installer\;
C:\Program Files\TortoiseSVN\bin;
C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\170\Tools\Binn\;
C:\Program Files\Microsoft SQL Server\150\Tools\Binn\;
C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit\;
C:\Program Files (x86)\WiX Toolset v3.14\bin;
C:\Program Files\Microsoft SQL Server\130\DTS\Binn\;
C:\Program Files\Microsoft SQL Server\140\DTS\Binn\;
C:\Program Files\Microsoft SQL Server\150\DTS\Binn\;
C:\Program Files\Microsoft SQL Server\160\DTS\Binn\;
C:\Strawberry\c\bin;
C:\Strawberry\perl\site\bin;
C:\Strawberry\perl\bin;
C:\ProgramData\chocolatey\lib\pulumi\tools\Pulumi\bin;
C:\Program Files\CMake\bin;
C:\ProgramData\chocolatey\lib\maven\apache-maven-3.9.9\bin;
C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;
C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;
C:\Program Files\nodejs\;
C:\Program Files\Git\cmd;
C:\Program Files\Git\mingw64\bin;
C:\Program Files\Git\usr\bin;
C:\Program Files\GitHub CLI\;
c:\tools\php;
C:\Program Files (x86)\sbt\bin;
C:\Program Files\Amazon\AWSCLIV2\;
C:\Program Files\Amazon\SessionManagerPlugin\bin\;
C:\Program Files\Amazon\AWSSAMCLI\bin\;
C:\Program Files\Microsoft SQL Server\130\Tools\Binn\;
C:\Program Files\LLVM\bin;
C:\Users\runneradmin\.dotnet\tools;
C:\Users\runneradmin\.cargo\bin;
C:\Users\runneradmin\AppData\Local\Microsoft\WindowsApps
os.environ[PSMODULEPATH]=
+ C:\Users\runneradmin\Documents\PowerShell\Modules;
+ C:\Program Files\PowerShell\Modules;
+ c:\program files\powershell\7\Modules;
C:\\Modules\az_12.4.0;
- C:\Users\packer\Documents\WindowsPowerShell\Modules;
+ C:\Program Files\WindowsPowerShell\Modules;
+ C:\Windows\system32\WindowsPowerShell\v1.0\Modules;
C:\Program Files\Microsoft SQL Server\130\Tools\PowerShell\Modules\
os.environ[USERPROFILE]=
C:\Users\runneradmin |
|
adaptive: possibly a good thing. the runners are weird overloaded environments "with everything" that probably don't match dev boxes. is the ps version chosen purely by First In PATH, or are there other ways to select? |
In the optional vcpkg.bat call chain, the first In vsdevcmd.bat call chain, Both versions of powershell will use It may not be the worst idea to re-evaluate what is added to the For example, powershell 5 has been in the system path and Powershell 7 is not installed by default. Windows EnvironmentsThe Platform/win32.py environment is crazy minimal for modern Windows: As mentioned previously ad nauseum, the default Platform/win32 and msvc environments lack the common Windows system environment variables: The following system environment variables are known to be referenced in the msvc 2022 batch files:
If one were to compare the paths generated from the command-line versus from SCons, one would find minor differences in the paths configured. To date, either it doesn't matter and/or users adjust the paths themselves after the fact. It is likely that anyone attempting to run It almost appears like the windows environments were written before windows supported 64-bit platforms and really never evolved. Legend:
Default Windows 11, 10: Path=
C:\WINDOWS\system32;
- C:\WINDOWS;
+ C:\WINDOWS\System32\Wbem;
+ C:\WINDOWS\System32\WindowsPowerShell\v1.0\;
- C:\WINDOWS\System32\OpenSSH\;
- C:\Users\%USERNAME%\AppData\Local\Microsoft\WindowsApps;
- PSModulePath=
- C:\Program Files\WindowsPowerShell\Modules;
- C:\WINDOWS\system32\WindowsPowerShell\v1.0\ModulesDefault Windows 8, 7, Vista: Path=
C:\Windows\system32;
- C:\Windows;
+ C:\Windows\System32\Wbem;
+ C:\Windows\System32\WindowsPowerShell\v1.0\
- PSModulePath=
- C:\Windows\system32\WindowsPowerShell\v1.0\Modules\Default Windows XP: Path=
C:\WINDOWS\system32;
- C:\WINDOWS;
+ C:\WINDOWS\System32\Wbem |
|
@mwichmann #4226 appears to avoid all of the above as |
|
Based on what you say, I wonder if using a restricted environment for the setup run is the right choice. After all, we do want to accurately detect what's available. Then, of course, we'd have to figure out how many more things have to be exported to the build-time |
|
Looking at the vcpkg stuff, I guess it's this stanza? :: Call powershell which may or may not invoke bootstrap if there's a version mismatch
SET Z_POWERSHELL_EXE=
FOR %%i IN (pwsh.exe powershell.exe) DO (
IF EXIST "%%~$PATH:i" (
SET "Z_POWERSHELL_EXE=%%~$PATH:i"
GOTO :gotpwsh
)
)
:gotpwsh
"%Z_POWERSHELL_EXE%" -NoProfile -ExecutionPolicy Unrestricted -Command "iex (get-content \"%~dfp0\" -raw)#"Since the temporary variable with the powershell location is unset immediately afterward and not further used. And the expression evaluation seems to just be path fiddling? So how does this manage to go off in the weeds and take a long time? Do I even want to know (well, "want" == No here, but anyway...) |
I suspect if that PR were to be pushed forward, someone (probably me) would have suggested changing it to use |
|
All good questions. Blaise Pascal (Letter 16, 1657):
Apologies for the verbose responses that follow.
That is the dilemma in a nutshell. This is of course anathema to purists that do not want any possibility of tight coupling to an end user's environments. On the other hand, running in an OS-like environment probably removes a considerable maintenance/support burden. For preserved variables, the What a user cannot do right now is set the environment used to invoke the msvc batch files. Using the os.environ would eliminate the need to import variables into the environment and manually set the system path elements. With the VS VCPKG component, we would likely have to While this is less of problem today, keep in mind that the command shell environment could be 32-bit, and/or the python architecture could be 32-bit on a Windows 64-bit host. Running the environment set by running the vcvars batch file would be subtly different than running the same batch files from a 64-bit shell and 64-bit build of python. This effects the system variables. I once used a Windows third-party toolbar to launch a command shell. The tool was 32-bit so the default launched environment was configured for 32-bit applications. I believe that the vcvars batch files should be run in the same environment native to the host. This would require more fiddling and explicitly launching the vcvars batch files via a fully qualified path to the command interpreter due to WoW redirection. On ARM64, there is a subtle difference in vcvars results between running an amd64 build of python versus an ARM64 build of python due to PROCESSOR_ARCHITECTURE. The latest toolsets for VS2022 have native ARM64 builds. While unlikely, on ARM64, python could be 32-bit arm, 32-bit x86, 64-bit amd64, or 64-bit ARM64. I believe there are 3 shell environment (I could be wrong): 32-bit x86, 64-bit amd64, or 64-bit ARM64. It is almost trivial to run the one-time vcvars batch files with an explicit command interpreter if necessary.
Bit of trivia: the Passing env=dict(os.environ) to
My guess is that when calling itself as a powershell script, there is delay is introduced in one or more of the following: determining the default PSModulePath, loading powershell modules, downloading and "installing" information from github. Somewhere in the snippet below (alas some code removed) lies the root cause of the delay. Rough call sequence:
|
|
Forgot to add: an SCons |
Not sure I quite get this comment. If you open a "developer command prompt" (cmd or ps variant) you get a shell with a boatload of environment variables set. Of course that's different to what we run the build commands with, given the scons policy we all know and love. That doesn't seem likely to have been your point? |
I did a poor job trying to provide a reminder that in the current code, the environment passed to the subprocess invocation of the msvc batch files ignores everything in the user's MSCommon/common.py MSCommon/vc.py The get_output invocation from MSCommon/vc.py is not passed an env argument. Inside get_output, a new environment is materialized if the env argument is None. It does not matter what is in the user's env['ENV']. It won't be used at all. In this respect, the msvc tool initialization may be very different than other tools. That is why suggestions like the following aren't going to work: The environment in which the msvc batch files is invoked won't contain the |
RELEASE.txt
Outdated
| @@ -35,7 +35,7 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY | |||
| one from PEP 308 introduced in Python 2.5 (2006). The idiom being | |||
| replaced (using and/or) is regarded as error prone. | |||
|
|
|||
| - MSVS: The default Windows powershell 7 path is added before the default | |||
| - MSVC: The default Windows powershell 7 path is added before the default | |||
There was a problem hiding this comment.
Needs a blurb about the user impact. Perhaps something like.
"In some environments (such as Github Actions) this can yield a significant reduction in the time to initialize MSVC. (Without this change you may see > 10x slowdown) "
replace ##x slowdown with your best approximation of your test results.
The idea is that anyone reading this looking for a version with a fix should know from this blurb that it is indeed fixed.
RELEASE.txt is what gets sent to the mailing lists and other notification channels. Users then would have to go to the CHANGES.txt.
So RELEASE.txt should have more or less the same content as this releases CHANGE.txt blurb, but organized differently, without the attribution.
|
Prototype "adaptive" powershell results. Changes:
Results from two environments are shown below:
I think this is way it should be done. The only hesitation is in including the Current User powershell paths. In this limited use context, I don't think it matters and would be similar to running in the OS environment. Note: Not all of the PSModulePath elements are passed to the msvc environment. Only the elements that comprise the powershell "default paths" if PSModulePath is undefined. This does "fix" the runtime issue in the windows runner. Prototype ResultsDiff code language used for highlighting purposes. Windows runner (PS 7 & PS 5): os.environ[PATH]=
+ C:\Program Files\PowerShell\7;
C:\Program Files\MongoDB\Server\5.0\bin;
C:\aliyun-cli;
C:\vcpkg;
C:\Program Files (x86)\NSIS\;
C:\tools\zstd;
C:\Program Files\Mercurial\;
C:\hostedtoolcache\windows\stack\3.5.1\x64;
C:\cabal\bin;
C:\\ghcup\bin;
C:\mingw64\bin;
C:\Program Files\dotnet;
C:\Program Files\MySQL\MySQL Server 8.0\bin;
C:\Program Files\R\R-4.4.2\bin\x64;
C:\SeleniumWebDrivers\GeckoDriver;
C:\SeleniumWebDrivers\EdgeDriver\;
C:\SeleniumWebDrivers\ChromeDriver;
C:\Program Files (x86)\sbt\bin;
C:\Program Files (x86)\GitHub CLI;
C:\Program Files\Git\bin;
C:\Program Files (x86)\pipx_bin;
C:\npm\prefix;
C:\hostedtoolcache\windows\go\1.24.3\x64\bin;
C:\hostedtoolcache\windows\Python\3.9.13\x64\Scripts;
C:\hostedtoolcache\windows\Python\3.9.13\x64;
C:\hostedtoolcache\windows\Ruby\3.0.7\x64\bin;
C:\Program Files\OpenSSL\bin;
C:\tools\kotlinc\bin;
C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\8.0.452-9\x64\bin;
C:\Program Files\ImageMagick-7.1.1-Q16-HDRI;
C:\Program Files\Microsoft SDKs\Azure\CLI2\wbin;
C:\ProgramData\kind;
C:\ProgramData\Chocolatey\bin;
C:\Windows\system32;
C:\Windows;
C:\Windows\System32\Wbem;
+ C:\Windows\System32\WindowsPowerShell\v1.0\;
C:\Windows\System32\OpenSSH\;
C:\Program Files\dotnet\;
+ C:\Program Files\PowerShell\7\;
C:\Program Files\Microsoft\Web Platform Installer\;
C:\Program Files\TortoiseSVN\bin;
C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\170\Tools\Binn\;
C:\Program Files\Microsoft SQL Server\150\Tools\Binn\;
C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit\;
C:\Program Files (x86)\WiX Toolset v3.14\bin;
C:\Program Files\Microsoft SQL Server\130\DTS\Binn\;
C:\Program Files\Microsoft SQL Server\140\DTS\Binn\;
C:\Program Files\Microsoft SQL Server\150\DTS\Binn\;
C:\Program Files\Microsoft SQL Server\160\DTS\Binn\;
C:\Strawberry\c\bin;
C:\Strawberry\perl\site\bin;
C:\Strawberry\perl\bin;
C:\ProgramData\chocolatey\lib\pulumi\tools\Pulumi\bin;
C:\Program Files\CMake\bin;
C:\ProgramData\chocolatey\lib\maven\apache-maven-3.9.9\bin;
C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;
C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;
C:\Program Files\nodejs\;
C:\Program Files\Git\cmd;
C:\Program Files\Git\mingw64\bin;
C:\Program Files\Git\usr\bin;
C:\Program Files\GitHub CLI\;
c:\tools\php;
C:\Program Files (x86)\sbt\bin;
C:\Program Files\Amazon\AWSCLIV2\;
C:\Program Files\Amazon\SessionManagerPlugin\bin\;
C:\Program Files\Amazon\AWSSAMCLI\bin\;
C:\Program Files\Microsoft SQL Server\130\Tools\Binn\;
C:\Program Files\LLVM\bin;
C:\Users\%USERNAME%\.dotnet\tools;
C:\Users\%USERNAME%\.cargo\bin;
C:\Users\%USERNAME%\AppData\Local\Microsoft\WindowsApps
os.environ[PSMODULEPATH]=
+ C:\Users\%USERNAME%\Documents\PowerShell\Modules;
+ C:\Program Files\PowerShell\Modules;
+ c:\program files\powershell\7\Modules;
C:\\Modules\az_12.4.0;
- C:\Users\packer\Documents\WindowsPowerShell\Modules;
+ C:\Program Files\WindowsPowerShell\Modules;
+ C:\Windows\system32\WindowsPowerShell\v1.0\Modules;
C:\Program Files\Microsoft SQL Server\130\Tools\PowerShell\Modules\
env[PATH]=
C:\Windows\System32;
C:\Windows\System32\Wbem;
+ C:\Program Files\PowerShell\7;
C:\Windows\System32\WindowsPowerShell\v1.0\
+ env[PSModulePath]=
+ C:\Users\%USERNAME%\Documents\PowerShell\Modules;
+ C:\Program Files\PowerShell\Modules;
+ C:\Program Files\PowerShell\7\Modules;
+ C:\Program Files\WindowsPowerShell\Modules;
+ C:\Windows\System32\WindowsPowerShell\v1.0\ModulesLocal machine (PS 5): os.environ[PATH]=
C:\Data\venv\scons-dev\3.11.9\Scripts;
C:\Program Files (x86)\Common Files\Intel\Shared Libraries\redist\intel64_win\compiler;
C:\Program Files (x86)\Common Files\Intel\Shared Libraries\redist\ia32_win\compiler;
C:\Windows\system32;
C:\Windows;
C:\Windows\System32\Wbem;
+ C:\Windows\System32\WindowsPowerShell\v1.0\;
C:\Windows\System32\OpenSSH\;
C:\Program Files\dotnet\;
C:\Software\PuTTY\;
C:\Program Files\Microsoft SQL Server\110\Tools\Binn\;
C:\Program Files\Microsoft SQL Server\120\Tools\Binn\;
C:\Program Files (x86)\Microsoft SDKs\TypeScript\1.0\;
C:\Users\%USERNAME%\.dnx\bin;
C:\Program Files\Microsoft DNX\Dnvm\;
C:\Program Files\Microsoft SQL Server\130\Tools\Binn\;
C:\Program Files\Microsoft\Web Platform Installer\;
C:\Software\TortoiseHg\;
C:\Software\Git\cmd;
C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit\;
C:\Software\TortoiseGit\bin;
C:\Users\%USERNAME%\.pyenv\pyenv-win\bin;
C:\Users\%USERNAME%\AppData\Local\Microsoft\WindowsApps;
C:\Users\%USERNAME%\.dotnet\tools;
C:\Users\%USERNAME%\AppData\Local\GitHubDesktop\bin;
C:\Users\%USERNAME%\AppData\Roaming\npm;
C:\Users\%USERNAME%\AppData\Local\JpegminiPro4\;
C:\Users\%USERNAME%\AppData\Local\Markdown Monster
os.environ[PSMODULEPATH]=
+ C:\Program Files\WindowsPowerShell\Modules;
+ C:\Windows\system32\WindowsPowerShell\v1.0\Modules
env[PATH]=
C:\Windows\System32;
C:\Windows\System32\Wbem;
C:\Windows\System32\WindowsPowerShell\v1.0\
+ env[PSModulePath]=
+ C:\Program Files\WindowsPowerShell\Modules;
+ C:\Windows\System32\WindowsPowerShell\v1.0\Modules |
|
Adaptive powershell path elements and PSModulePath handling in MSCommon/common.py branch of this PR: If desired, this PR could be updated with the changes from the branch. Commit message:
Additional known "challenges" with the existing MSCommon/common and MSCommon/vc code should probably be documented. Comments and questions welcome. PS: I am time challenged for the next 7 days. |
|
The vcpkg search for the powershell executable will use pwsh.exe (PS 7) if it is anywhere on the path; otherwise, it will use powershell.exe (PS 5) if it is on the path. The path order does not matter: PS 7 will be used even if the PS 5 path precedes the PS 7 path. Sigh. |
|
That's what I see in the code snip above - it's a loop which exits on first match. |
|
Yep. The logic in the commit above is still the way to go. I have updated and committed MSCommon/common.py in the other branch with a revised comment: jcbrill@43270cb |
Corrected answer. vcvars Telemetry:
VS vcpkg powershell executable priority (path order does not matter):
|
|
So given how I now understand this information, here are two possible rewordings of the changelog snip - one verbose (I have this tendency too :-) ), and one very concise in a just-the-facts-without-details form. Just tossing these out there, feel free to ignore these, or meld them into a new version, or whatever.
|
I'd expect we want to be sure to get the one that's already been initialized, or we don't benefit from the cache warming effect. That may well turn out to be the default for that PS version - but if it was, then why do we see the problem with the variable unset (I have only PS 5 installed, and my cache is in |
For the msvc batch file invocations in the windows 2022 and 2025 runners: yes. I don't know if there are any additional uninstalled extra VS components that rely on powershell that could have materialized similar behavior. The powershell.exe invocation to setup telemetry in the windows 2019/2022/2035 runners does not manifest a significant slowdown.
From earlier comments:
In the windows 2019, 2022, and 2025 windows runners, The reason for importing It may not make any sense to define our own location in the environment as powershell will use its own default location when The benefit for the windows runners is that the powershell analysis module cache appears to be populated before the msvc batch files are invoked. Even if the cache were not pre-prepopulated, there may be a good reason why a user would like the cache to be populated in a certain location. For example, in the windows runners, if the cache were moved to a different drive (e.g., D:) there might be a runtime benefit. Perhaps a user is manually saving and restoring the powershell module analysis cache from their own known location. The powershell environment configuration could be
That was the source, and contains the rationale, for adding Some amount of caution needs to be exercised in assuming all environments will behave like the windows 2019/2022/2025 runners. For the limited msvc environment, importing At present, Future work might consider what was described in the alternate branch mentioned above:
P.S.: A similar argument could be made for importing |
|
@mwichmann I was in the process of writing the above and committing before seeing your post. Please check the following for accuracy.
This.
There is a cache persistence difference between running on a physical pc or a virtual machine environment compared to the windows runners. On a physical pc or a virtual machine, the default cache once constructed (if at all) is persistent. To emulate the windows runner behavior in a virtual machine, one would have to "reset the vm state" after each run. The default locations of the module analysis cache when
For the following, the size of the pre-populated cache in the Windows 2022 runner is 1,610,258 bytes (~1.6MB) as reported by python's
The only way to guarantee the same behavior as the shell environment is to: 1) add pwsh.exe to the msvc environment path, 2) include The prior PR does 1. This PR does 1 and 2. A limited form of 3 was discussed in my previous post under "future work".
It is confusing.
That would be my guess. PS 7 (Core) does not appear to add the hex suffix to the end of the |
|
@mwichmann Research implementation windows runner logs showing the complete OS environment and limited msvc environment, and the powershell module analysis cache file sizes (0 if non-existent). Let me know if you have any questions. These logs are representative of the windows runner findings to date. The logs also provide a decent view of the windows runner configuration. The table below contains modified versions of the windows 2022 logs for four experiments with the pure python implementation. The windows runner timestamp prefix was removed for readability. This is using the research implementation and not the SCons implementation. Log records legend:
PS 5.x and No Cache: SCons 4.9.1 and earlier. PS 7.x and No Cache: Previous PR (i.e., current SCons master). PS 7.x and Cache: This PR. PS 5.x and Cache: This PR (implied if pwsh.exe not on path). |
|
I think that Adding a shell environment variable to the limited msvc environment does not mean it will be preserved in the caller's SCons environment. Other than Effectively, powershell running in the limited msvc environment would be roughly equivalent to running powershell in the shell environment subject to the minimal This same argument applies for adding known Windows system variables (e.g., The diagram below shows the shell environment members of the limited environment in which the msvc batch files are invoked and the environment variables that are preserved in the caller's SCons environment. flowchart TD;
A[
**Construct Limited MSVC Environment**<br>
*==SCons/Platform/win32.py==*<br/>
PATH [minimal]<sup>1</sup>
PATHEXT [minimal]<sup>2</sup>
SystemDrive
SystemRoot
TEMP
TMP
USERPROFILE<br/>
*==SCons/Tool/MSCommon/Common.py==*<br/>
PATH [minimal extended]<sup>3</sup>
COMSPEC
OS
VS170COMNTOOLS
VS160COMNTOOLS
VS150COMNTOOLS
VS140COMNTOOLS
VS120COMNTOOLS
VS110COMNTOOLS
VS100COMNTOOLS
VS90COMNTOOLS
VS80COMNTOOLS
VS71COMNTOOLS
VSCOMNTOOLS
MSDevDir
VSCMD_DEBUG
VSCMD_SKIP_SENDTELEMETRY
windir
VCPKG_DISABLE_METRICS
VCPKG_ROOT<br>
*==Proposed SCons PR 4720==*<br/>
POWERSHELL_TELEMETRY_OPTOUT
PSDisableModuleAnalysisCacheCleanup
PSModuleAnalysisCachePath
PSModulePath [subset]<sup>4</sup>
]
B[
**Call MSVC Batch File**<br/>
vcvars64.bat & set
]
C[
**Extend Caller SCons env["ENV"]**<br>
*==SCons/Tool/MSCommon/Common.py==*<br/>
INCLUDE
LIB
LIBPATH
PATH
VSCMD_ARG_app_plat
VCINSTALLDIR
VCToolsInstallDir
]
A-->B;
B-->C;
Diagram footnotes:
Notes:
Edit [2025-05-29] to reflect PR code changes:
|
…PATH and add limited known shell PSModulePath paths. Changes: * Add the pwsh and powershell paths to the msvc environment PATH in the order discovered on the shell environment PATH. * Add the pwsh and powershell AllUsers and installation PSModule paths to the msvc environment PSModulePath in the order discovered on the shell environment PSModulePath.
# Manually resolved conflicts: # RELEASE.txt
jcbrill
changed the title
|
Changes in a nutshell:
The diagram and footnotes in the comment above, #4720 (comment), was updated to reflect these changes. Open to discussion concerning PSModulePath implementation. No further changes planned. |
|
@jcbrill thanks for all your (as always) thorough work on this. I'll make some time this weekend to properly review. |
Changes: * Prior implementation tested for path membership in a string. This would not add a path that was a proper prefix of an existing path member. Use a list of path elements to test for path membership. * Prior implementation tested for case sensitive path membership. Windows paths are not case sensitive. Use normalized case of absolute path in tests for membership.
|
I pushed an additional commit that fixed two known issues with the msvc environment system path construction. There is nothing particularly clever about the changes in this PR implementation. It appears longish mostly due to some utility functions to test for path membership of executables/directories coupled with internal caches to avoid doing the same work more than once. The prior implementation employed a test sequence like the following: The issues with this method are:
The subset of I can confirm that any one of the three changes for powershell in this PR reduces the significant delay in the windows 2022 and 2025 runners:
I think it is desirable to include all three to guard against performance degradation in unknown runtime environments that may exist in the wild. No more work is planned. The boy who cried wolf really means it this time. |
|
@mwichmann Off-topic Intel oneAPI on Windows
I setup a VMWare VM with VS2022 and Intel oneAPI C++ Essentials and poked arround a little to see how the environment is configued. If interested, I'm willing to share some of the findings. Probably best to move discussion somewhere else though. |
|
Sure... I've done this once before in a Windows VM, but don't think I have the results any longer, so you'd be saving time. Go ahead and create a discussion |
CHANGES.txt
Outdated
| values are not propagated to the SCons environment after running | ||
| the MSVC batch files. | ||
| - MSVC: A significant delay was experienced in the windows 2022 | ||
| and 2025 runners due to the limited environment in which the MSVC |
There was a problem hiding this comment.
can you add "GitHub Actions runners', instead of runners?
CHANGES.txt
Outdated
| the MSVC batch files. | ||
| - MSVC: A significant delay was experienced in the windows 2022 | ||
| and 2025 runners due to the limited environment in which the MSVC | ||
| batch files are invoked. The vcpkg component installed in Visual |
There was a problem hiding this comment.
"If the vcpkg component is installed with Visual Studio installler"... ?
CHANGES.txt
Outdated
| and 2025 runners due to the limited environment in which the MSVC | ||
| batch files are invoked. The vcpkg component installed in Visual | ||
| Studio invokes a powershell script when the MSVC batch files are | ||
| called. The limited MSVC environment 1) did not have the preferred |
There was a problem hiding this comment.
".. environment used by SCons to initialized MSVC"
CHANGES.txt
Outdated
| order discovered on the shell environment path, passing the powershell | ||
| module analysis cache location, and adding a limited subset of the | ||
| powershell module path appears to have eliminated the significant | ||
| delays. In the windows runners, any one of the these three changes |
There was a problem hiding this comment.
".. Github Actions windows runners.."
CHANGES.txt
Outdated
| combination of all three changes will guard against significant | ||
| delays in other environment configurations as well. | ||
| - MSVC: The following shell variables and values are passed to the | ||
| environment in which the MSVC batch files are invoked: |
There was a problem hiding this comment.
"environment used by SCons to initialize MSVC"..
RELEASE.txt
Outdated
| been installed on the system. | ||
|
|
||
| - MSVC: Fixed a significant delay that was experienced in the windows | ||
| 2022 and 2025 runners due to the limited environment in which the MSVC |
There was a problem hiding this comment.
"Github Actions windows..."
|
@jcbrill thanks for all your (as always) thorough work on this. I'll make some time this weekend to properly review. |
|
One question: if the user's Environment's |
Yes they will. That is my understanding of the code fragments that follow. A new blank environment is constructed which effectively has only the The relevant fragment from The only call from The As noted internally, |
# Manually resolve conflicts: # RELEASE.txt
|
@jcbrill - thanks again for the good work! Merging. |
Partially restore restore original GH #4717 CHANGES.txt text as a placeholder for reworked description.
Contributor Checklist:
CHANGES.txtandRELEASE.txt(and read theREADME.rst).