Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] Environment loading is not compatible with powershell 7 #17339

Closed
Todiq opened this issue Nov 19, 2024 · 4 comments
Closed

[bug] Environment loading is not compatible with powershell 7 #17339

Todiq opened this issue Nov 19, 2024 · 4 comments

Comments

@Todiq
Copy link
Contributor

Todiq commented Nov 19, 2024

Describe the bug

Hello,

Whenever trying to run a conan build on a windows container with powershell that links to pwsh, I am getting this error:

conanvcvars.bat: Activating environment Visual Studio 17 - amd64 - winsdk_version=None - vcvars_ver=14.4
[vcvarsall.bat] Environment initialized for: 'x64'
The argument '&'C:\workspace\metamodel\build\windows-msvc-194-x86_64\generators\conanbuild.ps1'' is not recognized as the name of a script file. Check the spelling of the name, or if a path was included, verify that the path is correct and try again.

Usage: pwsh[.exe]
...

To understand a bit the situation, I reran the command with -vvv, which gave me:

conanfile.py (castmetamodel/0.0.1): Full command: "C:\workspace\metamodel\build\windows-msvc-194-x86_64\generators\conanbuild.bat" && powershell.exe "&'C:\workspace\metamodel\build\windows-msvc-194-x86_64\generators\conanbuild.ps1'" ; cmd /c cmake -G 'Ninja Multi-Config' -DCMAKE_TOOLCHAIN_FILE='generators/conan_toolchain.cmake' -DCMAKE_INSTALL_PREFIX='C:/workspace/metamodel' -DCMAKE_POLICY_DEFAULT_CMP0091='NEW' 'C:/workspace/metamodel'
conanvcvars.bat: Activating environment Visual Studio 17 - amd64 - winsdk_version=None - vcvars_ver=14.4
[vcvarsall.bat] Environment initialized for: 'x64'
The argument '&'C:\workspace\metamodel\build\windows-msvc-194-x86_64\generators\conanbuild.ps1'' is not recognized as the name of a script file. Check the spelling of the name, or if a path was included, verify that the path is correct and try again.

Usage: pwsh[.exe]
...

Which made me question myself regarding the following call:
powershell.exe "&'C:\workspace\metamodel\build\windows-msvc-194-x86_64\generators\conanbuild.ps1'"

What is the rationale behind it? Couldn't one simply call powershell.exe "C:\workspace\metamodel\build\windows-msvc-194-x86_64\generators\conanbuild.ps1"? This way, the command would be compatible with both pwsh and powershell 5.

Otherwise, could we consider adding a new conf in order to support powershell > 5?

Thanks in advance for your time.

How to reproduce it

No response

@czoido czoido self-assigned this Nov 19, 2024
@czoido
Copy link
Contributor

czoido commented Nov 19, 2024

Hi @Todiq,
Thanks for reporting this!
We’ll check if we can adjust the command to work with both PowerShell versions or, if needed, expand the config to allow version selection.
Marking this to be reviewed in the next release. We'll keep you updated.

@czoido czoido added this to the 2.10.0 milestone Nov 19, 2024
@jcar87
Copy link
Contributor

jcar87 commented Nov 19, 2024

Thanks @Todiq -

windows container with powershell that links to pwsh

Is this common practice? The Microsoft documentation: https://learn.microsoft.com/en-us/powershell/scripting/whats-new/migrating-from-windows-powershell-51-to-powershell-7?view=powershell-7.4 and other references I can find on the matter, do mention that powershell.exe is intentionally different to pwsh.exe precisely to guarantee that powershell.exe (the one that Conan is invoking) is Windows Powershell 5.1.

Just checking if there is more insights as to to why powershell.exe would link to pwsh.exe given that they are in fact incompatible and have a different CLI

https://learn.microsoft.com/en-us/powershell/scripting/whats-new/differences-from-windows-powershell?view=powershell-7.4#powershell-executable-changes

What is the rationale behind it?

I believe that's how it is documented to be able to invoke a script in a quote-surrounded file path, which might otherwise result in the string being printed.

Possibly we can pass -Command to be compatible with both - however I'm also not entirely sure any logic inside has ever been tested to work in any newer version of powershell.

@Todiq
Copy link
Contributor Author

Todiq commented Nov 19, 2024

Hello @jcar87, @czoido

Thanks for your answers. It is in fact not a common practice, you are right. Here is the complete explanation:

My company has limited CPU resources. While we use GitLab cloud, we only have 2 on-premise GitLab runners running many jobs. One is Linux while the other one is Windows. Both are docker-based runners, meaning that they would start each job in its own docker container (linux or windows based).

GitLab runners are defined by a config.toml file.

On our Windows runner, we chose the docker-windows executor.

Many jobs would use the windows server core image in order to execute commands in a Windows environment. This image only contains powershell 5.

Although written otherwise, the shell does also affect the docker-windows executor. That's why we had to select powershell as the default shell (instead of pwsh). Otherwise, GitLab jobs would search for pwsh on Windows images that do not contain it.

On the other hand, for conan builds, using pwsh provides the conveniency to use the same commands in both Linux and Windows and fixes file encondings (pwsh defaults to UTF8) (#16921).

However, now that powershell has been set as a shell instead of pwsh, GitLab would only look for powershell 5 instead of 7.

The only solution that I found was to create a Windows docker image and replace the original powershell.exe by pwsh.exe to be sure that the runner would call it instead of powershell 5. But that trick led to the aforementioned issue.

@czoido
Copy link
Contributor

czoido commented Dec 17, 2024

Hi @Todiq,

We have merged this for 2.11.0 that will be released soon.
This PR extends the tools.env.virtualenv:powershell to be able to specify powershell.exe to use PowerShell 5 or pwsh to use version 7. It's also possible to add arguments doing something like: tools.env.virtualenv:powershell="pwsh -NoProfile".

I'm going to consider this issue closed by that PR but please feel free to reopen if you have any more questions or need any further assistance.

Thank's a lot!

@czoido czoido closed this as completed Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants