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

Properly attribute ntextapi symbols as extern #2064

Closed
wants to merge 1 commit into from

Conversation

georgthegreat
Copy link

Summary

  • OS: Windows
  • Bug fix: no
  • Type: core

Description

At the time, an attempt to compile the binding with (on by default) clang-cl's `-fno-common leads to the following error:
psutil.lib(_psutil_common.c.py2.obj) : error LNK2005: _RtlNtStatusToDosErrorNoTeb already defined in psutil.lib(_psutil_windows.c.py2.obj).

(as to my knowledge, the best description of fcommon mechanics can be found in GCC's bugzilla.

This PR properly attributes functions as extern in every place except for the _psutil_common.c which is going to assign proper values to them.

@giampaolo
Copy link
Owner

I don't know what fcommon mechanic is. It looks like you're using some sort of non standard/default compile configuration. Also I'm not sure why you mention GCC. The only supported compiler on Windows is Visual Studio. (are you using VS?)

@georgthegreat
Copy link
Author

georgthegreat commented Oct 19, 2022

-fcommon behavior is explained e. g. here. I believe this is a historical legacy related to Fortran COMMON keyword, but this does not actually matter:

This code is a variable definition, not variable declaration:

BOOL (WINAPI *_GetLogicalProcessorInformationEx) (
    LOGICAL_PROCESSOR_RELATIONSHIP relationship,
    PSYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX Buffer,
    PDWORD ReturnLength);

(hence this variable is defined in every compile unit the header is included into)

If MSVS is able to compile this, this is clearly a bug in MSVC's linker, as duplicate symbol error should be raised instead.

As for clang-cl support, many projects switched to clang on Windows. This list includes but not limited to Firefox and Chrome.

@giampaolo
Copy link
Owner

But are you using Visual Studio? This is Windows so I don't understand why you're mentioning gcc.

@georgthegreat
Copy link
Author

We use both clang-cl and MSVS on Windows.
This PR is clang-cl specific though.

I did not make a research why MSVS is able to compile and link this.
But there is some problem indeed (the code should not link), thanks for pointing this out.

@giampaolo
Copy link
Owner

The only officially supported compiler is Visual Studio (same policy adopted by the cPython project). In the past I tried to keep mingw support alive, but it proved to be too difficult to maintain, and after some years I removed it. Since then we're basically VS only. Closing.

@giampaolo giampaolo closed this Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants