Skip to content
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.

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

Get-OSArchitectureWidth doesn't do what it says it does #828

Closed
dtgm opened this issue Jun 24, 2016 · 6 comments
Closed

Get-OSArchitectureWidth doesn't do what it says it does #828

dtgm opened this issue Jun 24, 2016 · 6 comments
Assignees
Milestone

Comments

@dtgm
Copy link
Contributor

dtgm commented Jun 24, 2016

Ref: #713 Rename Get-ProcessorBits as a more appropriately named Get-OSArchitectureWidth

Get-OSArchitectureWidth returns the current process bit-width regardless of OS arch. So in 32-bit powershell running on 64-bit windows, it returns 32 because [System.IntPtr]::Size returns 4 in 32-bit process regardless of OS architecture (32 or 64)

The helper should be replaced by testing whether process is running on 64-bit OS if the intent is to return what the helper says it will.

So if [System.IntPtr]::Size returns 4, also do test-path env:\PROCESSOR_ARCHITEW6432

if ([IntPtr]::size -eq 8) {
  $bits = 64
} elseif (([IntPtr]::size -eq 4) -and (test-path env:\PROCESSOR_ARCHITEW6432)) {
  $bits = 64
} else {
  $bits = 32
}
@dtgm
Copy link
Contributor Author

dtgm commented Jun 24, 2016

Current function should be changed from

  $bits = 64
  if ([System.IntPtr]::Size -eq 4) {
    $bits = 32
  }

to

  $bits = 64
  if (([IntPtr]::Size -eq 4) -and (test-path env:\PROCESSOR_ARCHITEW6432)) {
    $bits = 64
  } elseif ([IntPtr]::Size -eq 4) {
    $bits = 32
  }

@ferventcoder ferventcoder added this to the 0.9.10.4 milestone Jun 24, 2016
@ferventcoder
Copy link
Member

Looks good

dtgm added a commit to dtgm/choco that referenced this issue Jun 24, 2016
Get-OSArchitectureWidth returns the current process bit-width regardless of
OS arch. So in 32-bit powershell running on 64-bit windows, it returns 32
because [System.IntPtr]::Size returns 4 in 32-bit process regardless of
OS architecture (32 or 64). This fix returns actual OS bit width.
@dtgm
Copy link
Contributor Author

dtgm commented Jun 24, 2016

How about adding parameter switches to this helper? so

Get-OSArchitectureWidth alias:Get-ProcessorBits

  • -System (default) and
  • -Process to return memory addressing bit width of posh we are working in.

or would Get-ProcessorBits -Process be too confusing?

If I had a blank slate to work with, I would do Get-BitWidth because that seems most intuitive (and I'm too lazy to type Architecture)
Get-BitWidth -OS
Get-BitWidth -Process

@dtgm
Copy link
Contributor Author

dtgm commented Jun 24, 2016

Checking process is simply if ([System.IntPtr]::Size -eq 4) {"32 bit posh"}

It is currently being used in 2 helpers:

Install-ChocolateyVsixPackage.ps1#L113
Install-ChocolateyVsixPackage.ps1#L129
Get-ChocolateyUnzip.ps1#L115

I can't think of any packages that were using Get-ProcessorBits to detect bit of posh it is running in tho, but may be good to have a simple way of providing that if maintainers were with minimal change on their end?

@jberezanski
Copy link

I'd vote for Get-OSBitness and Get-ProcessBitness, personally.
The term "bitness" seems more intuitive for me, besides, "bit width" sounds technically incorrect, as a bit is always a bit and it is the pointer length / address bus width that varies.
The latter cmdlet name would also tie in nicely with Test-ProcessAdminRights.

ferventcoder pushed a commit that referenced this issue Jul 24, 2016
Get-OSArchitectureWidth returns the current process bit-width regardless of
OS arch. So in 32-bit powershell running on 64-bit windows, it returns 32
because [System.IntPtr]::Size returns 4 in 32-bit process regardless of
OS architecture (32 or 64). This fix returns actual OS bit width.
ferventcoder added a commit that referenced this issue Jul 24, 2016
* pr831:
  (GH-828) Return OS bit width, not process
ferventcoder added a commit that referenced this issue Jul 24, 2016
* stable:
  (GH-828) Return OS bit width, not process
@ferventcoder ferventcoder self-assigned this Jul 24, 2016
@ferventcoder
Copy link
Member

Merged into stable at 45ccd77

ferventcoder added a commit that referenced this issue Jul 24, 2016
Get-OSArchitectureWidth is not the most intuitive name, so add
Get-OSBitness as an alias that can be used.
ferventcoder added a commit that referenced this issue Jul 24, 2016
* stable:
  (doc) update generated docs
  (GH-828) Add Alias Get-OSBitness
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants