Skip to content

Python 3.11 loses the ability to set PYTHON_DECIMAL_WITH_MACHINE #98557

Open
Open
Python 3.11 loses the ability to set PYTHON_DECIMAL_WITH_MACHINE#98557
Labels
buildThe build process and cross-buildtype-bugAn unexpected behavior, bug, or error

Description

@Bo98
Bo98
opened on Oct 22, 2022

Bug report

Python 3.10 had the ability to set PYTHON_DECIMAL_WITH_MACHINE to override the choice of configuration for the _decimal module:

cpython/setup.py

Lines 2388 to 2397 in dcb342b

machine = os.environ.get('PYTHON_DECIMAL_WITH_MACHINE')
if machine:
# Override automatic configuration to facilitate testing.
define_macros = config[machine]
elif MACOS:
# Universal here means: build with the same options Python
# was built with.
define_macros = config['universal']
elif sizeof_size_t == 8:

Since Python 3.11, this is no longer possible. This feature was necessary, on macOS particularly, with the --with-system-libmpdec option if that system libmpdec is configured differently to the default Python config. On macOS, the default Python config forces universal, while setting PYTHON_DECIMAL_WITH_MACHINE allowed it to be single-arch.

libmpdec produces different headers depending on how it was built, which is why the setting is important to be able to override. Without it, the _decimal module will fail to compile if the default does not match how system libmpdec was built.

Homebrew's Python currently depends on this feature.

A test within CPython also seems to depend on this feature:

cpython/Modules/_decimal/tests/runall-memorydebugger.sh

Lines 63 to 79 in f4c0348

unset PYTHON_DECIMAL_WITH_MACHINE
libmpdec_config=$config
if [ X"$config" != X"auto" ]; then
PYTHON_DECIMAL_WITH_MACHINE=$config
export PYTHON_DECIMAL_WITH_MACHINE
else
libmpdec_config=""
fi
############ refleak tests ###########
print_config "refleak tests: config=$config" $args
printf "\nbuilding python ...\n\n"
cd ../../
$GMAKE distclean > /dev/null 2>&1
./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" --with-pydebug $args > /dev/null 2>&1
$GMAKE | grep _decimal

Your environment

  • CPython versions tested on: 3.11.0rc2
  • Operating system and architecture: macOS 12 (x86_64 and arm64)

Activity

  1. Bo98
    added
    type-bugAn unexpected behavior, bug, or error
    on Oct 22, 2022

  2. AlexWaygood commented on Oct 24, 2022

    @AlexWaygood
    AlexWaygood
    on Oct 24, 2022
    Member

    Cc. @rhettinger for decimal module expertise.

  3. erlend-aasland commented on Oct 24, 2022

    @erlend-aasland
    erlend-aasland
    on Oct 24, 2022
    Contributor

    FTR, this was removed in GH-29541 (cc. @tiran as PR author and @mdickinson as PR reviewer)

  4. erlend-aasland commented on Oct 24, 2022

    @erlend-aasland
    erlend-aasland
    on Oct 24, 2022
    Contributor

    AFAICS from GH-29541, it should be possible to set PYTHON_DECIMAL_WITH_MACHINE via CFLAGS (for example LIBMPDEC_CFLAGS). @Bo98, can you please test this?

  5. erlend-aasland
    added
    buildThe build process and cross-build
    on Oct 24, 2022

  6. Bo98 commented on Oct 24, 2022

    @Bo98
    Bo98
    on Oct 24, 2022
    ContributorAuthor

    What reads PYTHON_DECIMAL_WITH_MACHINE? It's not a define to be passed to C but something that used to change what item is selected from:

    cpython/configure.ac

    Lines 3882 to 3891 in 75a6fad

    AS_CASE([$libmpdec_machine],
    [x64], [AS_VAR_APPEND([LIBMPDEC_CFLAGS], [" -DCONFIG_64=1 -DASM=1"])],
    [uint128], [AS_VAR_APPEND([LIBMPDEC_CFLAGS], [" -DCONFIG_64=1 -DANSI=1 -DHAVE_UINT128_T=1"])],
    [ansi64], [AS_VAR_APPEND([LIBMPDEC_CFLAGS], [" -DCONFIG_64=1 -DANSI=1"])],
    [ppro], [AS_VAR_APPEND([LIBMPDEC_CFLAGS], [" -DCONFIG_32=1 -DANSI=1 -DASM=1 -Wno-unknown-pragmas"])],
    [ansi32], [AS_VAR_APPEND([LIBMPDEC_CFLAGS], [" -DCONFIG_32=1 -DANSI=1"])],
    [ansi-legacy], [AS_VAR_APPEND([LIBMPDEC_CFLAGS], [" -DCONFIG_32=1 -DANSI=1 -DLEGACY_COMPILER=1"])],
    [universal], [AS_VAR_APPEND([LIBMPDEC_CFLAGS], [" -DUNIVERSAL=1"])],
    [AC_MSG_ERROR([_decimal: unsupported architecture])]
    )

    Unless you suggest that I instead pass LIBMPDEC_CFLAGS=-DCONFIG_64=1 -DASM=1 on x86_64 and LIBMPDEC_CFLAGS=-DCONFIG_64=1 -DANSI=1 -DHAVE_UINT128_T=1 on arm64? (Assuming this list is never changing.)

  7. erlend-aasland commented on Oct 24, 2022

    @erlend-aasland
    erlend-aasland
    on Oct 24, 2022
    Contributor

    Ah, sorry, you are right. I misread the diff from GH-29541; it is used as an index, not a CFLAG.

  8. erlend-aasland commented on Oct 24, 2022

    @erlend-aasland
    erlend-aasland
    on Oct 24, 2022
    Contributor

    Yes, it seems this ability is now lost; there is no way to manually override that AS_CASE. We'd either have to reintroduce PYTHON_DECIMAL_WITH_MACHINE, or provide a new configure switch for this, if we were to reintroduce this.

  9. erlend-aasland commented on Oct 24, 2022

    @erlend-aasland
    erlend-aasland
    on Oct 24, 2022
    Contributor

    You mentioned Homebrew? Does this prevent Homebrew from building and distributing Python 3.11? Is there a Homebrew ticket for this?

  10. erlend-aasland commented on Oct 24, 2022

    @erlend-aasland
    erlend-aasland
    on Oct 24, 2022
    Contributor

    From #98557 (comment):

    Unless you suggest that I instead pass LIBMPDEC_CFLAGS=-DCONFIG_64=1 -DASM=1 on x86_64 and LIBMPDEC_CFLAGS=-DCONFIG_64=1 -DANSI=1 -DHAVE_UINT128_T=1 on arm64? (Assuming this list is never changing.)

    Then from the OP:

    libmpdec produces different headers depending on how it was built, which is why the setting is important to be able to override. Without it, the _decimal module will fail to compile if the default does not match how system libmpdec was built.

    I suggest you use LIBMPDEC_CFLAGS to set these flags depending on how your libmpdec was built. That seems to me to be a more robust approach than letting configure chose flags which might not reflect how your libmpdec was built.

    ISTM we don't need a way to override the libmpdec_machine switch in configure.

  11. Bo98 commented on Oct 24, 2022

    @Bo98
    Bo98
    on Oct 24, 2022
    ContributorAuthor

    Is there a Homebrew ticket for this?

    Not yet considering we don't ship betas, but what we'll probably do for 3.11.0 (and what I've done in a local test rc2 build) is a s/libmpdec_machine=universal/libmpdec_machine=x64/ (uint128 on arm64) on configure. Not a desirable long-term solution but should be ok for a short-term fix.

    I suggest you use LIBMPDEC_CFLAGS to set these flags depending on how your libmpdec was built. That seems to me to be a more robust approach than letting configure chose flags which might not reflect how your libmpdec was built.

    PYTHON_DECIMAL_WITH_MACHINE did directly match the option you would pass to libmpdec configure. If you passed MACHINE=x64 to libmpdec while building then you would pass the same to Python.

    It made sense and is what Stefan Krah recommended we did.

    Might I propose an alternative way forward here however. I propose looking into not setting any CFLAGS for the system libmpdec and checking MPD_CONFIG_64 and MPD_CONFIG_32 in _decimal source code, given mpdecimal.h should tell you how it was built by setting those.

    FWIW, this is what the relevent section of mpdecimal.h looks like on a different configurations of system libmpdec:

    Universal 
    /* Mac OS X: support for building universal binaries */
#if defined(MPD_CONFIG_64) || defined(MPD_CONFIG_32)
  #error "cannot use MPD_CONFIG_64 or MPD_CONFIG_32 with universal header."
#endif

#if defined(CONFIG_64) || defined(CONFIG_32)
  #error "cannot use CONFIG_64 or CONFIG_32 with universal header."
#endif

#if defined(__ppc__)
  #define MPD_CONFIG_32 1
  #ifdef UNIVERSAL
    #define CONFIG_32
    #define ANSI
  #endif
#elif defined(__ppc64__)
  #define MPD_CONFIG_64 1
  #ifdef UNIVERSAL
    #define CONFIG_64
    #define ANSI
  #endif
#elif defined(__i386__)
  #define MPD_CONFIG_32 1
  #ifdef UNIVERSAL
    #define CONFIG_32
    #define ANSI
  #endif
#elif defined(__x86_64__)
  #define MPD_CONFIG_64 1
  #ifdef UNIVERSAL
    #define CONFIG_64
    #define ASM
  #endif
#elif defined(__arm64__)
  #define MPD_CONFIG_64 1
  #ifdef UNIVERSAL
    #define CONFIG_64
    #define ANSI
  #endif
#else
  #error "unknown architecture for universal build."
#endif
    x64 and uint128 
    /* ABI: 64-bit */
#define MPD_CONFIG_64 1

#ifdef MPD_CONFIG_32
  #error "cannot use MPD_CONFIG_32 with 64-bit header."
#endif

#ifdef CONFIG_32
  #error "cannot use CONFIG_32 with 64-bit header."
#endif

  12. erlend-aasland commented on Oct 24, 2022

    @erlend-aasland
    erlend-aasland
    on Oct 24, 2022
    Contributor

    Might I propose an alternative way forward here however. I propose looking into not setting any CFLAGS for the system libmpdec and checking MPD_CONFIG_64 and MPD_CONFIG_32 in _decimal source code, given mpdecimal.h should tell you how it was built by setting those.

    That sounds like a better way forward indeed. I've already pinged Christian on this issue; let's wait and see if he also agrees.

  13. carlocab commented on Oct 4, 2024

    @carlocab
    carlocab
    on Oct 4, 2024

    It's been a while here. Can we find a way to move this forward?

  14. erlend-aasland commented on Oct 14, 2024

    @erlend-aasland
    erlend-aasland
    on Oct 14, 2024
    Contributor

    It's been a while here. Can we find a way to move this forward?

    For 3.11? Unfortunately not; 3.11 only received security fixes. For 3.12 and newer, the environment variables LIBMPDEC_CFLAGS and LIBMPDEC_LIBS are available if you want to customise how _decimal is built.

    See also #115119.

  15. Bo98 commented on Oct 14, 2024

    @Bo98
    Bo98
    on Oct 14, 2024
    ContributorAuthor

    Looks like #115406 might have fixed this by moving the assumptions into a AS_VAR_IF([with_system_libmpdec], [no] in 3.13. Specifically, the flag and decimal.c changes rather than the actual pkg-config part, so the fix works for pre-4.0 too.

    For 3.11 and 3.12 it was previously unfixable without patching the configure file (LIBMPDEC_CFLAGS won't override it), unless a backport of that new patch is applied.

  16. erlend-aasland commented on Oct 14, 2024

    @erlend-aasland
    erlend-aasland
    on Oct 14, 2024
    Contributor

    Right, so my gut feel would be that a backport of #115406 probably won't happen; such changes to the configure script are always slight behavioural changes to the build system, so backporting them may create havoc for distros.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    buildThe build process and cross-buildtype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions