Skip to content

(chore): Using PowerShell Core instead of PowerShell Desktop#46

Merged
niheaven merged 1 commit into
ScoopInstaller:mainfrom
baneeishaque:main
Jul 30, 2024
Merged

(chore): Using PowerShell Core instead of PowerShell Desktop#46
niheaven merged 1 commit into
ScoopInstaller:mainfrom
baneeishaque:main

Conversation

@baneeishaque
Copy link
Copy Markdown
Contributor

No description provided.

@baneeishaque baneeishaque requested a review from a team as a code owner July 29, 2024 18:32
@baneeishaque
Copy link
Copy Markdown
Contributor Author

Is there any specific reason to stick to PowerShell Desktop instead of PowerShell Core?

Will anything break if we switch from PowerShell Desktop to PowerShell Core? If something breaks and we can fix it on the Scoop side, we can work on it. But, if it needs to be fixed on the PowerShell Core side - we can try to work on it.

Most of my manifests leverage on checkver.script field and these scripts are developed under PowerShell Core. So, these are not compatible with PowerShell Desktop. That's why, I want to switch from PowerShell Desktop to PowerShell Core in my Excavator GitHub Actions.

If we want to stick on PowerShell Desktop for some reason, I have to migrate the scripts to PowerShell Desktop compatibility. Since PowerShell Core is the future, I think that migration is a waste of resources. Anyway, If migration is necessary - I am ready for it.

@HUMORCE
Copy link
Copy Markdown
Member

HUMORCE commented Jul 29, 2024

  • Windows PowerShell is a pre-installed system component that stopped updating with new features but still in lifecycle.
  • PowerShell Core is not a replacement for Windows PowerShell.

Scoop should work out of the box. Even the Scoop Core project itself makes limited use of the new features available in PowerShell Core.

Copy link
Copy Markdown
Member

@rasa rasa left a comment

Choose a reason for hiding this comment

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

If it introduces issues, we can always revert it back.

@niheaven
Copy link
Copy Markdown
Member

It's GitHub Actions, using PowerShell should be ok, IMO.

@niheaven niheaven merged commit 342b3ac into ScoopInstaller:main Jul 30, 2024
@chawyehsu
Copy link
Copy Markdown
Member

chawyehsu commented Jul 30, 2024

Hi, sorry for late repsonse on this, but this has to be reverted as it is intentional to use powershell to run actions. The ConvertFrom-Json function from pwsh can be problematic because it can not detect tailing commas when validating JSON files.

There had been issues of invalid JSON manfiests being committed to official buckets, led to checks for Scoop being taken down on repology. We don't want this to happen again and that's why powershell was used (ConvertFrom-Json from powershell throws error on tailing commas detected).

For 3rd party buckets, if you don't care about PowerShell 5 compatiblity, which is still officially supported by Scoop, you're always free to fork this actions repo and switch to the fork with the running shell changed to pwsh for your own buckets.

Tip: if you are not sure why one line of code is written like that or want to know the commit histroy of that line of code, you may use git blame to do such investigation, which may provide you context you might not know, before taking further action. It is also available on the GitHub web interface.
image

niheaven added a commit that referenced this pull request Jul 30, 2024
@niheaven
Copy link
Copy Markdown
Member

Reverts by 944bf8e and requires further investigation

xrgzs pushed a commit to xrgzs/ScoopGithubActions that referenced this pull request Nov 9, 2024
@o-l-a-v
Copy link
Copy Markdown
Contributor

o-l-a-v commented Feb 20, 2025

Seems PowerShell >= v7.4 supports detecting trailing commas with Test-Json:

image

So if $PSversionTable.PSVersion -ge '7.4' do a Test-Json too?

niheaven added a commit to o-l-a-v/ScoopInstaller--GithubActions that referenced this pull request Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants