[issue4347] Circular dependency causes SystemError when adding new syntax

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

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: $(srcdir)/Include/graminit.h   ↪ 
$(srcdir)/Include/Python-ast.h

Feel free to re-open if we are missing something :)

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Brett Cannon]

Brett Cannon  added the comment:

Pablo, is this still an issue?

--
nosy: +pablogsal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Brett Cannon]

Change by Brett Cannon :


--
resolution: wont fix -> 

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Brett Cannon]

Brett Cannon added the comment:

Ah, the fact parsetok.c gets compiled twice into different object files is the 
detail I was overlooking. Thanks for the clarification.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Martin Panter]

Martin Panter added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Brett Cannon]

Brett Cannon added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Martin Panter]

Martin Panter added the comment:

Equivalent patch for 2.7

--
Added file: http://bugs.python.org/file45576/graminit-dep.py2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Martin Panter]

Martin Panter added the comment:

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.

--
stage: needs patch -> patch review
status: closed -> open
versions: +Python 3.5, Python 3.6, Python 3.7 -Python 2.6
Added file: http://bugs.python.org/file45363/graminit-dep.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Martin Panter]

Martin Panter added the comment:

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 
import importlib.machinery # importlib first so we can test #15386 via -m
  File "/home/proj/python/cpython/Lib/importlib/__init__.py", line 57, in 

import types
  File "/home/proj/python/cpython/Lib/types.py", line 166, in 
import functools as _functools
  File "/home/proj/python/cpython/Lib/functools.py", line 345, in 
_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 
, 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.

--
nosy: +martin.panter

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Brett Cannon]

Brett Cannon added the comment:

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.

--
resolution:  - wont fix
status: pending - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Brett Cannon]

Brett Cannon added the comment:

This still an issue?

--
status: open - pending

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Brett Cannon]

Changes by Brett Cannon br...@python.org:


--
assignee: brett.cannon - 

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Brett Cannon]

Changes by Brett Cannon br...@python.org:


--
priority:  - normal
stage: patch review - needs patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Thomas Lee]

Thomas Lee t...@vector-seven.com added the comment:

This would appear to be another build quirk: Lib/symbol.py needs to be
regenerated if Grammar/Grammar changes.

Brett, do you think it would be okay for this file to be generated
automatically as part of the build process? I can't think of any good
reason why this should continue to be a manual task.

Lib/token.py is in a similar boat, except its dependency is on
Include/token.h

Whatever the decision, I'll provide another review/functional patch pair.

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Brett Cannon]

Brett Cannon br...@python.org added the comment:

On Mon, Feb 9, 2009 at 01:11, Thomas Lee rep...@bugs.python.org wrote:

 Thomas Lee t...@vector-seven.com added the comment:

 This would appear to be another build quirk: Lib/symbol.py needs to be
 regenerated if Grammar/Grammar changes.

 Brett, do you think it would be okay for this file to be generated
 automatically as part of the build process? I can't think of any good
 reason why this should continue to be a manual task.


It should be a build rule and not be a manual step.

 Lib/token.py is in a similar boat, except its dependency is on
 Include/token.h


Same answer.

 Whatever the decision, I'll provide another review/functional patch pair.

Thanks!

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Brett Cannon]

Brett Cannon br...@python.org added the comment:

So I finally got around to reviewing the patch and while it looks fine,
I ended up with some failing tests: test_compiler test_quopri test_sys
test_transformer.

Do you know what is going on Thomas? This is after repeated make/make
clean calls and using your non-review patch with all of the
auto-generated changes included.

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Thomas Lee]

Thomas Lee t...@vector-seven.com added the comment:

Brett, any feedback on this one?

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Brett Cannon]

Brett Cannon br...@python.org added the comment:

I will take a look when I can.

--
assignee:  - brett.cannon

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Brett Cannon]

Brett Cannon [EMAIL PROTECTED] added the comment:

Because of all the patch includes a bunch of junk for generated files
(any chance you can make a diff, Thomas, with only the stuff that really
requires review?), I have not done a real review. But a quick look does
show that the comment added to Grammar needs to be tweaked. Please use
complete sentences, including punctuation (e.g., capitalize words, end
in a period, etc.). Also don't use XXX just to grab attention for
something that is not meant to be removed.

--
nosy: +brett.cannon
stage:  - patch review

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Thomas Lee]

Thomas Lee [EMAIL PROTECTED] added the comment:

Thanks for the review Brett, apologies for the mess.

I'm attaching two new patches -- one for review, the other containing
the generated files. These differ very slightly from the original patch
-- mainly just removing some stuff I don't think is necessary.

You're probably aware of this, but it's important for the changes to the
generated files to be checked in too -- likewise for testing the patch.

Added file: http://bugs.python.org/file12208/build-evilness-fix-03.patch

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Thomas Lee]

Thomas Lee [EMAIL PROTECTED] added the comment:

And here's the patch for review.

Added file: http://bugs.python.org/file12209/build-evilness-fix-03-review.patch

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Thomas Lee]

Thomas Lee [EMAIL PROTECTED] added the comment:

I mean pgen's dependency on graminit.h, not the parser's. :)

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Thomas Lee]

Thomas Lee [EMAIL PROTECTED] added the comment:

okay, so it turns out that putting rules in Grammar/Grammar before the
top-level non-terminals breaks things in a different way.

Attached is another (hopefully final and working) patch.

Added file: http://bugs.python.org/file12185/build-evilness-fix-02.patch

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue4347] Circular dependency causes SystemError when adding new syntax

[Thomas Lee]

Thomas Lee [EMAIL PROTECTED] added the comment:

Here's a new patch that I believe should fix this issue.

It modifies Makefile.pre.in to include a few additional dependency
declarations for source files that depend on Include/graminit.h and
Include/Python-ast.h, as well as moving encoding_decl to the top of
Grammar/Grammar.

This should ensure that changes to the syntax and/or AST nodes will not
cause cryptic errors.

Longer term, it would be great to remove the parser's dependency on
graminit.h to ensure this sort of problem is never an issue.

--
title: Dependencies of graminit.h are not rebuilt when the file is regenerated 
- Circular dependency causes SystemError when adding new syntax
Added file: http://bugs.python.org/file12184/build-evilness-fix.patch

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue4347
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com