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

Module 'parser' fails to build #48529

Closed
kirkshorts mannequin opened this issue Nov 7, 2008 · 13 comments
Closed

Module 'parser' fails to build #48529

kirkshorts mannequin opened this issue Nov 7, 2008 · 13 comments
Labels
build The build process and cross-build extension-modules C modules in the Modules dir

Comments

@kirkshorts
Copy link
Mannequin

kirkshorts mannequin commented Nov 7, 2008

BPO 4279
Nosy @loewis, @msapiro
Files
  • nu_diff.txt: Patch to use correct grammar file.
  • 2.6.1-parsermodule.patch: _PyParser_Grammar export patch (take 2)
  • 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 2009-01-11.09:33:52.717>
    created_at = <Date 2008-11-07.13:58:04.305>
    labels = ['extension-modules', 'build']
    title = "Module 'parser' fails to build"
    updated_at = <Date 2009-01-11.09:33:52.705>
    user = 'https://bugs.python.org/kirkshorts'

    bugs.python.org fields:

    activity = <Date 2009-01-11.09:33:52.705>
    actor = 'loewis'
    assignee = 'none'
    closed = True
    closed_date = <Date 2009-01-11.09:33:52.717>
    closer = 'loewis'
    components = ['Extension Modules']
    creation = <Date 2008-11-07.13:58:04.305>
    creator = 'kirkshorts'
    dependencies = []
    files = ['11970', '12682']
    hgrepos = []
    issue_num = 4279
    keywords = ['patch']
    message_count = 13.0
    messages = ['75603', '75664', '75665', '75666', '75667', '75668', '75685', '76445', '76449', '76450', '79584', '79585', '79592']
    nosy_count = 5.0
    nosy_names = ['loewis', 'msapiro', 'LambertDW', 'kirkshorts', 'yselkowitz']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'compile error'
    url = 'https://bugs.python.org/issue4279'
    versions = ['Python 2.6', 'Python 3.0']

    @kirkshorts
    Copy link
    Mannequin Author

    kirkshorts mannequin commented Nov 7, 2008

    With a clean checkout of the py3k source it fails to build the Parser
    exension module (based on Modules/parsermodule.c) when building against
    a cygwin target.

    This appears to be related to the movement of the symbol
    _PyParser_Grammar from graminit.c to Parser\metagrammar.c. (At least
    this is where it was going by the code comments)

    Because of this the module now requires Parser\metagrammar.c to get
    this information via Py_meta_grammar.

    The patch modifies setup.py to add the required file and modifies
    parsermodule.c to use the accessor function.

    (This fails on a clean trunk build in the same way as well - which
    makes me very suspicious that this is not a "real" issue as the
    buildbots seem to be green.)

    My gut feeling is that my modification to setup.py for the module is
    incorrect - it just looks messy. So I await the inevitable: "That's not
    how to fix it...." :-)

    @kirkshorts kirkshorts mannequin added extension-modules C modules in the Modules dir build The build process and cross-build labels Nov 7, 2008
    @kirkshorts
    Copy link
    Mannequin Author

    kirkshorts mannequin commented Nov 9, 2008

    bah I *am* a idiot, bpo-4288 and Christian's comments point out that I
    can't use 'find' & 'xargs' properly :-(

    Will modify patch to use the correct grammar file &c.

    (and maybe one day I might actually say something sensible to do with
    Python development :-) )

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Nov 9, 2008

    As Christian said in bpo-4288: this links in a separate of metagrammar.c,
    which is undesirable. However, I think you can fix this by exporting
    Py_meta_grammar from pythonxy.dll.

    @kirkshorts
    Copy link
    Mannequin Author

    kirkshorts mannequin commented Nov 9, 2008

    a new patch that will use the grammar definition from Python/graminit.c

    • it is as of yet untested for Cygwin (can't get to that machine right
      now). It follows the same pattern as the previous, i.e. it makes us of
      an accessor function to get the grammar definition.

    This has expanded the patch somewhat to include changes to:

    All of which makes me think that the change to make the symbol "public"
    and use it directly without hiding it is a better way to go.

    Will look at this under my Cygwin environment tomorrow. (I have run a
    configre - make - test cycle on Ubuntu (hardy heron) and it is OK [but
    then its not broken there anyway :-) ] )

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Nov 9, 2008

    That patch is too complicated. We already have meta_grammar and
    Py_meta_grammar, and now you also add a third function
    (get_PyParserGrammar) that does the same thing again. I don't see why
    you can't call one of the existing functions, and I fail to see the need
    to change pythonrun.c at all.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Nov 9, 2008

    As a style guide remark: drop the parentheses around the expression in
    the return statement (return is a statement, not a function), and prefix
    all global symbols with Py or _Py. See PEP-7 for further instructions.

    @kirkshorts
    Copy link
    Mannequin Author

    kirkshorts mannequin commented Nov 10, 2008

    Martin:

    Looking at it I agree with you 100% - the patch is too complicated for
    what it is intending to resolve. It simply does not need another
    accessor function to muddy the waters when making the symbol public as
    done in bpo-4288 resolves the issue.

    My personal preference is to try to hide such data structures - but
    that doesn't always mean its the correct thing to do :-)

    @yselkowitz
    Copy link
    Mannequin

    yselkowitz mannequin commented Nov 26, 2008

    I can confirm this problem in 3.0rc3 on Cygwin. If I could get some
    direction from the Python devs on which method would be preferred
    (accessor function vs. exposing data structure), I would be happy to
    provide a patch.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Nov 26, 2008

    I think the parser module should call one of the existing functions.

    @yselkowitz
    Copy link
    Mannequin

    yselkowitz mannequin commented Nov 26, 2008

    Attempting to export and use Py_meta_grammar() gives me a broken module,
    based on the results of test/test_parser.py.

    This patch, which exports _PyParser_Grammar, is very simple and the test
    passes.

    I will gladly take guidance on this patch.

    @msapiro
    Copy link
    Mannequin

    msapiro mannequin commented Jan 10, 2009

    This problem also occurs when building the 2.6.1 parser module on Cygwin
    1.5.25. It did not occur with Python 2.6 or 2.5.x.

    The error from 'make' is

    building 'parser' extension
    gcc -shared -Wl,--enable-auto-image-base
    build/temp.cygwin-1.5.25-i686-2.6/cygdrive/c/Python_dist/Python-2.6.1/Modules/parsermodule.o
    -L/usr/local/lib -L. -lpython2.6 -o
    build/lib.cygwin-1.5.25-i686-2.6/parser.dll
    build/temp.cygwin-1.5.25-i686-2.6/cygdrive/c/Python_dist/Python-2.6.1/Modules/parsermodule.o:
    In function parser_expr': /cygdrive/c/Python_dist/Python-2.6.1/Modules/parsermodule.c:552: undefined reference to __PyParser_Grammar'
    build/temp.cygwin-1.5.25-i686-2.6/cygdrive/c/Python_dist/Python-2.6.1/Modules/parsermodule.o:
    In function parser_suite': /cygdrive/c/Python_dist/Python-2.6.1/Modules/parsermodule.c:552: undefined reference to __PyParser_Grammar'
    collect2: ld returned 1 exit status

    I was able to work around the error and build a parser module that
    passed unit test by manually running

    gcc -shared -Wl,--enable-auto-image-base
    build/temp.cygwin-1.5.25-i686-2.6/cygdrive/c/Python_dist/Python-2.6.1/Modules/parsermodule.o
    Python/graminit.o -L/usr/local/lib -L. -lpython2.6 -o
    build/lib.cygwin-1.5.25-i686-2.6/parser.dll

    i.e. by including Python/graminit.o in the explicit object files to load.

    I have also confirmed that applying the parser-grammar.patch from bpo-4288
    will allow make to successfully build a parser module that passes unit
    tests.

    @yselkowitz
    Copy link
    Mannequin

    yselkowitz mannequin commented Jan 11, 2009

    Here's the patch I used for the Cygwin Ports 2.6 and 3.0 packages. It's
    the simplest working solution that I could find.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Jan 11, 2009

    I have now committed 2.6.1-parsermodule.patch as r68523, r68524, r68525,
    and r68526. Thanks for the patch.

    @loewis loewis mannequin closed this as completed Jan 11, 2009
    @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 extension-modules C modules in the Modules dir
    Projects
    None yet
    Development

    No branches or pull requests

    0 participants