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

Update PC/pyconfig.h to support disabling auto linking #82909

Open
jcfr mannequin opened this issue Nov 6, 2019 · 6 comments
Open

Update PC/pyconfig.h to support disabling auto linking #82909

jcfr mannequin opened this issue Nov 6, 2019 · 6 comments
Labels
build The build process and cross-build OS-windows type-feature A feature request or enhancement

Comments

@jcfr
Copy link
Mannequin

jcfr mannequin commented Nov 6, 2019

BPO 38728
Nosy @pfmoore, @tjguk, @zware, @zooba, @mathstuf, @jcfr
PRs
  • gh-82909: Update PC/pyconfig.h to allow disabling pragma based auto-linking #19740
  • 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 2019-11-06.22:41:31.365>
    labels = ['3.8', 'type-feature', '3.7', '3.9', 'OS-windows']
    title = 'Update PC/pyconfig.h to support disabling auto linking'
    updated_at = <Date 2020-05-12.23:29:17.552>
    user = 'https://github.com/jcfr'

    bugs.python.org fields:

    activity = <Date 2020-05-12.23:29:17.552>
    actor = 'mathstuf'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Windows']
    creation = <Date 2019-11-06.22:41:31.365>
    creator = 'Jean-Christophe Fillion-Robin'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 38728
    keywords = ['patch']
    message_count = 6.0
    messages = ['356158', '356310', '356365', '368474', '368744', '368749']
    nosy_count = 7.0
    nosy_names = ['paul.moore', 'tim.golden', 'python-dev', 'zach.ware', 'steve.dower', 'mathstuf', 'Jean-Christophe Fillion-Robin']
    pr_nums = ['19740']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue38728'
    versions = ['Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9']

    @jcfr
    Copy link
    Mannequin Author

    jcfr mannequin commented Nov 6, 2019

    When configuring project using build-system generator like CMake, the linking is explicitly handled and does not to be implicitly hard-coded in pyconfig.h

    Having the "pythonXY.lib" library hard-coded in pyconfig.h currently requires to explicitly specify a link directory. While possible, this require to increase the complexity of our build-system.

    I suggest we introduce a new macro (e.g PY_CONFIG_AUTOLINK_DISABLED)

    @jcfr jcfr mannequin added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes OS-windows type-feature A feature request or enhancement labels Nov 6, 2019
    @zooba
    Copy link
    Member

    zooba commented Nov 9, 2019

    I'm not totally averse to just removing the link pragmas completely, but since that's got significant compatibility impact, I'm happy with this approach.

    Let's shorten the name though. "PY_NO_LINK_LIB" might be better?

    @jcfr
    Copy link
    Mannequin Author

    jcfr mannequin commented Nov 11, 2019

    PY_NO_LINK_LIB will work well. I will work on a patch later this week.

    @jcfr
    Copy link
    Mannequin Author

    jcfr mannequin commented May 8, 2020

    Associated pull request has been updated, CLA signed and it is ready for final review and integration.

    Thanks so much for your time,

    @zooba
    Copy link
    Member

    zooba commented May 12, 2020

    Thanks for the ping - sorry, I've been largely offline while I'm between internet providers.

    The change is fine, but I wonder whether we should document it somewhere? Most likely in the devguide, which is in a separate repo, but perhaps a mention in PCbuild/readme.txt too?

    Presumably you looked around for ideas before figuring out the issue - anywhere you might have found it? Just in the source code?

    @mathstuf
    Copy link
    Mannequin

    mathstuf mannequin commented May 12, 2020

    Presumably you looked around for ideas before figuring out the issue

    Usually when "could not find foo.lib" popping up without any mention of "foo.lib" on the link line points directly to these "autolinking" "features" being the culprit. It's just something I've learned through experience. If there's an FAQ of common problems when building C extensions, it belongs there.

    While this functionality sounds nice in principle, it only really works if something also adds the directory to *look* for the library to the link line as well. But if you can get *that* to the link line, you may as well just add the ".lib" to the link line directly and not send the linker on a wild goose chase based on a header. In addition, nothing ties "find python.lib" to the one that actually goes with the header that's telling it to be found either, so you can get the wrong one too (probably not so much an issue for Python now that ABI flags are gone, but it's still a thing).

    Due to these behaviors and the lack of a link directory pragma (not that you could write a relocatable one in C preprocessor anyways), I find #pragma comment(lib) to just be a misfeature more than anything.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @arhadthedev arhadthedev added build The build process and cross-build and removed 3.9 only security fixes 3.8 only security fixes 3.7 (EOL) end of life labels Apr 2, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    build The build process and cross-build OS-windows type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants