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
Circular dependency causes SystemError when adding new syntax #48597
Comments
It's important that dependencies of grammar.h get rebuilt if graminit.h The net result is that a program afflicted by this might build without The patch is quite simple and ensures that all files that currently It also removes an unnecessary #include from Python/future.c. I believe a similar situation might occur with Python-ast.h and the |
Updating affected versions. Probably affects 3.x too. |
A deeper issue here is that Parser/parsetok.c has a dependency on This breaks Python whenever syntax is added to or removed from A simple work around for those wishing to change the syntax is a "make; |
Here's a new patch that I believe should fix this issue. It modifies Makefile.pre.in to include a few additional dependency This should ensure that changes to the syntax and/or AST nodes will not Longer term, it would be great to remove the parser's dependency on |
I mean pgen's dependency on graminit.h, not the parser's. :) |
okay, so it turns out that putting rules in Grammar/Grammar before the Attached is another (hopefully final and working) patch. |
Because of all the patch includes a bunch of junk for generated files |
Thanks for the review Brett, apologies for the mess. I'm attaching two new patches -- one for review, the other containing You're probably aware of this, but it's important for the changes to the |
And here's the patch for review. |
Brett, any feedback on this one? |
I will take a look when I can. |
So I finally got around to reviewing the patch and while it looks fine, Do you know what is going on Thomas? This is after repeated make/make |
This would appear to be another build quirk: Lib/symbol.py needs to be Brett, do you think it would be okay for this file to be generated Lib/token.py is in a similar boat, except its dependency is on Whatever the decision, I'll provide another review/functional patch pair. |
On Mon, Feb 9, 2009 at 01:11, Thomas Lee <report@bugs.python.org> wrote:
It should be a build rule and not be a manual step.
Same answer.
Thanks! |
This still an issue? |
Since this has been sitting here for two months as pending I'm closing as won't fix since mucking with the grammar is such a rarity and getting the build rules right is so complicated it isn't worth changing. |
I occasionally get the following error, due to Parser/parsetok.o being older than Include/graminit.h. ./python -E -S -m sysconfig --generate-posix-vars ; if test $? -ne 0 ; then echo "generate-posix-vars failed" ; rm -f ./pybuilddir.txt ; exit 1 ; fi
Could not import runpy module
Traceback (most recent call last):
File "/home/proj/python/cpython/Lib/runpy.py", line 14, in <module>
import importlib.machinery # importlib first so we can test python/issues-test-cpython#15386 via -m
File "/home/proj/python/cpython/Lib/importlib/__init__.py", line 57, in <module>
import types
File "/home/proj/python/cpython/Lib/types.py", line 166, in <module>
import functools as _functools
File "/home/proj/python/cpython/Lib/functools.py", line 345, in <module>
_CacheInfo = namedtuple("CacheInfo", ["hits", "misses", "maxsize", "currsize"])
File "/home/proj/python/cpython/Lib/collections/__init__.py", line 428, in namedtuple
exec(class_definition, namespace)
SystemError: invalid node 339 for PyAST_FromNode
generate-posix-vars failed
*** Error code 1 The best workaround for me (less brute force than removing the whole build tree) seems to be to add Parser/parsetok.c to the list of files to “touch” the timestamps of before building. The dependency in Parser/parsetok.c on Include/graminit.h was added by <https://hg.python.org/cpython/diff/06f1e2fd05bc/Parser/parsetok.c\>, presumably to support encoding declarations in Python source files. I haven’t tried, but perhaps to avoid pgen depending on its output, a quick fix would be to add #ifndef PGEN around the offending code. For Python 2, a Parser/parsetok_pgen.c wrapper file would have to added, like in revision 6e9dc970ac0e. |
Here is a patch for Python 3 implementing my idea. There is already code conditionally compiled out for the pgen build, so my patch just expands that to avoid the "graminit.h" include and derived functions. |
Equivalent patch for 2.7 |
So what does having the tokenizer not depend on pgen accomplish? Does it break a circular dependency where the tokenizer will get rebuilt with pgen before a full build starts? |
I’m not sure I understand your questions Brett (Which tokenizer? What rebuilding?), but I will try to explain some relevant parts. My main goal is to add a makefile dependency of parsetok.o on $(GRAMMAR_H). In Python 3, that is easy, and (I realize now) my problem would be solved without changing any other file. However my changes in parsetok.c eliminate an unnecessary bootstrapping problem, so I still think they are worthwhile. For Python 3, the parsetok.c code gets compiled to parsetok_pgen.o, and after executing, also gets compiled to a separate parsetok.o file. Rebuilding the code is not a problem here. In Python 2, the same parsetok.o object is both part of the pgen program, and the eventual Python program. In order to add my desired makefile dependency, I have to duplicate parsetok.c compilation into two makefile rules (parsetok.o and parsetok_pgen.o). So you could say I am breaking a circle _by_ rebuilding the code. Dependencies between files with my patch applied: Parser/parsetok_pgen.o -> Parser/pgen -> Include/graminit.h -> Parser/parsetok.o -> python |
Ah, the fact parsetok.c gets compiled twice into different object files is the detail I was overlooking. Thanks for the clarification. |
Pablo, is this still an issue? |
I think there is nothing missing now that the C version of pgen is gone. And parsetok is already rebuilt when gramminit.h changes: Python/compile.o Python/symtable.o Python/ast_unparse.o Python/ast.o Python/future.o Parser/parsetok.o: Feel free to re-open if we are missing something :) |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: