Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup and simplify setup.py #89906

Closed
tiran opened this issue Nov 7, 2021 · 21 comments
Closed

Cleanup and simplify setup.py #89906

tiran opened this issue Nov 7, 2021 · 21 comments
Assignees
Labels
3.11 only security fixes build The build process and cross-build OS-mac type-feature A feature request or enhancement

Comments

@tiran
Copy link
Member

tiran commented Nov 7, 2021

BPO 45743
Nosy @gpshead, @ronaldoussoren, @tiran, @ned-deily, @erlend-aasland
PRs
  • bpo-45743: Move __APPLE_USE_RFC_3542 into socketmodule.c (GH-29456) #29456
  • bpo-45743: Move __APPLE_USE_RFC_3542 into socketmodule.c (GH-29456) #29456
  • bpo-45743: Remove workaround for zlib CVE from 2002 (GH-29457) #29457
  • bpo-45743: -Wl,-search_paths_first is no longer needed (GH-29464) #29464
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2021-11-07.16:37:24.260>
    labels = ['OS-mac', 'type-feature', 'build', '3.11']
    title = 'Cleanup and simplify setup.py'
    updated_at = <Date 2021-11-09.08:56:25.176>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2021-11-09.08:56:25.176>
    actor = 'christian.heimes'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Build', 'macOS']
    creation = <Date 2021-11-07.16:37:24.260>
    creator = 'christian.heimes'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 45743
    keywords = ['patch', 'patch']
    message_count = 10.0
    messages = ['405908', '405912', '405914', '405915', '405942', '405943', '405944', '405998', '406002', '406006']
    nosy_count = 5.0
    nosy_names = ['gregory.p.smith', 'ronaldoussoren', 'christian.heimes', 'ned.deily', 'erlendaasland']
    pr_nums = ['29456', '29456', '29457', '29464']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue45743'
    versions = ['Python 3.11']

    @tiran
    Copy link
    Member Author

    tiran commented Nov 7, 2021

    Motivated by deprecation of distutils, I like to move more logic and checks from setup.py into configure.ac. Eventually I like to get rid of setup.py. The file contains a bunch of complicated checks and macOS-specific adjustments that I cannot verify on Linux.

    1. socketmodule setup defines __APPLE_USE_RFC_3542 https://github.com/python/cpython/blob/v3.11.0a2/setup.py#L1229 Can we move the define into the __APPLE__ block of socketmodule.c?

    2. -Wl,-search_paths_first linker arg, e.g. https://github.com/python/cpython/blob/v3.11.0a2/setup.py#L1616 Would it be safe to make the option default for all core extensions on $ac_sys_system = Darwin? We could add it to PY_CORE_LDFLAGS or add a new Makefile variable for core extensions.

    3. detect_dbm_gdbm has about 200 lines of complicated code to detect old to ancient versions of Berkeley DB (libdb), https://github.com/python/cpython/blob/v3.11.0a2/setup.py#L1233 . Can I remove support for libdb-3 / libdb-4 and only support libdb-5 in standard locations? libdb-5.3 has been around for over a decade.

    Note: libdb, gdbm, ndbm, and gdbm-compat don't provide pkg-config .pc files. We cannot use pkg-config to detect them.

    1. sqlite's setup https://github.com/python/cpython/blob/v3.11.0a2/setup.py#L1531 does extra work to search for header and library files in non-standard locations. Can we replace the code with pkg-config checks in configure.ac with a fallback to AC_CHECK_LIB() and AC_CHECK_HEADERS()?

    2. zlib's setup code https://github.com/python/cpython/blob/v3.11.0a2/setup.py#L1661 has a check for a CVE from 2002 (!). I think we can safely assume that everybody has upgraded to a fixed version. The check is mixed with macOS specific code.

    @tiran tiran added 3.11 only security fixes build The build process and cross-build OS-mac type-feature A feature request or enhancement labels Nov 7, 2021
    @ned-deily
    Copy link
    Member

    It seems to me that some (most?) of the macOS-specific workarounds in setup.py were added to make it easy to build with the system-provided copies of the third-party libraries, like zlib and sqlite3 and openssl. At least one of the workarounds, search_paths_first, became the default ld behavior since around macOS 10.6 (Xcode 4). So when building on more recent macOS systems, either the workarounds aren't necessary any more or the third-party library is no longer provided (like openssl) or is otherwise too old (like Tk) so that you can no longer rely on building with just using system-supplied libraries to have a useful Python on macOS. And, if you are building on very old systems (because of hardware requirements or whatever), in most cases, the versions of the system-supplied libraries are so old that you shouldn't be relying on them anyway. So I think that, for 3.11, we could take a stand and say that we no longer support building with specific system-supplied third-party libraries on macOS systems older than, say, macOS 10.9; IOW, you will need to supply your own copies of those libs on older systems. That's essentially the approach the MacPorts project has taken for years; they still support providing current Pythons on old versions of macOS but they also supply current version of the third-party libs, too.

    And Christian's work here for 3.11 to move away from setup.py and use pkg-config to remove the guesswork on header and lib locations should make life easier for everyone. There is a small issue here in that macOS does not supply a built-in pkg-config but there are implementations available, if you are unable or unwilling to use open source distributors like MacPorts or Homebrew, including an actively-maintained permissive-licensed version with no build dependencies that seems to work on even macOS 10.6 (I didn't try anything older than that!): https://github.com/pkgconf/pkgconf.

    Ronald, what do you think? If this sounds reasonable, we could draw up a list of libs and document it somewhere in the 3.11 code.

    @tiran
    Copy link
    Member Author

    tiran commented Nov 7, 2021

    Thanks for the detailed explanation, Ned. Much appreciated! For non-Mac users: macOS 10.6 was released in 2009 and 10.9 in 2013. IMHO it is reasonable to ask people to provide their own copies of libraries on older system. We also require OpenSSL 1.1.1, which is not available on older systems like CentOS 7 (released 2014).

    Regarding pkg-config, I like to tidy up setup.py first to get a better understanding what kind of builds and systems I need to support. Thanks for pointing out https://github.com/pkgconf/pkgconf! I keep it in mind.

    @tiran
    Copy link
    Member Author

    tiran commented Nov 7, 2021

    For reference: https://opensource.apple.com/source/ld64/ld64-242/doc/man/man1/ld.1.auto.html

    search_paths_first
    This is now the default (in Xcode4 tools). When processing -lx the linker now searches each directory
    in its library search paths for libx.dylib' then libx.a' before the moving on to the next path
    in the library search path.

    According to Wikipedia, Xcode4 was released in 2011.

    @tiran
    Copy link
    Member Author

    tiran commented Nov 8, 2021

    New changeset 24af9a4 by Christian Heimes in branch 'main':
    bpo-45743: Move __APPLE_USE_RFC_3542 into socketmodule.c (GH-29456)
    24af9a4

    @ronaldoussoren
    Copy link
    Contributor

    1. __APPLE_USE_RFC_3542 should have been in socketmodule.c from the start, not sure why it was added in setup.py.

    2. as you and Ned noticed -search_paths_first has been the default for a long while, we can just drop it and anyone building on ancient systems can add the flag to CFLAGs/LDFLAGS as needed. IIRC we added the flag to be able to build with local copies of libraries also shipped in the OS, in particular when using static libraries for those local copies.

    3. I don't know about detect_dbm_gdbm, and don't use those libraries myself. It would be nice to be able to open system files created using dbopen etc, but on the other hand there is at least one bug report complaining about data corruption when using one o the dbm extensions linked with the system version of the library.

    4/5) Fine by me, although I'm slightly worried about using pkg-config because the system doesn't ship that tool.

    Something you don't mention is the logic dealing with SDK roots. I haven't checked yet if similar logic would be necessary in configure. With some luck it isn't, but that depends on what's supported by autoconf and the particular probes we want to use.

    @ned: Not being able to use system versions of libraries is a bit annoying, but that's something you already have to do due to openssl. Maybe we should also try to clean up and refactor the build-installer.py script to do have a way to build/install those 3th-party dependencies.

    @erlend-aasland
    Copy link
    Contributor

    1. __APPLE_USE_RFC_3542 should have been in socketmodule.c from the start, not sure why it was added in setup.py.

    FTR, it was added by me in bpo-35569, #63725. I moved the check to setup.py because Ned made me do it :P #19526 (comment)

    @ned-deily
    Copy link
    Member

    > 1) __APPLE_USE_RFC_3542 should have been in socketmodule.c from the start, not sure why it was added in setup.py.

    FTR, it was added by me in bpo-35569, #63725. I moved the check to setup.py because Ned made me do it :P

    Yeah, well, it seemed like a good thing at the time :)

    Something you don't mention is the logic dealing with SDK roots. I haven't checked yet if similar logic would be necessary in configure. With some luck it isn't, but that depends on what's supported by autoconf and the particular probes we want to use.

    I haven't looked closely at it in this context but I believe most, if not all, of the SDK root shenanigans in setup.py shouldn't be necessary in autoconf tests where the Apple compiler driver handles the SDK details and the few other cases, like for when there was only a 32-bit version of Tk for macOS, are no longer relevant. In any case, that's something for us to follow up on once Christian's changes are merged and stabilized on other platforms.

    @ned: Not being able to use system versions of libraries is a bit annoying, but that's something you already have to do due to openssl. Maybe we should also try to clean up and refactor the build-installer.py script to do have a way to build/install those 3th-party dependencies.

    Yes. I want to pull the third-party lib building out of build-installer.py into a simple separate build step, probably just using make, that can produce the minimum sets of third-party libs for use by both installer builds and by developers so that there is no dependency on other third-party distributors like Homebrew or MacPorts. With a little bit of care, it wouldn't have to be limited to just macOS builds.

    @tiran
    Copy link
    Member Author

    tiran commented Nov 9, 2021

    New changeset 8fefaad by Christian Heimes in branch 'main':
    bpo-45743: -Wl,-search_paths_first is no longer needed (GH-29464)
    8fefaad

    @tiran
    Copy link
    Member Author

    tiran commented Nov 9, 2021

    New changeset 6a1cc8b by Christian Heimes in branch 'main':
    bpo-45743: Remove workaround for zlib CVE from 2002 (GH-29457)
    6a1cc8b

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @kumaraditya303
    Copy link
    Contributor

    Closing as it seems fixed, feel free to reopen if required.

    @tiran
    Copy link
    Member Author

    tiran commented Jul 23, 2022

    You should not close tickets that affects supported versions of Python without seeking consent from the ticket author first, especially when the ticket author is a triager or core dev. 3.11 is still affected.

    @tiran tiran reopened this Jul 23, 2022
    @kumaraditya303
    Copy link
    Contributor

    Okay, but I did say that "feel free to reopen if required." and there was no activity in the last 6 months or so and setup.py is now removed.

    @tiran
    Copy link
    Member Author

    tiran commented Jul 23, 2022

    It's not removed from 3.11.

    @kumaraditya303
    Copy link
    Contributor

    kumaraditya303 commented Jul 23, 2022

    You should not close tickets that affects supported versions of Python without seeking consent from the ticket author first,

    This issue is marked as type-feature and we don't usually backport enhancements after beta, only bug fixes. Do you consider this as a bug? If yes would you mind labelling it as a bug?

    @erlend-aasland
    Copy link
    Contributor

    With beta 5 around the corner and the release candidates appearing in near future, I believe it would be wise to just leave setup.py as it is for 3.11. What do you say, Christian?

    @tiran
    Copy link
    Member Author

    tiran commented Jul 25, 2022

    There is still some documentation work and checks to do for 3.11.

    @CAM-Gerlach
    Copy link
    Member

    @tiran Do you still intend to do additional work on this for 3.11, given setup.py is completely gone in 3.12?

    @erlend-aasland erlend-aasland added the pending The issue will be closed if no feedback is provided label Jan 22, 2023
    @erlend-aasland erlend-aasland removed the pending The issue will be closed if no feedback is provided label Jun 19, 2023
    @arhadthedev
    Copy link
    Member

    1. detect_dbm_gdbm has about 200 lines of complicated code to detect old to ancient versions of Berkeley DB (libdb), https://github.com/python/cpython/blob/v3.11.0a2/setup.py#L1233 . Can I remove support for libdb-3 / libdb-4 and only support libdb-5 in standard locations? libdb-5.3 has been around for over a decade.

    Note: libdb, gdbm, ndbm, and gdbm-compat don't provide pkg-config .pc files. We cannot use pkg-config to detect them.

    1. sqlite's setup https://github.com/python/cpython/blob/v3.11.0a2/setup.py#L1531 does extra work to search for header and library files in non-standard locations. Can we replace the code with pkg-config checks in configure.ac with a fallback to AC_CHECK_LIB() and AC_CHECK_HEADERS()?

    @erlend-aasland For issue archeology of the future, could you link to PRs that resolve these points or explain why they can be abandoned, please?

    @erlend-aasland
    Copy link
    Contributor

    @arhadthedev: see gh-90005

    @erlend-aasland
    Copy link
    Contributor

    @arhadthedev: for sqlite3: gh-89932

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes build The build process and cross-build OS-mac type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants