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

[Patch] Fix the ability to cross compile Python when doing a rebuild of importlib.h #72253

Closed
EdSchouten mannequin opened this issue Sep 10, 2016 · 6 comments
Closed
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@EdSchouten
Copy link
Mannequin

EdSchouten mannequin commented Sep 10, 2016

BPO 28066
Nosy @vadmium, @EdSchouten
Files
  • patch-freeze-importlib: Patch to make cross compilation work
  • patch-freeze-importlib: Revised patch: make sure that _freeze_importlib writes to the right location
  • srcdir-check.patch
  • 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 = <Date 2016-09-12.03:35:03.551>
    created_at = <Date 2016-09-10.19:02:01.637>
    labels = ['interpreter-core']
    title = '[Patch] Fix the ability to cross compile Python when doing a rebuild of importlib.h'
    updated_at = <Date 2016-09-12.03:35:03.549>
    user = 'https://github.com/EdSchouten'

    bugs.python.org fields:

    activity = <Date 2016-09-12.03:35:03.549>
    actor = 'martin.panter'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-09-12.03:35:03.551>
    closer = 'martin.panter'
    components = ['Interpreter Core']
    creation = <Date 2016-09-10.19:02:01.637>
    creator = 'EdSchouten'
    dependencies = []
    files = ['44536', '44545', '44549']
    hgrepos = []
    issue_num = 28066
    keywords = ['patch']
    message_count = 6.0
    messages = ['275674', '275712', '275735', '275754', '275799', '275911']
    nosy_count = 3.0
    nosy_names = ['python-dev', 'martin.panter', 'EdSchouten']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue28066'
    versions = ['Python 3.5', 'Python 3.6']

    @EdSchouten
    Copy link
    Mannequin Author

    EdSchouten mannequin commented Sep 10, 2016

    For CloudABI (https://mail.python.org/pipermail/python-dev/2016-July/145708.html) we're providing packages containing a precompiled copy of Python. As we had to make some changes to importlib (namely to deal with directory file descriptors), we have to do a rebuild of importlib.h during the cross compilation process, using Programs/_freeze_importlib.

    We've run into a couple of issues that require us to patch up the build system to make this work:

    • First of all, Programs/_freeze_importlib is built using the compiler for the target. There is no way to override this. The OpenWRT folks have also run into this issue and have patched up the Makefile to add a $FREEZE_IMPORTLIB that allows you to use a different executable:

    https://github.com/openwrt/packages/blob/master/lang/python3/patches/013-make-freeze-import-lib-into-an-override-able-var.patch

    This patch is almost correct; it doesn't prevent _freeze_importlib from being built, which is needed in our case as it doesn't build cleanly on CloudABI.

    • Second, if an out-of-tree build is performed, we need to make sure that Python/frozen.c imports the right copy of importlib.h. We must add -IPython to the compiler flags to realise.

    Attached is a patch that contains the modifications that we've made to Makefile.pre.in to make cross compilation work for us.

    @EdSchouten EdSchouten mannequin added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Sep 10, 2016
    @vadmium
    Copy link
    Member

    vadmium commented Sep 10, 2016

    Can you get your importlib changes so they are compatible with a native build? If so, maybe you can build the frozen module with a native build rather than a cross build.

    “freeze_importlib is built using the compiler for the target”: I may be mistaken, but I thought we recently prevented this from being cross-compiled: bpo-27641, revision bc677cb34889, Jul 2016. Your patch seems to be based ontop of this change. Perhaps you have found some way to bypass that cross-compilation detection logic.

    The addition of $(FREEZE_IMPORTLIB) is a bit like the outdated cross-override.patch I once proposed for bpo-22625.

    @EdSchouten
    Copy link
    Mannequin Author

    EdSchouten mannequin commented Sep 11, 2016

    The nice thing is that in our case, the importlib changes are already compatible with the native build. So yes, we can reuse the frozen module from the native build. :-)

    Ah, yes. bpo-27641 already prevents that it's cross compiled. This patch was written prior to that and simply rebased.

    Still, one problem remains: our strategy for creating importlib.h and importlib_external.h doesn't take out-of-tree builds into account. Even if we regenerate it, frozen.c won't pick it up because its directory is not part of the compiler search path.

    Attached is a patch that makes us write the results of _freeze_importlib into $(srcdir)/Python/importlib*.h instead, so that even with an out-of-tree build, we actually update the header files used by frozen.c.

    @vadmium
    Copy link
    Member

    vadmium commented Sep 11, 2016

    There are various tricky cases to be considered with the regenerated files like importlib.h. One of them is that you are supposed to be able to build Python from a read-only source tree. See bpo-15819. Writing files into $(srcdir) would break this.

    Also, as I just wrote in that bug, the built Python/importlib.h is already supposed to be searchable. However the logic seems to be broken. Can you see if this patch helps?

    @EdSchouten
    Copy link
    Mannequin Author

    EdSchouten mannequin commented Sep 11, 2016

    It does. I can now cross build Python for CloudABI by copying importlib.h and importlib_external.h from the native build directory to the target build directory. Thanks!

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 12, 2016

    New changeset c26dce72a4da by Martin Panter in branch '3.5':
    Issue bpo-28066: Fix include search directory logic for out-of-tree builds
    https://hg.python.org/cpython/rev/c26dce72a4da

    New changeset 3ef078f96494 by Martin Panter in branch 'default':
    Issue bpo-28066: Merge srcdir fix from 3.5
    https://hg.python.org/cpython/rev/3ef078f96494

    @vadmium vadmium removed the 3.7 (EOL) end of life label Sep 12, 2016
    @vadmium vadmium closed this as completed Sep 12, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant