Skip to content

Add powershell scripts #21158

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add powershell scripts #21158

wants to merge 1 commit into from

Conversation

hamzaremmal
Copy link
Member

No description provided.

@hamzaremmal hamzaremmal added release-notes Should be mentioned in the release notes needs-minor-release This PR cannot be merged until the next minor release labels Jul 10, 2024
@hamzaremmal hamzaremmal self-assigned this Jul 10, 2024
@Gedochao
Copy link
Contributor

This has been decided to be included in 3.6.0

@SethTisue
Copy link
Member

SethTisue commented Jul 17, 2024 •
edited
Loading

Perhaps some others shared my misconception that scala would directly be scala-cli — but I understand now that the new scala is actually a short script that invokes scala-cli.

@bishabosha
Copy link
Member

bishabosha commented Jul 17, 2024 •
edited
Loading

@SethTisue there's a strange bootstrapping issue of baking in the default scala version, and providing an offline classpath , otherwise we'd have to build scala-cli from source in scala3 ci

@hamzaremmal hamzaremmal force-pushed the powershell-support branch 2 times, most recently from c442eaa to a7a27cd Compare July 30, 2024 12:55
@hamzaremmal hamzaremmal force-pushed the powershell-support branch 7 times, most recently from 38da491 to 563db30 Compare October 6, 2024 17:12
@hamzaremmal hamzaremmal marked this pull request as ready for review October 6, 2024 21:06
@hamzaremmal hamzaremmal requested a review from bishabosha October 6, 2024 21:06
@hamzaremmal hamzaremmal assigned bishabosha and unassigned hamzaremmal Oct 6, 2024
bishabosha
bishabosha reviewed Oct 7, 2024
View reviewed changes
dist/bin/scaladoc.ps1
$_QUIET = $true
}
"-colors" {
$_COLORS = $true
Copy link
Member

@bishabosha bishabosha Oct 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

you're basically swallowing this setting without passing it on, same with $_QUIET, I understand the bat file did that too for compat - but why bother setting the variable

bishabosha
bishabosha reviewed Oct 7, 2024
View reviewed changes
dist/libexec-native-overrides/cli-common-platform.ps1 Outdated
@@ -0,0 +1,9 @@
function Scala-GetScalaCLIBinaryName {
if ($PSVersionTable.PSEdition -eq "Core") {
return 'scala-cli'
Copy link
Member

@bishabosha bishabosha Oct 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The .NET core still runs on Windows, does this work still?

https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_powershell_editions?view=powershell-7.4#long-description

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, good catch. That means I also have to change the Get-JavaClasspathSeparator function. let me do it right now.

hamzaremmal
hamzaremmal commented Oct 8, 2024
View reviewed changes
dist/libexec-native-overrides/cli-common-platform.ps1
@@ -0,0 +1,9 @@
function Get-ScalaCLIBinaryName {
if ($IsWindows) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This now requires PowerShell >= 6.0

See: https://learn.microsoft.com/en-us/previous-versions/powershell/scripting/whats-new/what-s-new-in-powershell-core-60?view=powershell-6#engine-updates

Copy link
Member

Choose a reason for hiding this comment

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

Which released in 2023, is that too new?

Copy link
Member Author

Choose a reason for hiding this comment

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

2018: https://github.com/PowerShell/PowerShell/releases/tag/v6.0.0

is that too new?

Mmmh, the issue with this is that some scripts might not work on some machines without installing pwsh >= 6.0

Copy link
Member

@bishabosha bishabosha Oct 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

oh ok, the blog you linked says written 2023, so my bad

@hamzaremmal hamzaremmal requested a review from bishabosha October 8, 2024 09:33
@WojciechMazur
Copy link
Contributor

What's the status of these changes? What's still missing before we're be able to merge and release in 3.6.0?

@hamzaremmal
Copy link
Member Author

We will not merge it for 3.6.0 (maybe in a few months but it's too soon). Sorry, I forgit to mention it on the PR !

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

@bishabosha bishabosha Awaiting requested review from bishabosha

At least 1 approving review is required to merge this pull request.

Assignees

@bishabosha bishabosha

Labels
needs-minor-release This PR cannot be merged until the next minor release release-notes Should be mentioned in the release notes stat:do not merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@hamzaremmal @Gedochao @SethTisue @bishabosha @WojciechMazur