Skip to content

Fix generated PowerShell init script - #7070

Open
flandia wants to merge 6 commits into
ocaml:masterfrom
flandia:agent/fix-powershell-init-condition
Open

Fix generated PowerShell init script#7070
flandia wants to merge 6 commits into
ocaml:masterfrom
flandia:agent/fix-powershell-init-condition

Conversation

@flandia

@flandia flandia commented Aug 10, 2026

Copy link
Copy Markdown

Problem

Since #5864, opam has generated the following PowerShell:

if Test-Path "...\variables.ps1" { ... }

PowerShell requires parentheses around an if condition and rejects this
script with:

Missing '(' after 'if' in if statement.

Fix

Generate valid PowerShell syntax:

if (Test-Path "...\variables.ps1") { ... }

The Windows reftest now executes the generated script using Windows
PowerShell, ensuring future syntax errors fail the test.

Testing

dune build --profile=release --root . '@reftest-init-scripts.win32' --force

The generated script was also executed successfully with Windows PowerShell
5.1 and PowerShell 7.

### cat root/opam-init/init.ps1
if Test-Path "${BASEDIR}/root/opam-init/variables.ps1" { . "${BASEDIR}/root/opam-init/variables.ps1" *> $null }
if (Test-Path "${BASEDIR}/root/opam-init/variables.ps1") { . "${BASEDIR}/root/opam-init/variables.ps1" *> $null }
### powershell.exe -NoLogo -NoProfile -NonInteractive -ExecutionPolicy Bypass -File root/opam-init/init.ps1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neat idea to at least test the syntax, it should probably be done for every shell we support (you're welcome to do it but it's not strictly necessary for this PR so no pressure)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to add them but I'm not sure if it works. Would you approve the workflow and test it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workflow failed and I guess that's due to an upstream problem which has been fixed. I updated the branch. Would you approve the workflow again? Thanks

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, you can open a PR on your own fork and enable GitHub Action on it, if you want to debug something. This way i don't have to approve it every time

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tip. The checks succeeded. If maintainers do not have further comments, I believe this PR is now ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants