regen.vcxproj cannot regenerate some necessary files

[pjx206]

BPO	43567
Nosy	@gvanrossum, @zooba, @tonybaloney, @pablogsal, @tonybaloney, @pjx206
PRs	bpo-43567: Improved generated code refresh on Windows #25120

^{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/zooba'
closed_at = <Date 2021-04-06.22:57:11.973>
created_at = <Date 2021-03-20.11:02:18.427>
labels = ['build', '3.10', 'OS-windows']
title = 'regen.vcxproj cannot regenerate some necessary files'
updated_at = <Date 2021-04-06.22:57:11.972>
user = 'https://github.com/pjx206'

bugs.python.org fields:

activity = <Date 2021-04-06.22:57:11.972>
actor = 'steve.dower'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2021-04-06.22:57:11.973>
closer = 'steve.dower'
components = ['Build', 'Windows']
creation = <Date 2021-03-20.11:02:18.427>
creator = 'pjx206'
dependencies = []
files = []
hgrepos = []
issue_num = 43567
keywords = ['patch']
message_count = 22.0
messages = ['389155', '389743', '389765', '389766', '389770', '389778', '389781', '389835', '389841', '389863', '389866', '389868', '389870', '389880', '389924', '389927', '389929', '389935', '389954', '390037', '390383', '390384']
nosy_count = 6.0
nosy_names = ['gvanrossum', 'steve.dower', 'anthonypjshaw', 'pablogsal', 'anthony shaw', 'pjx206']
pr_nums = ['25120']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue43567'
versions = ['Python 3.10']

[pjx206]

I tried to modify Grammar/python.gram, Grammar/Tokens, Parser/Python.asdl, to add a new token to the grammar. And when using build.bat --regen, only parser.c is newly generated. Other files, as mentioned in https://devguide.python.org/grammar/ that should be updated:ast.c, Python-ast.h etc.
regen.vcxproj is now not capable for the new PEG parser. So an update is need.

[gvanrossum]

Steve: let's talk about this in person today.

[pablogsal]

regen.vcxproj is now not capable for the new PEG parse

Hummmm, that file should handle the peg parser already:

cpython/PCbuild/regen.vcxproj

Lines 156 to 163 in 09b90a0

	<Target Name="_RegenPegen" BeforeTargets="Build">
	<!-- Regenerate Parser/parser.c -->
	<SetEnv Name="PYTHONPATH" Prefix="true" Value="$(PySourcePath)Tools\peg_generator\" />
	<Exec Command="&quot;$(PythonExe)&quot; -m pegen -q c &quot;$(PySourcePath)Grammar\python.gram&quot; &quot;$(PySourcePath)Grammar\Tokens&quot; -o &quot;$(IntDir)parser.c&quot;" />
	<Copy SourceFiles="$(IntDir)parser.c" DestinationFiles="$(PySourcePath)Parser\parser.c">
	<Output TaskParameter="CopiedFiles" ItemName="_UpdatedParse" />
	</Copy>
	<Warning Text="Pegen updated. You will need to rebuild pythoncore to see the changes." Condition="'@(_UpdatedParse)' != ''" />

[vstinner]

By the way, it would be nice if someone could review my change in this project related to pycore_ast.h ;-) (commit 94faa07) I mean checking if pycore_ast.h is updated as expected.

[gvanrossum]

I've found build.bat --regen unreliable myself. I went over it with Steve and one issue that came up is that it uses the python.exe that is built to run the code generation scripts; OTOH on Linux/Mac these scripts are run using a suitable pre-existing Python binary, e.g. a recent enough system Python. (However, the frozen files require a binary (_freeze_importlib).

It will take some time to refactor things so that this is more usable.

Perhaps instead of (or in addition to) doing everything via .vcxproj files, we could have a simple(r) regen.bat script that runs all the regen scripts (or at least the non-freeze ones) using the system Python?

FWIW my own issues weren't with the parser, they were with regenerating opcode.h and opcode_targets.h.

[gvanrossum]

Workaround:

For opcode.py changes, I can probably use the following strategy:

• build.bat
• Add new opcode to opcode.py
• build.bat --regen
• build.bat
• Make changes to compile.c and ceval.c to implement new opcode
• build.bat

For the parser perhaps you can adopt a similar strategy, where you first
add new tokens and get the code generation working?

[pjx206]

I did:

• modified Grammar/python.gram
  • added new operator to compare_op_bitwise_or_pair
• added corresponding token to Grammer/Tokens
• build.bar --regen
• not having Include/Python-ast.h and Python/Python-ast.c

I agree that a regen.bat that using a system python is better solution. regen.vcxproj is confusing.

[gvanrossum]

Can you past the full output from your command?

Also, try making a dummy change (e.g. add a blank line) to each of the input files -- this will trigger the rebuild.

PS. Please don't remove nosy folks unless they request so.

[gvanrossum]

@Steve, Anthony: I am now in a state where I have *no* changes anywhere, build.bat completes without errors, but build.bat --regen produces this error (after first doing a regular build):

C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\Common7\IDE\VC\VCTargets\Microsoft.Cpp.Platform.targets(67,5): error MSB8020: The build to
ols for v142 (Platform Toolset = 'v142') cannot be found. To build using the v142 build tools, please install v142 build tools. Alternatively, you may u
pgrade to the current Visual Studio tools by selecting the Project menu or right-click the solution, and then selecting "Retarget solution". [C:\Users\gv
anrossum\cpython\PCbuild\regen.vcxproj]

Help!

[gvanrossum]

FWIW, could this issue recently merged by Victor be related?
bpo-43244: Remove ast.h, asdl.h, Python-ast.h headers (GH-24933)

[pjx206]

build.bat --regen produces such error because it uses build tool from visual studio to build regen.vcxproj, and you do not have v142 toolset. I'm using vs2019 and have v142 toolset installed and regen.vcxproj workd, just cannot actually regenerate files needed completely.

[gvanrossum]

So is there a bug in regen.vcxproj, or in https://devguide.python.org/setup/#windows-compiling ? The latter says VS 2017.

@pjx206: please stop clearing out the nosy list. You've done that twice now. If you do it again, I'll close this issue.

[pjx206]

The regen.vcxproj was modified but not test well. In the first version of regen.vcxproj there were such build targets:

_RegenGrammar(BeforeTargets="Build") -> _RegenAST_H -> _RegenAST_C -> _RegenOpcodes -> _RegenTokens -> _RegenKeywords -> _RegenSymbols

In commit 1ed83ad: bpo-40939: Remove the old parser (GH-20768),
"_RegenGrammar" was removed, but there's still "_RegenAST_H" as AfterTargets of "_RegenGrammar".

@gvanrossum sorry about removing nosy folks, This website displays in my language so I did not realize what is "nosy list".

[vstinner]

not having Include/Python-ast.h and Python/Python-ast.c

FYI Include/Python-ast.h was renamed to Include/internal/pycore_ast.h.

[gvanrossum]

Anthony, can you help us out here? Is there a problem with regen.vcsproj, either by depending on VS 2019 (which we don't officially support yet) or by missing the changes in the names of some generated files?

[zooba]

I've attached my PR that streamlines this, it should be good (as in, reliable/fast) to do the regen on every build, but the "build.bat --regen" command is retained to force it.

[gvanrossum]

@jiaxin Please test steve's PR.

[tonybaloney]

Guido, regen.vcxproj targets 142 as the SDK version, which is most likely a mistake.
The other projects are part of the main PCBuild.sln solution, which has a variable for the base SDK version.

If you need to change it quickly, you can either open it in VS and right click on the project and change the properties to an older SDK, or edit the vcxproj file itself.

I can make this change in a patch, but because its a standalone project, it doesn't share the base SDK version with the others.

I've reviewed Steve's patch and it would fix this because it changes regen into a build target instead of a build project, so it doesn't specify the SDK version at all.
Steve's implementation is also much, much cleaner!

[pjx206]

Steve's PR zooba:bpo-43567 works as expected. Really clean implementation!
Thanks!

[gvanrossum]

Jiaxin: "Resolution: works for me" is meant for the triager to indicate that there is no bug, or at least that the repro given in the bug report doesn't trigger the bug in the triager's environment. It does *not* mean that a patch works for the submitter of the issue. But thanks for letting us know that Steve's patch works for you! :-)

[zooba]

New changeset 7482838 by Steve Dower in branch 'master':
bpo-43567: Improved generated code refresh on Windows (GH-25120)
7482838

[zooba]

We're updating all files now, and doing it automatically on build (or manually with --regen).