Removing WIN32 define breaks Windows builds

[crazydef]

I updated to the latest code.

I expected everything to still work.

curl/libcurl version 7.69.0-DEV

operating system: Windows 10

The commit that removes the WIN32 define didn't also remove all references to WIN32 in other places, so now nothing builds on Windows.

I understand the desire to remove WIN32, but existing usage of that macro should have been changed to use the compiler defined _WIN32 at the same time. As things now stand, this non-standard macro needs to be declared externally when building any project that includes curl.h on Windows.

[bagder]

now nothing builds on Windows.

I'm afraid that's wrong. All of our CI builds on Windows still build perfectly. You need to be much more specific and explain exactly what doesn't build and preferably why.

WIN32 should not be defined by curl headers.

[crazydef]

Your CI builds probably still build because your toolchain (cmake, by any chance?) defines WIN32 itself. Other build systems (like Visual Studio, for example) do not. Any project which includes curl.h is now forced to define WIN32 on Windows.

[crazydef]

I agree WIN32 shouldn't be defined by Curl headers. In the same way I agree that the Curl headers shouldn't try to rely on non-standard, user-defined macros to detect the platform.

[bagder]

Other build systems (like Visual Studio, for example) do not

You mean using the files under projects/Windows ? Then we should fix those builds.

In the same way I agree that the Curl headers shouldn't try to rely on non-standard, user-defined macros to detect the platform.

Feel free to suggest and provide improvements to our headers and other code. We only do the best we can and there is always room for improvements.

[crazydef]

No, I mean projects that use curl. Not any code in the curl directory. (Although that might be affected too.)

I'm not talking about building curl, I'm talking about including curl.h in my projects.

[bagder]

Why should curl's headers be responsible for identifying win32 for your projects?

[crazydef]

Because curl requires WIN32 to be declared. It used to declare it itself if _WIN32 was declared, now it assumes it's declared externally. Curl shouldn't require the user to define the platform. That's the job of compiler writers and OS vendors. Curl should be using _WIN32 (or whatever) to detect the platform. I.e, the standard platform preprocessor definitions.

As for a recommendation, In system.h I would add CURL_WIN32, CURL_MACOS, etc platform definitions and use those throughout the rest of the curl source code.

[bagder]

Le me try to decipher this.

You're saying that curl uses preprocessor logic in its public header files that relies on WIN32 being defined when it is in fact not usually defined by Windows compilers?

[MarcelRaad]

@bagder

Why should curl's headers be responsible for identifying win32 for your projects?

It checks for the removed define throughout curl.h, e.g.

curl/include/curl/curl.h

Line 52 in 1adebe7

#if defined(WIN32) && !defined(_WIN32_WCE) && !defined(__CYGWIN__)

[crazydef]

Yes, that's what I'm saying. :)