[vcpkg, jsonnet, openssl-uwp] Enable use of the system powershell-core if it is present. #13805

BillyONeal · 2020-09-29T04:09:30Z

In #13267 we attempted to resolve failures to download PowerShell making the pool be relatively unstable by updating the version we look for to 7.0.3, and we have that version installed in the VMs. Unfortunately, this has not resulted in the reliability improvement we expected because vcpkg ignores the system copy. For example:

https://dev.azure.com/vcpkg/public/_build/results?buildId=43418&view=logs&j=5dffab7c-3299-5a76-7b02-e037cd868fad&t=70564965-7ae4-5af3-8764-a93ab8ddf245

[...]
A suitable version of powershell-core was not found (required v7.0.3). Downloading portable powershell-core v7.0.3...
Downloading powershell-core...
  https://github.com/PowerShell/PowerShell/releases/download/v7.0.3/PowerShell-7.0.3-win-x86.zip -> D:\downloads\PowerShell-7.0.3-win-x86.zip
Extracting powershell-core...
[...]

This change enables detection of a system pwsh.

Also note that callers in the scripts tree were changed to explicitly look for "pwsh" rather than "powershell". Previously we copied the binary under the old name which worked as a fallback, but this fails when pwsh is in Program Files:

D:\vcpkg\toolsrc\src\vcpkg-test\commands.build.cpp(16): FAILED:
due to unexpected exception with message:
  copy: Access is denied.: "C:\Program Files\PowerShell\7\pwsh.exe", "C:\
  Program Files\PowerShell\7\powershell.exe"

Also fixed a few ports to explicitly name pwsh for more consistent behavior, and drive by attempted to fix #12237 since the problem repro'd in the validation build for x86-windows 682fa1f.

ras0219-msft

There is a remaining case of calling powershell.exe via vcpkg_build_msbuild(USE_VCPKG_INTEGRATION) -- this will pass flags to msbuild which pull in our standard applocal deployment stuff [1].

I recall when we spoke about this you mentioned that you tested some MSBuild cases and they didn't trigger the powershell bug, but I'm not sure if this specific one was considered.

The fix for this case is probably to add an internal VcpkgPowershell property to our MSBuild scripts that can be redirected to pwsh inside vcpkg but defaults to powershell outside.

[1]

vcpkg/scripts/cmake/vcpkg_build_msbuild.cmake

Lines 109 to 116 in c902748

    
               if(_csc_USE_VCPKG_INTEGRATION) 
        
                   list( 
        
                       APPEND _csc_OPTIONS 
        
                       /p:ForceImportBeforeCppTargets=${SCRIPTS}/buildsystems/msbuild/vcpkg.targets 
        
                       "/p:VcpkgTriplet=${TARGET_TRIPLET}" 
        
                       "/p:VcpkgCurrentInstalledDir=${CURRENT_INSTALLED_DIR}" 
        
                   ) 
        
               endif()

ras0219-msft · 2020-10-05T18:20:20Z

scripts/cmake/vcpkg_copy_tool_dependencies.cmake

    endif()
    macro(search_for_dependencies PATH_TO_SEARCH)
        file(GLOB TOOLS ${TOOL_DIR}/*.exe ${TOOL_DIR}/*.dll)
        foreach(TOOL ${TOOLS})
            vcpkg_execute_required_process(
-                COMMAND ${PS_EXE} -noprofile -executionpolicy Bypass -nologo
-                    -file ${SCRIPTS}/buildsystems/msbuild/applocal.ps1
+                COMMAND ${PWSH_EXE} -noprofile -executionpolicy Bypass -nologo


Suggested change

COMMAND ${PWSH_EXE} -noprofile -executionpolicy Bypass -nologo

COMMAND "${PWSH_EXE}" -noprofile -executionpolicy Bypass -nologo

ras0219-msft · 2020-10-05T18:27:16Z

scripts/buildsystems/vcpkg.cmake

+    if (NOT DEFINED _VCPKG_POWERSHELL_PATH)
+        find_program(_VCPKG_PWSH_PATH pwsh)
+        if (_VCPKG_PWSH_PATH-NOTFOUND)
+            message(WARNING "vcpkg: Could not find PowerShell Core; falling back to PowerShell")


Since most users will not have PowerShell Core installed, this adds a warning for most/all users. I think we should probably not warn here.

BillyONeal · 2020-10-07T08:58:16Z

There is a remaining case of calling powershell.exe via vcpkg_build_msbuild(USE_VCPKG_INTEGRATION) -- this will pass flags to msbuild which pull in our standard applocal deployment stuff [1].

Yes, this is the test I meant I tried. Consider activemq-cpp which uses that feature but I'm still left with a truetype font afterwards:

so I think we're good with no changes here.

BillyONeal · 2020-10-07T09:07:08Z

The fix for this case is probably to add an internal VcpkgPowershell property to our MSBuild scripts that can be redirected to pwsh inside vcpkg but defaults to powershell outside.

That said I think this is an improvement even if it's unnecessary so I did that anyway.

BillyONeal · 2020-10-07T09:20:25Z

Oh I see, we don't trigger the problem because in those places we set /p:VcpkgApplocalDeps=false so we never try to invoke the powershell. So I'm going to leave that alone.

ras0219 · 2020-10-09T06:43:15Z

ports/vs-yasm/portfile.cmake

 set(_files yasm.props yasm.targets yasm.xml)
 foreach(_file ${_files})
    file(INSTALL "${SOURCE_PATH}/${_file}" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}/")
 endforeach()
 set(_file "${CURRENT_PACKAGES_DIR}/share/${PORT}/yasm.props")
 file(READ "${_file}" _contents)
-string(REPLACE "$(VCInstallDir)" "" _contents "${_contents}")
+string(REPLACE "$(VCInstallDir)" "${YASM_NATIVE}" _contents "${_contents}")


This inserts absolute paths into the package which will become invalid if it is restored elsewhere.

JackBoosY · 2020-10-09T06:45:58Z

At the beginning, the use of powershell in the user system was prohibited because some functions in our script lacked compatibility in different versions. So this PR should focus on this issue.

BillyONeal · 2020-10-09T06:51:20Z

At the beginning, the use of powershell in the user system was prohibited because some functions in our script lacked compatibility in different versions. So this PR should focus on this issue.

That's why this checks for the version and only uses the system one if the version matches

…sh.exe comes from Program Files.

BillyONeal · 2020-10-28T03:48:07Z

The failures are all the same as the most recent CI build, so merging this anyway.

…e if it is present. (microsoft#13805)

PhoebeHui assigned PhoebeHui and JackBoosY Sep 29, 2020

JackBoosY added category:infrastructure Pertaining to the CI/Testing infrastrucutre category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:internal This PR or Issue was filed by the vcpkg team. labels Sep 29, 2020

ras0219-msft approved these changes Oct 5, 2020

View reviewed changes

BillyONeal requested a review from ras0219-msft October 7, 2020 19:58

BillyONeal added the category:port-bug The issue is with a library, which is something the port should already support label Oct 8, 2020

BillyONeal changed the title ~~[vcpkg] Enable use of the system powershell-core if it is present.~~ [vcpkg, vs-yasm, jsonnet, openssl-uwp] Enable use of the system powershell-core if it is present. Oct 8, 2020

ras0219 suggested changes Oct 9, 2020

View reviewed changes

BillyONeal added 9 commits October 26, 2020 19:50

Fix lack of detection of installed powershell-core 7.

f897243

Remove attempt to copy pwsh.exe as powershell.exe which fails when pw…

2e04c28

…sh.exe comes from Program Files.

Apply Robert CR feedback.

fe9fcde

Fix build failure on *nix.

e5cc78e

Try to use pwsh.exe when using plain msbuild integration.

d72b80d

Add docstring to make cmake happy.

53af2ef

[jsonnet, openssl-uwp] Use pwsh

2c5294d

Fix unit test failures on non-Win32.

f3495ac

clang-format

c784469

BillyONeal force-pushed the powershell_version branch from 2a946f3 to c784469 Compare October 27, 2020 02:52

BillyONeal changed the title ~~[vcpkg, vs-yasm, jsonnet, openssl-uwp] Enable use of the system powershell-core if it is present.~~ [vcpkg, jsonnet, openssl-uwp] Enable use of the system powershell-core if it is present. Oct 28, 2020

BillyONeal merged commit a6a1722 into microsoft:master Oct 28, 2020

BillyONeal deleted the powershell_version branch October 28, 2020 03:48

JackBoosY mentioned this pull request Oct 28, 2020

Vcpkg does not find system installed Powershell Core #9606

Closed

strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021

[vcpkg, jsonnet, openssl-uwp] Enable use of the system powershell-cor…

dab0b54

…e if it is present. (microsoft#13805)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vcpkg, jsonnet, openssl-uwp] Enable use of the system powershell-core if it is present. #13805

[vcpkg, jsonnet, openssl-uwp] Enable use of the system powershell-core if it is present. #13805

BillyONeal commented Sep 29, 2020 •

edited

Loading

ras0219-msft left a comment

ras0219-msft Oct 5, 2020

ras0219-msft Oct 5, 2020

BillyONeal commented Oct 7, 2020

BillyONeal commented Oct 7, 2020

BillyONeal commented Oct 7, 2020

ras0219 Oct 9, 2020

JackBoosY commented Oct 9, 2020

BillyONeal commented Oct 9, 2020

BillyONeal commented Oct 28, 2020

	if(_csc_USE_VCPKG_INTEGRATION)
	list(
	APPEND _csc_OPTIONS
	/p:ForceImportBeforeCppTargets=${SCRIPTS}/buildsystems/msbuild/vcpkg.targets
	"/p:VcpkgTriplet=${TARGET_TRIPLET}"
	"/p:VcpkgCurrentInstalledDir=${CURRENT_INSTALLED_DIR}"
	)
	endif()

	COMMAND ${PWSH_EXE} -noprofile -executionpolicy Bypass -nologo
	COMMAND "${PWSH_EXE}" -noprofile -executionpolicy Bypass -nologo

[vcpkg, jsonnet, openssl-uwp] Enable use of the system powershell-core if it is present. #13805

[vcpkg, jsonnet, openssl-uwp] Enable use of the system powershell-core if it is present. #13805

Conversation

BillyONeal commented Sep 29, 2020 • edited Loading

ras0219-msft left a comment

Choose a reason for hiding this comment

ras0219-msft Oct 5, 2020

Choose a reason for hiding this comment

ras0219-msft Oct 5, 2020

Choose a reason for hiding this comment

BillyONeal commented Oct 7, 2020

BillyONeal commented Oct 7, 2020

BillyONeal commented Oct 7, 2020

ras0219 Oct 9, 2020

Choose a reason for hiding this comment

JackBoosY commented Oct 9, 2020

BillyONeal commented Oct 9, 2020

BillyONeal commented Oct 28, 2020

BillyONeal commented Sep 29, 2020 •

edited

Loading