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

OSPlatform return win32 in win64 #2246

Closed
jecisc opened this issue Jan 15, 2019 · 14 comments
Closed

OSPlatform return win32 in win64 #2246

jecisc opened this issue Jan 15, 2019 · 14 comments

Comments

@jecisc
Copy link
Member

jecisc commented Jan 15, 2019

OSPlatform current returns an instance of Win32Platform on Windows 64bits instead of Win64Platform.

@stale
Copy link

stale bot commented Jul 16, 2019

To limit bug bankruptcy (see https://www.joelonsoftware.com/2012/07/09/software-inventory/) this issue has been automatically marked as stale because it has not had any activity in 8 months. It will be closed in 1 month if no further activity occurs. If this issue remains important to you, please comment to reactivate the issue. Thank you for your contributions.

Joel on Software
Imagine, for a moment, that you came upon a bread factory for the first time. At first it just looks like a jumble of incomprehensible machinery with a few people buzzing around. As your eyes adjus…

@stale stale bot added the stale label Jul 16, 2019
@jecisc
Copy link
Member Author

jecisc commented Jul 16, 2019

Still valid.

@stale stale bot removed the stale label Jul 16, 2019
@stale
Copy link

stale bot commented Mar 22, 2020

To limit bug bankruptcy (see https://www.joelonsoftware.com/2012/07/09/software-inventory/) this issue has been automatically marked as stale because it has not had any activity in 6 months. If no further activity occurs, it might be closed. If this issue remains important to you, please comment to reactivate the issue. Thank you for your contributions.

Joel on Software
Imagine, for a moment, that you came upon a bread factory for the first time. At first it just looks like a jumble of incomprehensible machinery with a few people buzzing around. As your eyes adjus…

@stale stale bot added the stale label Mar 22, 2020
@jecisc
Copy link
Member Author

jecisc commented Mar 22, 2020

Still valid.

@stale stale bot removed the stale label Mar 22, 2020
@astares
Copy link
Member

astares commented Mar 22, 2020

This boils down to the fact that the 64 bit VM here with Pharo 9 returns the string 'Win32' in the VM system attribute 1001
in

currentPlatformName
	"Return the name of the platform we're running on"

	^ Smalltalk vm getSystemAttribute: 1001

while

Win64Platform is checking for 'Win64'

isActivePlatform
	"Answer whether the receiver is the active platform"
	^ self currentPlatformName = 'Win64'

There is more infos in the VM parameters like 'X64':

Smalltalk vm getSystemAttribute: 1001. "'Win32'"
Smalltalk vm getSystemAttribute: 1002. "'6.2'"
Smalltalk vm getSystemAttribute: 1003. "'X64'"

Using a 32 bit VM with Pharo 9 I get:

Smalltalk vm getSystemAttribute: 1001.  "'Win32'"
Smalltalk vm getSystemAttribute: 1002.  "'6.2'"
Smalltalk vm getSystemAttribute: 1003.  "'IX86'"

so maybe we should just check for VM parameter 1003 to be 'Win32' and use parameter 1001 (X64 or IX86) additionally to distinguish.

Maybe @tesonep or @estebanlm can say something about it.

@bencoman
Copy link
Contributor

TLDR; Probably parameter 1001 should "WindowsAPI" since the bitness is covered by 1003.


The implication that there is an "platform" API called Win64 is incorrect. I had a vague impression of this situation but I wasn't completely clear, so I went a bit overboard researching this to clear up my own understanding. Happy to learn more if anyone can provide counter examples...

As mentioned here "Win32 is the name of the Windows API, similar to the role of POSIX on Unix/Linux systems. The name may have originated from 32-bit processors, but that should be seen as a historic artefact."

While use of "Win64" in common discussion and compile targets confounds the issue, a Win64 API was never released by Microsoft. The Win32 API was just made to support 64-bits, as seen in the answers when others have asked... "I'm using Visual Studio ... and tried to build my x86 project in x64 environment so I thought that I should create a win64 console project instead of win32 console project but there were not such as an option in VC2010."

Similarly, there is no mention of Win64 to Target Visual Studio for 64-Bit, x64 Platforms

Microsoft's documentation on using windows headers has no mention of Win64, only Win32. Its intro says... "The header files for the Windows API enable you to create 32- and 64-bit applications [...with...] data types that enable you to build both 32- and 64-bit versions of your application from a single source code base."

And this answer is too long to copy but provides good background info.


However all that took a while to dig up and its understandably confusing to the layman to see "Win32" mentioned for a 64-bit executable. Now Microsoft says.... " formerly called the Win32 API, the name Windows API more accurately reflects its roots in 16-bit Windows and its support on 64-bit Windows." So if there was to be any update to 1001 parameter, perhaps it should changed to a non-bit-specific "WindowsAPI" or just "Windows".

@jecisc
Copy link
Member Author

jecisc commented Mar 23, 2020

In that case, it might be better to detect the platform from argument 1001 and 1003

@bencoman
Copy link
Contributor

For comparison, other platforms are...

with no indication of bitness.

@bencoman
Copy link
Contributor

@jecisc Probably. btw, what does OSPlatform current return for 32 and 64 bit Linux?

@bencoman
Copy link
Contributor

bencoman commented Mar 23, 2020

For consistency, perhaps should replicate the Unix definitions of #isActivePlatform...

Unix32Platform>>isActivePlatform
	^ (self currentPlatformName = 'unix') and: [ Smalltalk vm wordSize = 4 ]


Unix64Platform>>isActivePlatform
	^ (self currentPlatformName = 'unix') and: [ Smalltalk vm wordSize = 8 ]

As a check (on Win10)...

  • from a 64-bit Pharo 9 image: Smalltalk vm wordSize "==> 8"
  • from a handy 32-bit Pharo 6 image: Smalltalk vm wordSize "==> 4"

So to replace problematic Windows definitions...

Win32Platform>>isActivePlatform
	^ self currentPlatformName = 'Win32'


Win64Platform>>isActivePlatform
	^ self currentPlatformName = 'Win64'

I propose something like...

Unix32Platform>>isActivePlatform
	^ (self currentPlatformName = 'windows_api') and: [ Smalltalk vm wordSize = 4 ]


Unix64Platform>>isActivePlatform
	^ (self currentPlatformName = 'windows_api') and: [ Smalltalk vm wordSize = 8 ]


OSPlatform class >> currentPlatformName
    "Historically 'win32' was the API used by even 64-bit Microsoft Windows applications.
     Later Microsoft renamed this API to the more generic 'Windows API'.  
     To ease backward compatibility of VM, the correction is encapsulated here in-Image." 

    |platform|
    platform := Smalltalk vm getSystemAttribute: 1001.
    platform = 'win32' ifTrue: [ ^ 'windows_api' ]
    ^ platform

    "For more information refer to...
       https://github.com/pharo-project/pharo/issues/2246#issuecomment-602546482"

@guillep
Copy link
Member

guillep commented Mar 27, 2020

Hi @bencoman,

I think the solution below is good. I wonder if keeping 'win32' is not good, because it goes along well with Windows APIs and documentation, so it is more familiar to people used to windows development. 'windows_api' is just a bit alien from that point of view, I'd prefer to be aligned to what other people know ^^.

Unix32Platform>>isActivePlatform
^ (self currentPlatformName = 'windows_api') and: [ Smalltalk vm wordSize = 4 ]

Unix64Platform>>isActivePlatform
^ (self currentPlatformName = 'windows_api') and: [ Smalltalk vm wordSize = 8 ]

OSPlatform class >> currentPlatformName
"Historically 'win32' was the API used by even 64-bit Microsoft Windows applications.
Later Microsoft renamed this API to the more generic 'Windows API'.
To ease backward compatibility of VM, the correction is encapsulated here in-Image."

|platform|
platform := Smalltalk vm getSystemAttribute: 1001.
platform = 'win32' ifTrue: [ ^ 'windows_api' ]
^ platform

"For more information refer to...
   https://github.com/pharo-project/pharo/issues/2246#issuecomment-602546482"

I like the change.
Any plans on doing a PR?

@astares
Copy link
Member

astares commented Mar 27, 2020

Agree with @guillep that "windows_api" is too Alien.

@astares
Copy link
Member

astares commented May 2, 2021

Update:

Smalltalk vm getSystemAttribute: 1003

now returns 'x86_64' for recent 64 bit VM. Not clear yet about all possible results for 1003.

Maybe @tesonep can respond.

@MarcusDenker
Copy link
Member

fix was merged

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

No branches or pull requests

5 participants