You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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...." :-)
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 commented on Nov 9, 2008
loewismannequin
Mannequin
More actions
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 commented on Nov 9, 2008
kirkshortsmannequin
MannequinAuthor
More actions
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 commented on Nov 9, 2008
loewismannequin
Mannequin
More actions
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 commented on Nov 9, 2008
loewismannequin
Mannequin
More actions
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 commented on Nov 10, 2008
kirkshortsmannequin
MannequinAuthor
More actions
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 commented on Nov 26, 2008
yselkowitzmannequin
Mannequin
More actions
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 commented on Nov 26, 2008
loewismannequin
Mannequin
More actions
I think the parser module should call one of the existing functions.
yselkowitz commented on Nov 26, 2008
yselkowitzmannequin
Mannequin
More actions
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 commented on Jan 10, 2009
msapiromannequin
Mannequin
More actions
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
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 commented on Jan 11, 2009
yselkowitzmannequin
Mannequin
More actions
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 commented on Jan 11, 2009
loewismannequin
Mannequin
More actions
I have now committed 2.6.1-parsermodule.patch as r68523, r68524, r68525,
and r68526. Thanks for the patch.
Activity
kirkshorts commentedon 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 commentedon 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 commentedon 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 commentedon Nov 9, 2008
a new patch that will use the grammar definition from Python/graminit.c
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 commentedon 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 commentedon 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 commentedon 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 commentedon 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 commentedon Nov 26, 2008
I think the parser module should call one of the existing functions.
yselkowitz commentedon 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 commentedon 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 commentedon 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 commentedon Jan 11, 2009
I have now committed 2.6.1-parsermodule.patch as r68523, r68524, r68525,
and r68526. Thanks for the patch.