Skip to content

Conversation

@fMichaleczek
Copy link
Contributor

@fMichaleczek fMichaleczek commented Feb 4, 2025

PR Summary

Add static properties IsMonoRuntime, IsBrowser, IsWasi, IsAndroid, IsSingleFile to SMA.Platform

PR Context

Added :

  • System.Management.Automation.Platform.IsMonoRuntime
  • System.Management.Automation.Platform.IsBrowser
  • System.Management.Automation.Platform.IsWasi
  • System.Management.Automation.Platform.IsAndroid
  • System.Management.Automation.Platform.IsSingleFile

Related issues :

PR Checklist

@iSazonov iSazonov added WG-NeedsReview Needs a review by the labeled Working Group Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Feb 4, 2025
@SteveL-MSFT
Copy link
Member

I agree that exposing the information is useful and could be extended in the future. However, I don't want a bunch of $Is* variables particularly as some of them may conflict with existing user usage. Instead, my initial proposal would be a container variable: $PSEnvironment. Then it would make sense to bucketize the different aspects:

$PSEnvironment.OperatingSystem -eq [System.Management.Automation.Environment]::Windows
$PSEnvironment.OperatingSystem -eq 'Windows'

$PSEnvironment.Packaging -eq 'SingleExe'
$PSEnvironment.Architecture -eq 'Wasi'

This also means you can dump out all the info using just

$PSEnvironment

@fMichaleczek
Copy link
Contributor Author

fMichaleczek commented Feb 6, 2025

@SteveL-MSFT I would like to merge only new Platform properties to prepare next PR for Browser and don't want to repeat the works for others Mono platforms.
I can make them internal for 7.6-Preview timeline if you prefer.

Platform property naming should stick to dotnet runtime/sdk convention :

Platform Property Pattern
IsBrowser Use dotnet OperatingSystem.IsBrowser
IsWasi Use dotnet OperatingSystem.IsWasi
IsAndroid Use dotnet OperatingSystem.IsAndroid
IsMonoRuntime MSBuild Property is UseMonoRuntime, detection is based on type Mono.RuntimeStructs
IsSingleFile MSBuild Property is PublishSingleFile

The IsSingleFile implementation is naive (Assembly Location is empty) because the SingleFile Bundle has multiple implementations between an generic AppHost app and a WasmAppHost.

Platforming for Browser is about compiling SMA for net9.0-browser with less dependencies.
If we remove dependency "Microsoft.Win32.Registry" for net9.0-Browser, do I need to add a IsRegistrySupported property on Platform ? The answer is no for this one, but it’s the kind of question to think about.
https://learn.microsoft.com/en-us/nuget/create-packages/supporting-multiple-target-frameworks#declaring-dependencies-advanced

{1D148F65-9832-4021-A5EB-2C7D166FF40B}

.

@iSazonov
Copy link
Collaborator

iSazonov commented Feb 6, 2025

Why create a new entity? Why not use what you already have? I mean $PSVersion? We even use SG for this variable, i.e. we could form values at compile time.

@fMichaleczek
Copy link
Contributor Author

fMichaleczek commented Feb 6, 2025

This PR is a change on the class System.Management.Platform in a scenario of using Microsoft.PowerShell.Sdk with other TFMs than those distributed by Microsoft.

A RFC is mandatory to endorse a discussion about $PSEnvironment vs $PSPlatform vs $PSVersion vs etc..
I suggest not continuing the topic here.

Are we able to merge these changes ? (this PR) :

  • System.Management.Automation.Platform.IsMonoRuntime
  • System.Management.Automation.Platform.IsBrowser
  • System.Management.Automation.Platform.IsWasi
  • System.Management.Automation.Platform.IsAndroid
  • System.Management.Automation.Platform.IsSingleFile

@SteveL-MSFT
Copy link
Member

Why create a new entity? Why not use what you already have? I mean $PSVersion? We even use SG for this variable, i.e. we could form values at compile time.

If we continue to add environment type information, $PSVersionTable gets more misleading on what it represents, so I don't think we want to just keep adding non-version info to that variable.

@SteveL-MSFT
Copy link
Member

SteveL-MSFT commented Feb 19, 2025

@PowerShell/powershell-committee discussed this again, here's what we would accept as an Experimental Feature the following:

  • A new $PSEnvironment built-in variable that contains:
    • OperatingSystem which would be an enum [System.Management.Automation.Environment.OperatingSystem] with values Windows, Linux, macOS, Android
      • This list can be extended in the future depending on community needs and what our team publishes (and what dotnet supports)
    • Package which would be an enum [System.Management.Automation.Environment.Package] with values SingleExe, FrameworkDependent, SelfContained
    • Architecture which would be an enum [System.Management.Automation.Environment.Architecture] with values WebAssembly, x64, x86, arm32, arm64, MSIL
      • We chose WebAssembly instead of Browser, also Wasi was not included as it's not clear to us how this would be used

@SteveL-MSFT SteveL-MSFT added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee WG-NeedsReview Needs a review by the labeled Working Group labels Feb 19, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Feb 27, 2025
@microsoft-github-policy-service
Copy link
Contributor

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

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

Labels

Committee-Reviewed PS-Committee has reviewed this and made a decision Review - Needed The PR is being reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants