Would it be possible to modify expat_config.h instead of modifying code copied from libexpat?

The AppVeyor failure is unrelated to your change, it's a regression that I introduced when I made threading_cleanup() stricter: https://bugs.python.org/issue31234#msg302163

We could fix it in configure.ac by replacing AC_DEFINE for the macro to:

AH_VERBATIM([_POSIX_C_SOURCE],
[/* Define to activate features from IEEE Stds 1003.1-2008 */
#ifdef _POSIX_C_SOURCE
#undef _POSIX_C_SOURCE
#endif
#define _POSIX_C_SOURCE 200809L
])

Hum, I suggest to not define _POSIX_C_SOURCE if it's already set, rather than replacing the value:

#ifdef _POSIX_C_SOURCE
#  define _POSIX_C_SOURCE 200809L
#endif

Do we really need pyconfig.h to configure expat?

We use pyconfig.h to get endianess.

I tried to generate expat_config.h from a expat_config.h.in template which doesn't include pyconfig.h.

Problem: Expat requires the endian, but also the HAVE_MEMMOVE define. I don't know autotools good enough to propose a working change.

I tried the following code in configure.ac:

# Define a BYTEORDER variable for Moules/expat/expat_config.h.in
AC_SUBST(BYTEORDER)
AC_C_BIGENDIAN(
  BYTEORDER=4321,
  BYTEORDER=1234,
  AC_MSG_ERROR(unknown endianess),
  AC_MSG_ERROR(universial endianess not supported)
)

but then noticed that autoheader genereates more complex code in pyconfig.h for the endian:

/* Define WORDS_BIGENDIAN to 1 if your processor stores words with the most
   significant byte first (like Motorola and SPARC, unlike Intel). */
#if defined AC_APPLE_UNIVERSAL_BUILD
# if defined __BIG_ENDIAN__
#  define WORDS_BIGENDIAN 1
# endif
#else
# ifndef WORDS_BIGENDIAN
#  undef WORDS_BIGENDIAN
# endif
#endif

Oh wow, I didn't know that getting the endian would be so complex.

I don't know how to pass HAVE_MEMMOVE from configure.ac to my Modules/expat/expat_config.h.in.

Well, as @tiran told me on IRC, my solution is maybe too complex just to fix a single compiler warning :-)

--

But I dislike modifying pyconfig.h because portability is also complex, and I don't know what are the consequences. I proposed a different fix which leaves pyconfig.h unchanged: PR #3601. It also documents what are get from pyconfig.h.

@tiran: What do you think?

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I didn't expect the Spanish Inquisition!. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

I didn't expect the French Inquisition!

And I didn't expect the Spanish Inquisition!

Nobody expects the Spanish Inquisition!

@Haypo: please review the changes made to this pull request.

I tested this PR when HAVE_MEMMOVE is false (undefined): expat compilation fails because HAVE_BCOPY is unset, whereas my Fedora 29 supports bcopy() :-/

libexpat uses more "HAVE_xxx" macros:

HAVE_ARC4RANDOM
HAVE_ARC4RANDOM_BUF
HAVE_BCOPY
HAVE_EXPAT_CONFIG_H
HAVE_GETRANDOM
HAVE_LIBBSD
HAVE_SYSCALL_GETRANDOM

expat_config.h seems like the best approach, but I'm not longer sure that it's worth it. This approach requires a lot of work, whereas most platforms use system libexpat, not our vendored libexpat copy.

This approach seems too complex :-( Since we are trying to fix a minor warning, I wrote a simpler patch which modifies the vendored copy of libexpat: PR #11064.

I merged a simpler approach: PR #11064.