Skip to content
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

bpo-36500: Enable Windows users to regen grammar, opcodes, tokens and symbols from Visual Studio #12654

Merged
merged 22 commits into from
Dec 17, 2019

Conversation

tonybaloney
Copy link
Contributor

@tonybaloney tonybaloney commented Apr 2, 2019

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)

@zooba

TODO:

  • Remove references to new project versions

https://bugs.python.org/issue36500

…pythoncore as uses python*.exe binary to run the new Pgen module
@tonybaloney
Copy link
Contributor Author

Added regen-keyword and regen-symbol equivalent stages.

@tonybaloney 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 Apr 2, 2019
@tonybaloney
Copy link
Contributor Author

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

@tonybaloney 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 Apr 2, 2019
@tonybaloney
Copy link
Contributor Author

Added regen-ast and the ASDL/ast related files

Copy link
Member

@zooba zooba left a comment

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.

PCbuild/regen.vcxproj Outdated Show resolved Hide resolved
PCbuild/regen.vcxproj Outdated Show resolved Hide resolved
PCbuild/regen.vcxproj Show resolved Hide resolved
PCbuild/regen.vcxproj Outdated Show resolved Hide resolved
<!-- Regenerate Include/Python-ast.h using Parser/asdl_c.py -h -->
<Exec Command="&quot;$(OutDir)python$(PyDebugExt).exe&quot; &quot;..\Parser\asdl_c.py&quot; -h &quot;..\Include\Python-ast.h.new&quot; &quot;..\Parser\Python.asdl&quot;" />
<Copy SourceFiles="..\Include\Python-ast.h.new" DestinationFiles="..\Include\Python-ast.h">
<Output TaskParameter="CopiedFiles" ItemName="_UpdatedH" />
Copy link
Member

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.)

Copy link
Contributor Author

@tonybaloney tonybaloney Apr 2, 2019

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?

Copy link
Member

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.

Copy link
Contributor Author

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)

Copy link
Member

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.

@tonybaloney
Copy link
Contributor Author

@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)

@tonybaloney
Copy link
Contributor Author

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

</Target>
<Target Name="_RegenOpcodes" AfterTargets="_RegenAST_C">
<!-- Regenerate Include/opcode.h from Lib/opcode.py using Tools/scripts/generate_opcode_h.py-->
<Exec Command="&quot;$(PythonExe)&quot; ..\Tools\scripts\generate_opcode_h.py &quot;..\Lib\opcode.py&quot; &quot;$(IntDir)opcode.h&quot;" />
Copy link
Member

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.

@tonybaloney
Copy link
Contributor Author

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

@aeros aeros added the type-feature A feature request or enhancement label Aug 22, 2019
…er must have built the binaries first before using the process. Also fix a leading '.' in the vcxproj
@tonybaloney
Copy link
Contributor Author

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.

@tonybaloney
Copy link
Contributor Author

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

@zooba zooba merged commit 9e36589 into python:master Dec 17, 2019
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
@tonybaloney tonybaloney deleted the pcbuildregen branch May 31, 2021 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS-windows type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants