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

Do not build Programs/_freeze_importlib when cross-compiling #71828

Closed
thp mannequin opened this issue Jul 28, 2016 · 8 comments
Closed

Do not build Programs/_freeze_importlib when cross-compiling #71828

thp mannequin opened this issue Jul 28, 2016 · 8 comments
Labels
build The build process and cross-build type-bug An unexpected behavior, bug, or error

Comments

@thp
Copy link
Mannequin

thp mannequin commented Jul 28, 2016

BPO 27641
Nosy @xdegaye, @vadmium, @moreati, @thp
Files
  • python-freeze-importlib-cross-compiling.patch: Only build Programs/_freeze_importlib when it is going to be used
  • comment-out-regen.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-08-04.04:12:12.030>
    created_at = <Date 2016-07-28.11:04:44.797>
    labels = ['type-bug', 'build']
    title = 'Do not build Programs/_freeze_importlib when cross-compiling'
    updated_at = <Date 2016-08-04.04:12:12.028>
    user = 'https://github.com/thp'

    bugs.python.org fields:

    activity = <Date 2016-08-04.04:12:12.028>
    actor = 'martin.panter'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-08-04.04:12:12.030>
    closer = 'martin.panter'
    components = ['Cross-Build']
    creation = <Date 2016-07-28.11:04:44.797>
    creator = 'thomas.perl'
    dependencies = []
    files = ['43920', '43933']
    hgrepos = []
    issue_num = 27641
    keywords = ['patch']
    message_count = 8.0
    messages = ['271519', '271606', '271647', '271737', '271760', '271936', '271942', '271947']
    nosy_count = 5.0
    nosy_names = ['xdegaye', 'python-dev', 'martin.panter', 'Alex.Willmer', 'thomas.perl']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue27641'
    versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

    @thp
    Copy link
    Mannequin Author

    thp mannequin commented Jul 28, 2016

    Based on http://bugs.python.org/issue27490 and http://bugs.python.org/msg271495, here is a patch that makes sure Programs/_freeze_importlib is only built when not cross-compiling.

    @thp thp mannequin added build The build process and cross-build type-bug An unexpected behavior, bug, or error labels Jul 28, 2016
    @vadmium
    Copy link
    Member

    vadmium commented Jul 29, 2016

    Here is another possible option. It is still a bit of a hack, because the configure script inserts comments into the makefile, but this is already done e.g. with @EXPORT_MACOSX_DEPLOYMENT_TARGET@. The advantage is we get to keep the filenames of the dependencies in the makefile, so it should be easier to read and maintain. And we get to eliminate the $(cross_compiling) check as well.

    In native compiling mode, everything should be as normal. When cross-compiling, the rules for regenerating files should read like

    Python/importlib.h: # $(srcdir)/Lib/importlib/_bootstrap.py Programs/_freeze_importlib
    ./Programs/_freeze_importlib \
    $(srcdir)/Lib/importlib/_bootstrap.py Python/importlib.h

    Since the Python/importlib.h should already exist, the rule in this form (dependencies commented out) won’t be run, and Make won’t need to build Programs/_freeze_importlib either.

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Jul 29, 2016

    Nice.

    FWIW cross-compilation for Android works fine with comment-out-regen.patch.

    In native compiling mode, everything should be as normal. When cross-compiling, the rules for regenerating files should read like

    Python/importlib.h: # $(srcdir)/Lib/importlib/_bootstrap.py Programs/_freeze_importlib
    ./Programs/_freeze_importlib \
    $(srcdir)/Lib/importlib/_bootstrap.py Python/importlib.h

    They do read like this.

    @thp
    Copy link
    Mannequin Author

    thp mannequin commented Jul 31, 2016

    +1 on comment-out-regen.patch, makes things much cleaner and removes the shell "if" in the rule body.

    Just a small bikeshed issue: Instead of COMMENT_REGEN, maybe call it "CROSS_COMPILE_COMMENT" or "GENERATED_COMMENT" or "COMMENT_IF_CROSS" or somesuch? This way, might be easier to read/understand the makefile rules ("COMMENT_IF_CROSS" -> "a comment character will be inserted here if cross-compiling")?

    @vadmium
    Copy link
    Member

    vadmium commented Aug 1, 2016

    Thanks for the feedback. I did wonder about the name. Perhaps I will go with GENERATED_COMMENT:

    GENERATED_COMMENT='#'

    Python/importlib_external.h: @GENERATED_COMMENT@ $(srcdir)/Lib/importlib/_bootstrap_external.py Programs/_freeze_importlib

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 4, 2016

    New changeset bc677cb34889 by Martin Panter in branch '3.5':
    Issue bpo-27641: Comment out regeneration rules when cross compiling
    https://hg.python.org/cpython/rev/bc677cb34889

    New changeset fc034d3607a8 by Martin Panter in branch 'default':
    Issue bpo-27641: Merge cross-compiling improvement from 3.5
    https://hg.python.org/cpython/rev/fc034d3607a8

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 4, 2016

    New changeset e3757f3e1848 by Martin Panter in branch '2.7':
    Issue bpo-27641: Comment out regeneration rules when cross compiling
    https://hg.python.org/cpython/rev/e3757f3e1848

    @vadmium
    Copy link
    Member

    vadmium commented Aug 4, 2016

    _freeze_import is only in Python 3, but I backported my final change because it is more general and also affects pgen.

    @vadmium vadmium closed this as completed Aug 4, 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
    build The build process and cross-build type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant