8000 PSVersionTable should have entry for OS · Issue #1635 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

PSVersionTable should have entry for OS #1635

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
Closed
lzybkr opened this issue Aug 4, 2016 · 22 comments · Fixed by #3654
Closed

PSVersionTable should have entry for OS #1635

lzybkr opened this issue Aug 4, 2016 · 22 comments · Fixed by #3654
Assignees
@chunqingchen
Labels
Committee-Reviewed PS-Committee has reviewed this and made a decision Issue-Enhancement the issue is more of a feature request than a bug Resolution-Fixed The issue is fixed. Waiting - DotNetCore waiting on a fix/change in .NET WG-Engine core PowerShell engine, interpreter, and runtime
Milestone
6.0.0-beta1

Comments

@lzybkr
Copy link
Contributor
lzybkr commented Aug 4, 2016

Steps to reproduce

$PSVersionTable.OS

Expected behavior

Something useful

Actual behavior

empty string

Environment data

> $PSVersionTable
@lzybkr lzybkr mentioned this issue Aug 4, 2016
Merged
@oising
Copy link
Contributor
oising commented Aug 4, 2016

Host sniffing is bad; testing for features is good. Is this something we envisage being useful information in the grand scheme things? Esp. since there's an issue somewhere else tracking a desire for a Get-OperatingSystemInfo cmdlet/function.

@lzybkr
Copy link
Contributor Author
lzybkr commented Aug 4, 2016

Good point - I was thinking more from a "how can I repro" perspective, but folks would likely start using the property in scripts thinking it's useful.

Maybe we need a Get-PowerShellReproEnvironment cmdlet that we ask folks to run instead.

@SteveL-MSFT SteveL-MSFT modified the milestone: 6.0.0-Alpha.11 Aug 4, 2016
@rkeithhill
Copy link
Collaborator

And it might be helpful for such an environment to list loaded module names & version numbers as well. Might also be nice to have list the architecture (x86, x64, ARM32) of the OS and PowerShell (for those cases where x86 PS is running on x64 Windows).

@lzybkr lzybkr mentioned this issue Aug 21, 2016
Closed
@Jaykul
Copy link
Contributor
Jaykul commented Aug 21, 2016

@oising: testing for features is good, but you don't want to test for features to decide simple things like what library to load. E.g. if I ship System.Data.SqlClient in my module, it has .Net452, .Net46, and separate .NetStandard assemblies for Windows and for Unix. Just to be able to load the right one I need to know OS and Arch. I think it would be enough to know:

  1. OS: Windows or Unix
  2. Architecture: ARM, amd64, x86

@vors vors added WG-Engine core PowerShell engine, interpreter, and runtime Issue-Enhancement the issue is more of a feature request than a bug labels Sep 9, 2016
@rkeithhill
Copy link
Collaborator

Just do it. It will make it easier for scripts to know where to look for environmental info and it will make it easier for you to get bug reports with that same environment info.

@SteveL-MSFT SteveL-MSFT mentioned this issue Oct 28, 2016
Closed
@joeyaiello joeyaiello mentioned this issue Oct 28, 2016
Closed
@SteveL-MSFT SteveL-MSFT mentioned this issue Oct 28, 2016
Closed
@SteveL-MSFT SteveL-MSFT modified the milestones: 6.0.0-beta, 6.0.0 Dec 6, 2016
@joeyaiello
Copy link
Contributor
joeyaiello commented Dec 14, 2016 •
edited
Loading

Yeah, I used to agree with @oising's opinion here that we shouldn't expose this because it encourages people to "do the wrong thing" when checking for certain kinds of compat at runtime.

Until I saw the kind of lengths people will go to in order to check for these sorts of things anyway. They'll do it one way or another, and at least this way it's easier to understand when and why people are checking for OS so we can mitigate it in the future. (E.g. I want to know that @Jaykul is testing for OS because of x-CLR assembly issues, so that I know we need to adopt .NET Standard 2.0).

That being said, I still agree that checking for features is generally the right thing to do, and we should absolutely have a PSSA rule to throw warnings on checks against $PSVersionTable.OS at runtime.

Oh, and of course, we still need to design this thing. Is it Windows, Mac, Linux? Or do we get down to the distro level? Nano Server? Linux kernel version? Probably needs an RFC...

@Jaykul
Copy link
Contributor
Jaykul commented Jan 14, 2017

But please ... get rid of the top level $IsWindows, $IsLinux variables, that's ... icky.

@joeyaiello
Copy link
Contributor

@PowerShell/powershell-committee I agree w/ @Jaykul on this one. We should try and move to CoreFX's Environment.OSVersion when it's available (looks like 2.0 timeframe): dotnet/corefx#9851

The next question is whether or not we should propagate $PSVersionTable with Environment.OSVersion or make users search the web to learn about it (leaning towards the former, but I'm open to being told I'm wrong here).

@joeyaiello joeyaiello added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Jan 23, 2017
@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 labels Jan 26, 2017
@SteveL-MSFT
Copy link
Member
8000 SteveL-MSFT commented Jan 26, 2017

@PowerShell/powershell-committee reviewed this, we agree that $psversiontable should use Environment.OSVersion to populate the new OS property. On the topic of $IsWindows/$IsLinux/$IsMac, we agree that the capability to check for the OS via a variable is needed, but should consider implemented in a different way (ie, a prefix to avoid collision or make them members of a feature check variable like $PSFeature.IsWindows). Recommendation is we should have a RFC for the OS check variable. @joeyaiello to author.

This was referenced Mar 1, 2017
Closed
Closed
@iSazonov iSazonov added the Waiting - DotNetCore waiting on a fix/change in .NET label Mar 8, 2017
@iSazonov
Copy link
Collaborator
iSazonov commented Mar 8, 2017 •
edited
Loading

I looked CoreFX and didn't found results of Unix tests for Environment.OSVersion under code coverage. CoreFX parse a kernel's uname (see here and here) so we can get bad UX if it will be not tested on all supported Unix distributives.

Also we use in Build.psm1 information from /etc/os-release. It is Linux "standart". We would use this if we would map Windows properties to /etc/os-release properties.

@SteveL-MSFT SteveL-MSFT mentioned this issue Mar 9, 2017
Closed
@daxian-dbw daxian-dbw self-assigned this Mar 28, 2017
@iSazonov iSazonov mentioned this issue Apr 10, 2017
Closed
@daxian-dbw daxian-dbw removed their assignment Apr 13, 2017
@daxian-dbw
Copy link
Member

System.Environment.OSVersion is back in netstandard2.0, but it's not very useful on unix platform. For example, on Ubuntu 16.04:

PS /> [System.Environment]::OSVersion | fl *                                                        

Platform      : Unix
ServicePack   : 
Version       : 4.8.0.41
VersionString : Unix 4.8.0.41

System.Runtime.InteropServices.RuntimeInformation.OSDescription may be a potential substitution:

PS /> [System.Runtime.InteropServices.RuntimeInformation]::OSDescription                            
Linux 4.8.0-41-generic #44~16.04.1-Ubuntu SMP Fri Mar 3 17:11:16 UTC 2017

@SteveL-MSFT
Copy link
Member

OSDescription looks like part of the output from uname -a. Seems like the way to go.

@chunqingchen chunqingchen self-assigned this Apr 14, 2017
@chunqingchen
Copy link
Contributor

taking a look on this

@iSazonov
Copy link
Collaborator

If the next what we want, then we could ask CoreFX team:

PS /> [System.Environment]::OSVersion | fl *                                                        

Platform      : Unix
ServicePack   : 
Version       : 4.8.0.41
VersionString : Ubuntu 16.04 

PS /> [System.Environment]::OSVersion | fl *                                                        

Platform      : Windows
ServicePack   : 
Version       : 10.0.15063
VersionString : Windows 10 Creator

@joeyaiello
Copy link
Contributor

I think OSVersion.Platform and OSVersion.Version are both super useful as they don't require any string parsing to get at Windows v. non-Windows and the kernel version. Beyond that, I think we should expose [System.Runtime.InteropServices.RuntimeInformation]::OSDescription as our own uname -a string as well.

I know this might be ballooning $PSVersionTable, but it all feels like useful info.

@chunqingchen
Copy link
Contributor

@joeyaiello could you give me a mock information $PSVersionTable.OS should give out?
for example, should $PSVersionTable.OS looks like this? (combine the osversion and osdescription)
$PSVersionTable.OS
Platform : Unix
ServicePack :
Version : 4.8.0.41
VersionString : Unix 4.8.0.41
OSDescription: Linux 4.8.0-41-generic #44~16.04.1-Ubuntu SMP Fri Mar 3 17:11:16 UTC 2017

@SteveL-MSFT
Copy link
Member

@joeyaiello Do we really want Platform since we're keeping $IsUnix/$IsWindows/etc... for automation purposes? Seems like OSDescription is the only thing we really need here primarily for logging purposes.

@joeyaiello
Copy link
Contributor

@chunqingchen Yeah, I guess just make $PSVersionTable.OS equivalent to [System.Environment]::OSVersion for now. That's fine.

I'm not super sold on $IsUnix/$IsWindows, but I'd like to understand .NET Core 2.0's platform differentiation a little better before I form a strong opinion.

@rkeithhill
Copy link
Collaborator

BTW the longer you wait, the more modules get published that rely on $Is*. :-) posh-git aleady relies on these variables but we could switch to the "preferred way" of testing for the OS platform.

@SteveL-MSFT
Copy link
Member

@joeyaiello did you really mean [System.Environment]::OSVersion (which is not really useful) or [System.Runtime.InteropServices.RuntimeInformation]::OSDescription which is equivalent to uname -a? I was saying the latter makes sense

@joeyaiello
Copy link
Contributor

Per our discussion today, I'm on board with the recommendation from @SteveL-MSFT.

We also agreed that OSVersion.Platform should probably be included as well. It's possible that $IsWindows and its ilk should become "dumb accelerators" and query that directly.

@iSazonov
Copy link
Collaborator

Is [System.Runtime.InteropServices.RuntimeInformation]::OSDescription really always different?
I mean can we distinguish between Ubuntu LTS, Release and so on? Windows Desktop, Server, Core, IoT?

@chunqingchen chunqingchen mentioned this issue Apr 26, 2017
Merged
@iSazonov iSazonov added the Resolution-Fixed The issue is fixed. label May 2, 2017
@iSazonov iSazonov mentioned this issue Oct 18, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chunqingchen chunqingchen

Labels
Committee-Reviewed PS-Committee has reviewed this and made a decision Issue-Enhancement the issue is more of a feature request than a bug Resolution-Fixed The issue is fixed. Waiting - DotNetCore waiting on a fix/change in .NET WG-Engine core PowerShell engine, interpreter, and runtime
Projects
None yet
Milestone
6.0.0-beta1
Development

Successfully merging a pull request may close this issue.

PSVersionTable should have entry for OS and Platform
0