classification
Title: windows: cross compilation fails due to headers with uppercase
Type: compile error Stage: resolved
Components: Cross-Build, Windows Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Alex.Willmer, erikjanss, eryksun, methane, miss-islington, paul.moore, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2018-07-24 23:42 by erikjanss, last changed 2018-09-06 21:31 by erikjanss. This issue is now closed.

Files
File name Uploaded Description Edit
windows_headers.py erikjanss, 2018-08-15 19:14 python3 script to detect uppercases in windows headers
Pull Requests
URL Status Linked Edit
PR 8453 merged erikjanss, 2018-07-24 23:47
PR 8472 merged erikjanss, 2018-07-29 09:03
PR 8560 merged miss-islington, 2018-07-30 06:03
PR 8782 merged miss-islington, 2018-08-16 06:41
Messages (20)
msg322333 - (view) Author: Erik Janssens (erikjanss) * Date: 2018-07-24 23:42
When cross compiling python on a linux host with a case sensitive file system for a windows target.  The compilation will fail due to header
files which contain uppercases.
msg322335 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-07-25 00:41
New changeset 6cf8255912c36fec6f87f62513034d0818f61390 by INADA Naoki (erikjanss) in branch 'master':
bpo-34217: Use lowercase header for Windows (GH-8453)
https://github.com/python/cpython/commit/6cf8255912c36fec6f87f62513034d0818f61390
msg322655 - (view) Author: miss-islington (miss-islington) Date: 2018-07-30 06:15
New changeset 6c89a9a4c7d1804e16038f9ee3a0b2efaa4bdee6 by Miss Islington (bot) in branch '3.7':
bpo-34217: Use lowercase header for Windows (GH-8453)
https://github.com/python/cpython/commit/6c89a9a4c7d1804e16038f9ee3a0b2efaa4bdee6
msg323582 - (view) Author: Erik Janssens (erikjanss) * Date: 2018-08-15 19:14
Attached a small helper script to detect camelcase windows headers,
2 more occurences found :

Modules/socketmodule.c versionhelpers.h 306 #include <VersionHelpers.h>
Tools/msi/bundle/bootstrap/pch.h uxtheme.h 18 #include <Uxtheme.h>
msg323586 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2018-08-15 23:12
Are those bugs? On Windows, they are spelled with that casing (and while NTFS is case-insensitive, it *is* case-preserving).

Whatever package you have used for cross-compiling may have renamed them (similarly Shlwapi.h). In which case, perhaps you could request that the package you are using preserve the case of the files it is cloning, rather than changing it?
msg323594 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-08-16 06:41
New changeset e6a4755e6793942b950c1595e0c34bd66a0ee13e by INADA Naoki (Erik Janssens) in branch 'master':
bpo-34217: Use lowercase for windows headers (GH-8472)
https://github.com/python/cpython/commit/e6a4755e6793942b950c1595e0c34bd66a0ee13e
msg323595 - (view) Author: Erik Janssens (erikjanss) * Date: 2018-08-16 06:49
@inada.naoki : thank you !

@steve.dower : I would not consider those bugs per se, but they are
inconsistencies, for example in the master branch :

number of "Windows.h" includes : 2
number of "windows.h" includes : 55

whatever might be the 'right' solution, it's always an advantage to
have a consistent codebase.
msg323596 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-08-16 07:28
New windows will support case sensitive filesystem too.
Stop relying to case insensitive filesystem makes sense to me.
msg323597 - (view) Author: miss-islington (miss-islington) Date: 2018-08-16 07:29
New changeset bf8e9d18dd75f58ce3b9761763ae10c0800b43d8 by Miss Islington (bot) in branch '3.7':
bpo-34217: Use lowercase for windows headers (GH-8472)
https://github.com/python/cpython/commit/bf8e9d18dd75f58ce3b9761763ae10c0800b43d8
msg323602 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2018-08-16 15:12
> Stop relying to case insensitive filesystem makes sense to me.

If this is the case, then we need to normalise the casing to match the actual files (which are mixed-case, not lowercase). You just broke this with your PR :)

So which is it? Do we match the casing of the real files? Or do we change to match someone's unofficial versions of the files?
msg323603 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2018-08-16 15:13
> whatever might be the 'right' solution, it's always an advantage to
> have a consistent codebase.

Not when you are consistently wrong, it's not.

Let's figure out the right answer here before making many changes to the codebase. First, can you tell me where you got your lowercased named files from? Because it wasn't the official SDK...
msg323619 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2018-08-16 19:53
> New windows will support case sensitive filesystem too.

NTFS in Windows 10 supports a flag to mark a directory as case sensitive [*] via NtSetInformationFile: FileCaseSensitiveInformation, which can be set from the command line via fsutil.exe. In recent builds this flag was made inheritable by child directories, so it doesn't have to be set individually on every directory in a tree. This feature was added for inter-operation with (WSL) Linux programs. It's not intended for system directories such as "Windows Kits".

> can you tell me where you got your lowercased named files from?

IIRC, MinGW lower-cases the filenames in its modified version of the SDK.

---

[*] Prior to Windows XP, case-sensitive file access was possible via FILE_FLAG_POSIX_SEMANTICS (CreateFile) and FIND_FIRST_EX_CASE_SENSITIVE (FindFirstFileEx). Windows XP (the first consumer version of NT) broke those flags by adding a (default-enabled) global flag in the kernel object manager that makes all device and file access case-insensitive -- regardless of the system call's OBJ_CASE_INSENSITIVE flag. Now in Windows 10 they're bringing back selective case sensitivity in NTFS, but this time using a per-directory flag that overrides the API. It seems they don't care that this breaks the existing OBJ_CASE_INSENSITIVE flag in the NT API. IMO, this could have been handled more cleanly by combining the per-directory flag with one or two new API flags.
msg323621 - (view) Author: Erik Janssens (erikjanss) * Date: 2018-08-16 21:31
The lowercase headers are indeed part of the mingw-w64 SDK, which is the most convenient option to cross compile on a linux host for a windows target.
msg323622 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2018-08-16 21:48
Have you posted to their mailing list to ask why they use different casing from the official SDK and whether they would consider supporting both (via symlink, presumably)?

I think it would be strange for CPython to add a style rule for a non-supported toolset. The supported tooling will default to using the correct case for the header files.
msg323625 - (view) Author: Erik Janssens (erikjanss) * Date: 2018-08-16 22:20
As I understand it, if mingw-w64 would change to camelcase headers, it would break backwards compatibility, since a lot of windows source code uses file names in includes that differ from those on this, which works in MSVC, so it goes unnoticed.  There is no good solution.

Even if camelcase headers would be available, the CPython source code would break in much more places.

I agree that it would be strange to have a style rule for a specific toolset, however mixing different styles is even stranger.

The current situation is that 100% of the windows includes are lowercase and allow cross compilation.  I would tend to see that as an advantage.

If a more comprehensive solution would be available, that would be ideal of course.
msg323629 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2018-08-16 22:59
> The current situation is that 100% of the windows includes are lowercase and allow cross compilation.

Please be precise with what you are saying - the "current situation" is only so because you proposed changes and got them merged without proper discussion, and "allow cross compilation" with only one alternate toolset. Pretending this is the status quo is dishonest, so please don't insult me like that.

> if mingw-w64 would change to camelcase headers, it would break backwards compatibility

I'm not recommending camel case - I'm recommending matching the casing used for each file in the official SDK (which I agree is a weird mix, but there are certainly compatibility reasons involved in keeping them this way). And yes, it would be a compatibility break for the clones of the SDK, but to a large extent that is their fault for choosing different casing in the first place, so I'm only a little bit sympathetic to that argument.


What I'm mostly opposed to is the very weak bug report and the random selection of changes that came with it. Failing to compile on an unsupported toolset is not an immediate "must-fix" issue, and it really ought to go through the affected platform experts so they can confirm acceptable impact on developers who are using the supported tools. There also needs to be this central point so that, assuming we decide to keep them this way, the next person who comes in and complains that the casing doesn't match the actual files is given the correct explanation - otherwise we'll keep switching them back and forth forever.

You're also likely to face a maintenance burden for this change, since there is currently no rule or validation that will prevent people from adding properly cased includes in the future (as I hinted, most IDEs on Windows will autocorrect filename casing for headers). If you want one, propose a change to PEP 7 on python-dev, and if it's approved then you can add a build step to validate. Otherwise, you'll have to track changes that are made and fix them as necessary. Without an explicit rule, our default position is "whatever the native target platform/tools prefer".

I hope that explains the position a bit better, and why I push back against changes with insufficient justification being provided. At this point, I'm not going to revert these particular changes, as the cases where they will affect native developers are startlingly few these days, but I'm also explicitly saying that this does not mean open-season for all the fixes required for mingw to be happy. Each one will be taken on its merits (primarily compatibility and maintainability), at least until someone has committed to fully support the toolset.
msg323693 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-08-18 05:43
I googled and checked what MSDN uses before I merged these PRs.
For example: https://msdn.microsoft.com/ja-jp/library/bb394814.aspx
msg323701 - (view) Author: Erik Janssens (erikjanss) * Date: 2018-08-18 10:59
I'll try to be more precise :

- I did an (imperfect) search : before these changes 98% of the windows headers were included in the .c files in lowercase

- These changes would bring it to 100%

- The advantage of these changes are consistency and the ability to cross compile those .c files on linux for a windows target, using the mingw-w64 sdk, where the sdk is on a case sensitive filesystem, as is the default when installing this sdk in most linux distributions.

- I did not consider, nor did I test other toolchains than mingw-w64/mingw-w64-sdk on linux and msvc/windows-sdk on windows.

I'm not enough of an expert on the different tools to come up with a comprehensive solution for this issue, but am willing to validate such solution for the use case of cross compiling on linux.
msg323713 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2018-08-18 14:01
As someone who has written MSDN samples, I'll spoil the secret and let you know that there's no consistency there, so you can find examples to prove either direction ;)

I wasn't expecting expertise or research. I was only expecting a description of the change that clearly specifies who benefits and who loses, and leaves a chance for the affected experts to confirm impact before being merged. There can be a surprising amount of subtlety around cross-platform issues, which is why we have designated people who are responsible for tracking them.
msg323781 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-08-20 09:01
I'm sorry about that.  I won't merge PR like that next time.
History
Date User Action Args
2018-09-06 21:31:24erikjansssetstatus: open -> closed
stage: patch review -> resolved
2018-08-20 09:01:03methanesetmessages: + msg323781
2018-08-18 14:01:48steve.dowersetmessages: + msg323713
2018-08-18 10:59:26erikjansssetmessages: + msg323701
2018-08-18 05:43:39methanesetmessages: + msg323693
2018-08-16 22:59:09steve.dowersetmessages: + msg323629
2018-08-16 22:20:11erikjansssetmessages: + msg323625
2018-08-16 21:48:12steve.dowersetmessages: + msg323622
2018-08-16 21:31:56erikjansssetmessages: + msg323621
2018-08-16 19:53:47eryksunsetnosy: + eryksun
messages: + msg323619
2018-08-16 15:13:08steve.dowersetmessages: + msg323603
2018-08-16 15:12:04steve.dowersetmessages: + msg323602
2018-08-16 07:29:24miss-islingtonsetmessages: + msg323597
2018-08-16 07:28:30methanesetmessages: + msg323596
2018-08-16 06:49:43erikjansssetmessages: + msg323595
2018-08-16 06:41:27miss-islingtonsetstage: resolved -> patch review
pull_requests: + pull_request8255
2018-08-16 06:41:00methanesetmessages: + msg323594
2018-08-15 23:12:00steve.dowersetmessages: + msg323586
2018-08-15 19:14:32erikjansssetstatus: closed -> open
files: + windows_headers.py
messages: + msg323582
2018-07-30 06:15:54miss-islingtonsetnosy: + miss-islington
messages: + msg322655
2018-07-30 06:03:06miss-islingtonsetpull_requests: + pull_request8074
2018-07-29 09:03:39erikjansssetpull_requests: + pull_request8058
2018-07-25 00:42:08methanesetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-07-25 00:41:50methanesetnosy: + methane
messages: + msg322335
2018-07-24 23:47:37erikjansssetkeywords: + patch
stage: patch review
pull_requests: + pull_request7978
2018-07-24 23:42:09erikjansscreate