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

_Py_identifier should be _Py_IDENTIFER #57452

Closed
ericsnowcurrently opened this issue Oct 22, 2011 · 8 comments
Closed

_Py_identifier should be _Py_IDENTIFER #57452

ericsnowcurrently opened this issue Oct 22, 2011 · 8 comments
Assignees
Labels
build The build process and cross-build interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@ericsnowcurrently
Copy link
Member

BPO 13243
Nosy @loewis, @meadori, @ericsnowcurrently
Files
  • asdl.diff
  • 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 = 'https://github.com/meadori'
    closed_at = <Date 2011-10-22.19:14:28.059>
    created_at = <Date 2011-10-22.06:38:19.495>
    labels = ['interpreter-core', 'build']
    title = '_Py_identifier should be _Py_IDENTIFER'
    updated_at = <Date 2011-10-23.21:57:23.601>
    user = 'https://github.com/ericsnowcurrently'

    bugs.python.org fields:

    activity = <Date 2011-10-23.21:57:23.601>
    actor = 'loewis'
    assignee = 'meador.inge'
    closed = True
    closed_date = <Date 2011-10-22.19:14:28.059>
    closer = 'meador.inge'
    components = ['Interpreter Core']
    creation = <Date 2011-10-22.06:38:19.495>
    creator = 'eric.snow'
    dependencies = []
    files = ['23493']
    hgrepos = []
    issue_num = 13243
    keywords = ['patch']
    message_count = 8.0
    messages = ['146159', '146178', '146179', '146182', '146183', '146228', '146239', '146260']
    nosy_count = 3.0
    nosy_names = ['loewis', 'meador.inge', 'eric.snow']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'compile error'
    url = 'https://bugs.python.org/issue13243'
    versions = ['Python 3.3']

    @ericsnowcurrently
    Copy link
    Member Author

    Looks like Parser/asdl_c.py did not get all the way updated when _Py_identifier switched over to _Py_IDENTIFER. I've included a patch that fixes it (though it's relatively trivial). With this fix I did not notice any further problems.

    @ericsnowcurrently ericsnowcurrently added interpreter-core (Objects, Python, Grammar, and Parser dirs) build The build process and cross-build labels Oct 22, 2011
    @meadori
    Copy link
    Member

    meadori commented Oct 22, 2011

    Good catch. I see what happened. 7109f31300fb updated
    Python/Python-ast.c but not Parser/asdl_c.py. I will apply your patch
    shortly. Thanks.

    @meadori meadori self-assigned this Oct 22, 2011
    @meadori
    Copy link
    Member

    meadori commented Oct 22, 2011

    Oh, and just to be clear I reproduced the build break by doing:

    ./Parser/asdl_c.py -c ./Python ./Parser/Python.asdl
    make

    in a built tree. The reason that this wasn't caught is that the make
    rules have the ASDL files as dependencies on the AST C files. So, the
    C files are *not* updated unless the ASDL files are.

    Maybe we should change the build system to always regenerate the files
    or add something to automation that regenerates the AST C file every
    time.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 22, 2011

    New changeset 941d015053c6 by Meador Inge in branch 'default':
    bpo-13243: Rename _Py_identifier to _Py_IDENTIFIER in asdl_c.py
    http://hg.python.org/cpython/rev/941d015053c6

    @meadori
    Copy link
    Member

    meadori commented Oct 22, 2011

    Committed. Thanks!

    @meadori meadori closed this as completed Oct 22, 2011
    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Oct 23, 2011

    Maybe we should change the build system to always regenerate the files
    or add something to automation that regenerates the AST C file every
    time.

    Most definitely not. It is very deliberate that asdl_c.py is only
    invoked when the ASDL sources change. Otherwise, having Python installed
    would be a build requirement for Python, which it must be not.

    @meadori
    Copy link
    Member

    meadori commented Oct 23, 2011

    Most definitely not. It is very deliberate that asdl_c.py is only
    invoked when the ASDL sources change. Otherwise, having Python installed
    would be a build requirement for Python, which it must be not.

    OK, thanks for the background. To be clear, though, the build dependency
    is already there. You just have to touch the ASDL sources to run
    into it (Lib/opcode.py as well). This is even documented in the
    Makefile*:

    # XXX Note that a build now requires Python exist before the build starts
    ASDLGEN= $(srcdir)/Parser/asdl_c.py

    However, modifying the ASDL source is, with respect to other source
    modifications, not as common. So, I see no reason to make the situation worse
    by running asd_c.py all the time. Suggestion withdrawn.

    • If not relying on an external Python is a hard requirement (and I do
      see why that may be useful),
      then the build system could be changed to only allow changes to
      problematic sources
      (ASDL, opcode.py) after Python has been built once. Then the built
      Python could be used.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Oct 23, 2011

    Am 23.10.2011 20:33, schrieb Meador Inge:

    Meador Inge <meadori@gmail.com> added the comment:

    > Most definitely not. It is very deliberate that asdl_c.py is only
    > invoked when the ASDL sources change. Otherwise, having Python installed
    > would be a build requirement for Python, which it must be not.

    OK, thanks for the background. To be clear, though, the build dependency
    is already there. You just have to touch the ASDL sources to run
    into it (Lib/opcode.py as well). This is even documented in the
    Makefile*:

    Just to be more clear: normally, generated files shouldn't be checked
    into the source repository. However, exceptions are typically made for
    generated files that may be difficult to regenerate on some target
    systems. For Python, this includes configure (depends on autoconf being
    run), and various files generated from Python scripts (also including
    the Unicode database, for example).

    So when we check in generated files, the build dependency becomes unused
    unless the source file gets modified - which, as you point out, happens
    rarely, and never happens for somebody who just wants to build Python.

    • If not relying on an external Python is a hard requirement (and I do
      see why that may be useful), then the build system could be changed to only allow changes to
      problematic sources (ASDL, opcode.py) after Python has been built once. Then the built
      Python could be used.

    See above. It's fine to require core contributors to have Python
    installed (and also autoconf). We just need to make sure that
    Python is not required on the target system (as people may try
    to install Python initially on that system).

    @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 interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants