bpo-36500: Enable Windows users to regen grammar, opcodes, tokens and symbols from Visual Studio by tonybaloney · Pull Request #12654 · python/cpython (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation26 Commits22 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Adds an additional project to the PCbuild.sln that runs the equivalent commands that are in the regen-all stage in the Makefile.

Currently implements:

regen-grammar
regen-opcode
regen-token
regen-keyword
regen-symbol
regen-ast

Has a custom clean stage to delete intermediate files.

Uses the generated python.exe from pythoncore, and so marks pythoncore as a depedency.

The solution file does not mark this project as to be built in the Debug or Release profiles (same as freezeimportlib)

TODO:

Remove references to new project versions

https://bugs.python.org/issue36500

…pythoncore as uses python*.exe binary to run the new Pgen module

Added regen-keyword and regen-symbol equivalent stages.

tonybaloney changed the title~~bpo-36500: Enable Windows users to regen parser tables from Visual Studio~~ bpo-36500: Enable Windows users to regen grammar, tokens and symbols from Visual Studio

Added regen-opcode, regen-token. Only regen-ast left (if required?)

tonybaloney changed the title~~bpo-36500: Enable Windows users to regen grammar, tokens and symbols from Visual Studio~~ bpo-36500: Enable Windows users to regen grammar, opcodes, tokens and symbols from Visual Studio

Added regen-ast and the ASDL/ast related files

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now this can't be a part of every build because it touches too many files. That's okay, but then I'd like a PCbuild/regen.bat script for launching it from the command line.

Alternatively, this could regenerate everything but only update source files that have changed (and break the build at that point), and otherwise leave everything clean. Then we could make it part of the regular build and wouldn't need the current freeze_importlib project. But it's got to not touch anything that doesn't need touching and not pollute the source directories.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't sufficient for detecting files that were regenerated without changing. Look in the importlib one for the contents check.

(If you want an excuse to learn about target batching in MSBuild - which is ridiculously powerful - you might consider having one target for doing the optional-copy and fail if updated step, then pass it all the target files in an ItemGroup with the generated file as metadata.)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zooba It copies them regardless in the same way that the Makefile does, which is why this project is an optional configuration and not part of the Debug or Release build dependencies.
Would you like me to add this check and make regen.vcxproj build automatically?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that'll be most reliable.

If you add all the source files as items in the project then it should skip rebuilding if none of them have changed.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried the target batching, it doesn't seem to be able to determine any better than this revision whether the contents have changed. The docs say that it uses timestamps on the files to determine (which won't work in this case)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you'll need to do content comparisons. See what the current freezeimportlib project does.

@zooba please could you review, instead of having regen.bat, I've added a --regen option to build.bat so it shares the same configuration flags.
Have made the other changes you requested and looked into target batching (see comment)

Actually, I’m going to remove the project from the solution and remove the errors and turn them into warnings

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not keen on the relative paths here. We have $(PySourcePath) for the root of the sources.

@zooba please can you review with the changes you requested.

…er must have built the binaries first before using the process. Also fix a leading '.' in the vcxproj

Modified the build.bat so that --regen will run after the primary build, it will issue warnings about the source files that it has updated:

C:\Users\anthonyshaw\source\repos\cpython\PCbuild>if not ERRORLEVEL 1 "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Current\Bin\msbuild.exe" "C:\Users\anthonyshaw\source\repos\cpython\PCbuild\regen.vcxproj" /t:Build /m /nologo /v:m /p:IncludeExternals=true /p:Configuration=Release /p:Platform=Win32 /p:UseTestMarker= /p:GIT="C:\Program Files\Git\cmd\git.exe"
  Killing any running python.exe instances...
  Getting build info from "C:\Program Files\Git\cmd\git.exe"
  Building heads/pcbuildregen-dirty:78639928d0 pcbuildregen
  pythoncore.vcxproj -> C:\Users\anthonyshaw\source\repos\cpython\PCbuild\win32\python39.dll
C:\Users\anthonyshaw\source\repos\cpython\PCbuild\regen.vcxproj(167,5): warning : Grammar updated. You will need to reb
uild pythoncore to see the changes.
C:\Users\anthonyshaw\source\repos\cpython\PCbuild\regen.vcxproj(175,5): warning : Python-ast.h updated. You will need t
o rebuild pythoncore to see the changes.
C:\Users\anthonyshaw\source\repos\cpython\PCbuild\regen.vcxproj(183,5): warning : Python-ast.c updated. You will need t
o rebuild pythoncore to see the changes.
  C:\Users\anthonyshaw\source\repos\cpython\PCbuild\obj\39win32_Release\regen\opcode.h regenerated from C:\Users\anthon
  yshaw\source\repos\cpython\Lib\opcode.py
C:\Users\anthonyshaw\source\repos\cpython\PCbuild\regen.vcxproj(191,5): warning : Opcodes updated. You will need to reb
uild pythoncore to see the changes.
C:\Users\anthonyshaw\source\repos\cpython\PCbuild\regen.vcxproj(209,5): warning : Keywords updated. You will need to re
build pythoncore to see the changes.

Co-Authored-By: Kyle Stanley aeros167@gmail.com

@zooba please could you look at this for going into 3.9-master?

shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request