classification
Title: regen.vcxproj cannot regenerate some necessary files
Type: compile error Stage: resolved
Components: Build, Windows Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: steve.dower Nosy List: anthony shaw, anthonypjshaw, gvanrossum, pablogsal, pjx206, steve.dower
Priority: normal Keywords: patch

Created on 2021-03-20 11:02 by pjx206, last changed 2021-04-06 22:57 by steve.dower. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 25120 merged steve.dower, 2021-03-31 18:52
Messages (22)
msg389155 - (view) Author: Jiaxin Peng (pjx206) Date: 2021-03-20 11:02
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.
msg389743 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-03-29 20:37
Steve: let's talk about this in person today.
msg389765 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-03-29 22:41
> regen.vcxproj is now not capable for the new PEG parse

Hummmm, that file should handle the peg parser already:

https://github.com/python/cpython/blob/09b90a037d18f5d4acdf1b14082e57bda78e85d3/PCbuild/regen.vcxproj#L156-L163
msg389766 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-03-29 22:43
By the way, it would be nice if someone could review my change in this project related to pycore_ast.h ;-) (commit 94faa0724f8cbae6867c491c8e465e35f4fdbfbb) I mean checking if pycore_ast.h is updated as expected.
msg389770 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-03-29 23:22
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.
msg389778 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-03-30 00:23
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?
msg389781 - (view) Author: Jiaxin Peng (pjx206) Date: 2021-03-30 00:55
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.
msg389835 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-03-30 16:05
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.
msg389841 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-03-30 17:30
@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!
msg389863 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-03-30 22:36
FWIW, could this issue recently merged by Victor be related?
bpo-43244: Remove ast.h, asdl.h, Python-ast.h headers (GH-24933)
msg389866 - (view) Author: Jiaxin Peng (pjx206) Date: 2021-03-31 00:42
`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.
msg389868 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-03-31 03:49
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.
msg389870 - (view) Author: Jiaxin Peng (pjx206) Date: 2021-03-31 04:24
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".
msg389880 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-03-31 09:33
> not having Include/Python-ast.h and Python/Python-ast.c

FYI Include/Python-ast.h was renamed to Include/internal/pycore_ast.h.
msg389924 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-03-31 17:24
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?
msg389927 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2021-03-31 18:53
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.
msg389929 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-03-31 19:18
@Jiaxin Please test steve's PR.
msg389935 - (view) Author: anthony shaw (anthony shaw) Date: 2021-03-31 22:10
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!
msg389954 - (view) Author: Jiaxin Peng (pjx206) Date: 2021-04-01 05:05
Steve's PR zooba:bpo-43567 works as expected. Really clean implementation!
Thanks!
msg390037 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-04-02 04:06
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! :-)
msg390383 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2021-04-06 22:54
New changeset 748283819043c60b1cb272c2cc9ab5b457afb03a by Steve Dower in branch 'master':
bpo-43567: Improved generated code refresh on Windows (GH-25120)
https://github.com/python/cpython/commit/748283819043c60b1cb272c2cc9ab5b457afb03a
msg390384 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2021-04-06 22:57
We're updating all files now, and doing it automatically on build (or manually with --regen).
History
Date User Action Args
2021-04-06 22:57:11steve.dowersetstatus: open -> closed
messages: + msg390384

assignee: steve.dower
resolution: fixed
stage: patch review -> resolved
2021-04-06 22:54:52steve.dowersetmessages: + msg390383
2021-04-02 04:06:20gvanrossumsetresolution: works for me -> (no value)
messages: + msg390037
2021-04-01 05:05:25pjx206setresolution: works for me
messages: + msg389954
2021-03-31 22:16:14vstinnersetnosy: - vstinner
2021-03-31 22:10:19anthony shawsetnosy: + anthony shaw
messages: + msg389935
2021-03-31 19:18:05gvanrossumsetmessages: + msg389929
2021-03-31 18:53:13steve.dowersetmessages: + msg389927
2021-03-31 18:52:19steve.dowersetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request23866
2021-03-31 17:24:11gvanrossumsetnosy: + anthonypjshaw
messages: + msg389924
2021-03-31 09:33:05vstinnersetmessages: + msg389880
2021-03-31 04:57:59gvanrossumsetnosy: + vstinner, pablogsal
2021-03-31 04:24:10pjx206setmessages: + msg389870
2021-03-31 03:49:08gvanrossumsetnosy: + steve.dower
messages: + msg389868
2021-03-31 00:42:35pjx206setnosy: + pjx206
messages: + msg389866
2021-03-31 00:37:01pjx206setnosy: - paul.moore, vstinner, tim.golden, zach.ware, steve.dower, anthonypjshaw, lys.nikolaou, pablogsal, pjx206
2021-03-30 22:36:59gvanrossumsetmessages: + msg389863
2021-03-30 17:30:45gvanrossumsetmessages: + msg389841
2021-03-30 16:05:37gvanrossumsetnosy: + paul.moore, vstinner, tim.golden, zach.ware, steve.dower, anthonypjshaw, lys.nikolaou, pablogsal
messages: + msg389835
2021-03-30 00:55:31pjx206setnosy: - paul.moore, vstinner, tim.golden, zach.ware, steve.dower, anthonypjshaw, lys.nikolaou, pablogsal
messages: + msg389781
2021-03-30 00:23:55gvanrossumsetmessages: + msg389778
2021-03-29 23:22:30gvanrossumsetmessages: + msg389770
2021-03-29 22:43:44vstinnersetnosy: + vstinner
messages: + msg389766
2021-03-29 22:41:04pablogsalsetmessages: + msg389765
2021-03-29 20:37:21gvanrossumsetmessages: + msg389743
2021-03-29 20:26:56steve.dowersetnosy: + gvanrossum, anthonypjshaw, lys.nikolaou, pablogsal
2021-03-21 20:16:14zach.waresetnosy: + paul.moore, tim.golden, zach.ware, steve.dower

components: + Windows
stage: needs patch
2021-03-20 11:02:18pjx206create