Skip to content

/cpython

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
Merged

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

merged 22 commits into from Dec 17, 2019
+243 −1

Conversation

@tonybaloney
Copy link
Contributor

tonybaloney commented Apr 2, 2019 •
edited

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
@tonybaloney
Add regen-grammar equivalent as optional build project, dependant on …
Loading status checks…
f291fb1 
…pythoncore as uses python*.exe binary to run the new Pgen module
@tonybaloney tonybaloney requested a review from python/windows-team as a code owner Apr 2, 2019
tonybaloney added 5 commits Apr 2, 2019
@tonybaloney
Downgrade solution file
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
0f4b23a
@tonybaloney
Remove win10 toolset configurations manually
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
759e241
@tonybaloney
Update pcbuild.sln
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
192f358
@tonybaloney
add regen-keywords and regen-symbols build stages
5eb7795
@tonybaloney
Merge remote-tracking branch 'upstream/pcbuildregen' into pcbuildregen
Loading status checks…
4017063
@tonybaloney

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

tonybaloney commented Apr 2, 2019

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 added 2 commits Apr 2, 2019
@tonybaloney
Copy both C and H for grammar, regen opcodes and order process
Loading status checks…
0be5632
@tonybaloney
All regen stages with comments (except regen-ast)
Loading status checks…
a2c1e0c
@tonybaloney

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

tonybaloney commented Apr 2, 2019

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 added 2 commits Apr 2, 2019
@tonybaloney
Added regen-ast in 2 stages
b52329f
@tonybaloney
Add the other artifacts to the linked files
Loading status checks…
95eaa9f
@tonybaloney

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

tonybaloney commented Apr 2, 2019

Added regen-ast and the ASDL/ast related files
@blurb-it
📜🤖 Added by blurb_it.
Loading status checks…
1396fd9
@zooba
zooba reviewed Apr 2, 2019
View changes
Copy link
Member

zooba left a comment

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
PCbuild/regen.vcxproj
<!-- 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" />

This comment has been minimized.

Sign in to view
Copy link
@zooba

zooba Apr 2, 2019

Member

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

This comment has been minimized.

Sign in to view
Copy link
@tonybaloney

tonybaloney Apr 2, 2019 •
edited

Author Contributor

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

This comment has been minimized.

Sign in to view
Copy link
@zooba

zooba Apr 12, 2019

Member

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.

This comment has been minimized.

Sign in to view
Copy link
@tonybaloney

tonybaloney May 9, 2019

Author Contributor

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)

This comment has been minimized.

Sign in to view
Copy link
@zooba

zooba May 15, 2019

Member

Yeah, you'll need to do content comparisons. See what the current freezeimportlib project does.
tonybaloney added 3 commits May 9, 2019
@tonybaloney
Update with feedback from Steve
80d2d9a
@tonybaloney
Error on regenerated files in Release configuration
Loading status checks…
45c7999
@tonybaloney
Add regen to the build.bat
Loading status checks…
2162db2
@tonybaloney

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

tonybaloney commented May 9, 2019

@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

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

tonybaloney commented May 9, 2019

Actually, I’m going to remove the project from the solution and remove the errors and turn them into warnings
@tonybaloney
Make regen a seperate project
Loading status checks…
89246a4
@zooba
zooba reviewed May 15, 2019
View changes
PCbuild/regen.vcxproj Outdated
</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;" />

This comment has been minimized.

Sign in to view
Copy link
@zooba

zooba May 15, 2019

Member

I'm not keen on the relative paths here. We have $(PySourcePath) for the root of the sources.
tonybaloney added 2 commits Aug 21, 2019
@tonybaloney
Merge branch 'master' into pcbuildregen
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
546cea5
@tonybaloney
Swap rel paths with $(PySourcePath)
Verified
This commit was signed with a verified signature.
tonybaloney Anthony Shaw
GPG key ID: A5B5836F7895863C Learn about signing commits
Loading status checks…
eb87863
@tonybaloney

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

tonybaloney commented Aug 21, 2019

@zooba please can you review with the changes you requested.
tonybaloney added 2 commits Aug 21, 2019
@tonybaloney
Bind project path correctly
Verified
This commit was signed with a verified signature.
tonybaloney Anthony Shaw
GPG key ID: A5B5836F7895863C Learn about signing commits
Loading status checks…
9fca9db
@tonybaloney
merge upstream
Loading status checks…
7863992
@aeros aeros added the OS-windows label Aug 22, 2019
@aeros
aeros reviewed Aug 22, 2019
View changes
Misc/NEWS.d/next/Build/2019-04-02-01-59-26.bpo-36500.fyG6_U.rst Outdated Show resolved Hide resolved
@tonybaloney
Change the regen to be a stage after the primary build so that the us…
Loading status checks…
370fcba 
…er must have built the binaries first before using the process. Also fix a leading '.' in the vcxproj
@tonybaloney

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

tonybaloney commented Aug 22, 2019

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 and others added 2 commits Aug 22, 2019
@tonybaloney @aeros
Update Misc/NEWS.d/next/Build/2019-04-02-01-59-26.bpo-36500.fyG6_U.rst
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
8f93907 
Co-Authored-By: Kyle Stanley <aeros167@gmail.com>
@tonybaloney
Update 2019-04-02-01-59-26.bpo-36500.fyG6_U.rst
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
0a6ab75
@tonybaloney

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

tonybaloney commented Oct 24, 2019

@zooba please could you look at this for going into 3.9-master?
@zooba zooba merged commit 9e36589 into python:master Dec 17, 2019
5 checks passed
5 checks passed
Azure Pipelines PR #20190822.13 succeeded
Details
bedevere/issue-number Issue number 36500 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zooba zooba

@aeros aeros

Assignees
No one assigned
Labels
CLA signed OS-windows type-enhancement
Projects
None yet
Milestone
No milestone
5 participants
@tonybaloney @zooba @aeros @the-knights-who-say-ni @bedevere-bot
You can’t perform that action at this time.