AcademySoftwareFoundation / rez

An integrated package configuration, build and deployment system for software
https://rez.readthedocs.io
Apache License 2.0
931 stars 330 forks source link

Powershell "eval" output style uses "&&" instead of ";" #1413

Open salvaom opened 1 year ago

salvaom commented 1 year ago

When formatting powershell code as OutputStyle.eval, rez is using && to chain the commands instead of ;. This causes the code to not be runnable by PowerShell.

from rez.resolved_context import ResolvedContext
from rez.rex import OutputStyle

ctx = ResolvedContext(["platform"])
print(ctx.get_shell_code("powershell", style=OutputStyle.eval))

This PowerShell code does not work:

Set-Item -Path "Env:REZ_USED_TIMESTAMP" -Value "1668893287"&& Set-Item -Path "Env:REZ_USED_VERSION" -Value "2.112.0"

While this one does:

Set-Item -Path "Env:REZ_USED_TIMESTAMP" -Value "1668893287"; Set-Item -Path "Env:REZ_USED_VERSION" -Value "2.112.0"

Environment

To Reproduce Execute the python snippet on the top of the issue.

Expected behavior The string get_shell_code outputs should be executable in PowerShell.

Actual behavior The string get_shell_code outputs is not executable in PowerShell due to && being used to chain commands instead of ;

bfloch commented 1 year ago

Maybe there is a spacing issue but changing to ; will change the error behavior which is not desirable. Both && and ; should be supported, but may require Powershell 7: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_pipeline_chain_operators?WT.mc_id=thomasmaurer-blog-thmaure&view=powershell-7

salvaom commented 1 year ago

I've tried and powershell has no issue with the spacing, it's just that Windows ships with PS5 and it does not support && yet.

About the error behavior, if you are referring to powershell not stopping after encountering an error, there are ways to mitigate this: https://stackoverflow.com/a/9949909/3853275

instinct-vfx commented 1 year ago

Not in front of the code atm, but we actually have two separate plugins (pwsh for Powershell core / Powershell > 7 and powershell for older powershell versions. So the powershell one could be adapted to support versions back to 5.1 (which i believe is what is shipping). I agree though that behaviour needs to be identical to current working behaviour which is pwsh (which is also the version we are using here).