[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[Steve Dower]

Steve Dower  added the comment:

> Microsoft provides stat and struct stat, but they prepend the names with an 
> underscore.
They are also influenced by various compiler options to choose between 
32-bit and 64-bit fields. This makes it impossible to use the standard 
names as part of an ABI, because we can't/don't enforce that the 
preprocessor definitions match.

We should isolate all structures from libc/equivalent in our public API 
so that we can ensure compatibility. (FILE* was historically also an 
issue, but that was bad enough that Windows fixed it on their side. The 
rest of the C runtime library still bleeds everywhere, so we definitely 
don't want it or its semantics in our public API if avoidable.)

--
title: Building Python with clang on Windows fails on _Py_stat(): struct stat 
is not defined -> _Py_stat and _Py_wstat using incorrect type for status 
argument

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[Paul Campbell]

Paul Campbell  added the comment:

Victor: The changes in the main branch gets me past this issue without having 
to make additional changes.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

STINNER Victor  added the comment:

> Seems it would go back to depending on some type nonstandard python macro to 
> translate between the two during build.

In the internal C API, there are less concerns about writing portable code. We 
expect users of this API to pay more attention to what they do and there is no 
backward compatibility warranty.

In the internal C API, it's acceptable to change the parameter type depending 
on the platform.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[Paul Campbell]

Paul Campbell  added the comment:

> In Python, we are trying to provide a same C API on all platforms. If "struct 
> stat" is no longer considered as portable, IMO we should attempt to avoid it, 
> at least in the public C API.

Microsoft provides stat and struct stat, but they prepend the names with an 
underscore. So functions like stat are named _stat and struct stat is named 
struct _stat, etc. Not sure if pros/cons of using such functions are though. 
Seems it would go back to depending on some type nonstandard python macro to 
translate between the two during build.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

STINNER Victor  added the comment:

> Hi Victor, I was trying to compile with clang on Windows 10. I will try to 
> pull your 3.11 changes and test. Sorry to cause so much churn. It looked to 
> me like a simple issue that was missed, probably because whatever was trying 
> to compile was not normally compiled on Windows. I was not trying to make a 
> lot of work to support a new platform :)

I tried to write a change which doesn't increase the maintenance on other 
platforms.

I dislike declaring private functions in the *public* Python.h header file, 
especially if they cause build error. Over the last years, I moved many private 
functions to the internal C API.

In Python, we are trying to provide a same C API on all platforms. If "struct 
stat" is no longer considered as portable, IMO we should attempt to avoid it, 
at least in the public C API. For these reasons, I dislike PR 30478. The 
problem is that if the work is written on Linux using "strut stat", the build 
can fail on Windows if "struct _Py_stat_struct" is now required on Windows. I 
expected "struct stat" to be portable, but it seems like I was wrong.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[Christian Heimes]

Christian Heimes  added the comment:

It's unlikely that you can reproduce the issue with clang. We use MSVC and a 
manually maintained pyconfig.h on Windows.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[Paul Campbell]

Paul Campbell  added the comment:

Hi Victor, I was trying to compile with clang on Windows 10. I will try to pull 
your 3.11 changes and test. Sorry to cause so much churn. It looked to me like 
a simple issue that was missed, probably because whatever was trying to compile 
was not normally compiled on Windows. I was not trying to make a lot of work to 
support a new platform :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

STINNER Victor  added the comment:

If possible, I would prefer to not change 3.9 and 3.10 to avoid any risk of 
introducing a *new* build error, while trying to support a new platform. I 
don't think that we currently supporting build Python with clang on Windows yet.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

STINNER Victor  added the comment:

Paul: Can you please try to build the main branch of Python with clang and tell 
me if you still have the compiler warnings? If yes, can you please copy/paste 
the compiler warnings?

Do you build Python on Windows or from another OS (cross-compilation)?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

STINNER Victor  added the comment:

Christian Heimes: "I set the release blocker flag for the ticket."

It's just a compiler warning, why marking it as a release blocker?

Anyway, it's now fixed.

--
priority: release blocker -> 

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 08bc1bad11cad39f508bd662c9b28fcd9c995512 by Victor Stinner in 
branch 'main':
bpo-46303: Fix fileutils.h compiler warnings (GH-30550)
https://github.com/python/cpython/commit/08bc1bad11cad39f508bd662c9b28fcd9c995512


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[Christian Heimes]

Christian Heimes  added the comment:

I set the release blocker flag for the ticket.

--
nosy: +christian.heimes

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[Christian Heimes]

Change by Christian Heimes :


--
priority: normal -> release blocker

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[Guido van Rossum]

Change by Guido van Rossum :


--
nosy: +gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

STINNER Victor  added the comment:

I marked bpo-46346 as a duplicate of this issue. Copy of the first message:

"""
I am getting these warnings:

C:\Users\gvanrossum\cpython\PC\_testconsole.c(70,38): warning C4013: 
'_Py_get_osfhandle' undefined; assuming extern returnin
g int [C:\Users\gvanrossum\cpython\PCbuild\_testconsole.vcxproj]
C:\Users\gvanrossum\cpython\PC\_testconsole.c(70,1): warning C4047: 
'initializing': 'HANDLE' differs in levels of indirectio
n from 'int' [C:\Users\gvanrossum\cpython\PCbuild\_testconsole.vcxproj]
C:\Users\gvanrossum\cpython\Modules\_tkinter.c(144,37): warning C4013: 
'_Py_stat' undefined; assuming extern returning int [
C:\Users\gvanrossum\cpython\PCbuild\_tkinter.vcxproj]

I noticed that GitHub also was flagging these in unrelated PRs.
"""

I proposed GH-30550 to fix these warnings.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +28751
pull_request: https://github.com/python/cpython/pull/30550

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

STINNER Victor  added the comment:

It seems like _Py_stat() and _Py_wstat() are only needed on non-Windows 
platforms: only the _get_tcl_lib_path() function of Modules/_tkinter.c uses 
_Py_stat(). I wrote PR 30539 to not define _Py_stat() and _Py_wstat() on 
Windows.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +28739
pull_request: https://github.com/python/cpython/pull/30539

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

STINNER Victor  added the comment:

Python.h indirectly (via fileutils.h) defines _Py_wstat() and _Py_stat() 
functions which use the "struct stat*" type for 12 years:

commit 4e31443c4d2c1fb211a6ea90fc6a8fbd9ff81c97
Author: Victor Stinner 
Date:   Thu Oct 7 21:45:39 2010 +

Create fileutils.c/.h

 * _Py_fopen() and _Py_stat() come from Python/import.c
 * (_Py)_wrealpath() comes from Python/sysmodule.c
 * _Py_char2wchar(), _Py_wchar2char() and _Py_wfopen() come from 
Modules/main.c
 * (_Py)_wstat(), (_Py)_wgetcwd(), _Py_wreadlink() come from 
Modules/getpath.c

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[Paul Campbell]

Paul Campbell  added the comment:

I was trying to build python core (-DMS_WINDOWS -DPy_BUILD_CORE). I was using 
clang, which I think is unsupported looking at Windows doc. After looking at 
the issue though, it seemed that it was just some slight mistake which is why I 
filed the bug.

clang version 13.0.0
Target: x86_64-pc-windows-msvc
Thread model: posix

I can test to see if it fixes the immediate build problem, but I still find 
your fix not quite addressing the issue which I initially tried to create a 
patch for. Someone has already developed a shim here for Windows and it just 
was not used properly. `_Py_stat_struct` is a define which either evaluates to 
`stat` on non-Windows systems or a compatibility structure on Windows. Simply 
replacing the use of `struct stat` with `struct _Py_stat_struct` should solve 
the issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

STINNER Victor  added the comment:

> While attempting to embed the full cpython source in my application, I found 
> that during compilation on Windows, there was a compilation issue due to 
> struct stat not being defined.

Do you get the error when building Python? Or on #include  when using 
the Python C API?

How do you build Python? What is your C compiler?

Can you test if my proposed PR 30528 fix your issue?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +28729
pull_request: https://github.com/python/cpython/pull/30528

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

STINNER Victor  added the comment:


New changeset ea1a54506b4ac38b712ba63ec884292025f16111 by Victor Stinner in 
branch 'main':
bpo-46303: Move fileutils.h private functions to internal C API (GH-30484)
https://github.com/python/cpython/commit/ea1a54506b4ac38b712ba63ec884292025f16111


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

STINNER Victor  added the comment:

It's not the first time that private functions included by the public Python.h 
are causing build errors event if these functions are not used. The previous 
issue were functions for atomic operations. I solved this build error by moving 
the whole private C API into the internal C API: 
Include/internal/pycore_atomic.h. Since that time, we no longer got build error 
related to this header file.

I propose a similar fix: move all private fileutils.h functions from 
Include/cpython/fileutils.h to the internal Include/internal/pycore_fileutils.h.

I wrote PR 30484 to implement this change.

To keep the implementation simple, I kept _Py_fopen_obj() in 
Include/cpython/fileutils.h, for _testcapi which must not use the internal C 
API.

If this PR is merged, for Python 3.9 and 3.10, I will write a simpler change: 
modify Include/cpython/fileutils.h to only define functions using "struct stat" 
in the internal C API (if Py_BUILD_CORE is defined).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +28688
pull_request: https://github.com/python/cpython/pull/30484

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46303] _Py_stat and _Py_wstat using incorrect type for status argument

[Paul Campbell]

Change by Paul Campbell :


--
title: _Py_stat using incorrect type for status argument -> _Py_stat and 
_Py_wstat using incorrect type for status argument

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com