Skip to content

bpo-29591: [2.7] Update VS project files #2310

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

Merged
merged 1 commit into from
Jun 21, 2017

Conversation

jkloth
Copy link
Contributor

@jkloth jkloth commented Jun 21, 2017

Updates the Visual Studio project files to fix regressions due to Expat 2.2.0

  • Silences new warnings when building with MSBuild
  • Fixes #include error when building with vcbuild

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Can you please elaborate the commit message to list removed define and explain that they are now already defined in Modules/expat/winconfig.h?

@@ -12,7 +12,7 @@
InlineFunctionExpansion="1"
EnableIntrinsicFunctions="true"
AdditionalIncludeDirectories="..\..\Include; ..\..\PC"
PreprocessorDefinitions="_WIN32"
PreprocessorDefinitions="_WIN32;WIN32"
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 sure that this change is still needed with expat 2.2.1. Would you mind to check please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this is unrelated to just Expat. It brings the VS9.0 project files inline with the MSBuild ones. Ever wonder why the Expat change "just worked" on the other releases? ;)

Copy link
Member

Choose a reason for hiding this comment

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

Ok fine. I'm just surprised that we have to define it manualy. It should already be defined by <windows.h> no?

@vstinner
Copy link
Member

The overall change LGTM, except of my (minor) comments. Can you please rebase it since I just merged my expat 2.2.1 change?

@jkloth jkloth force-pushed the issue29591-expat-2.2.0 branch from 6782ae7 to 80aa260 Compare June 21, 2017 17:55
* Silence warnings caused by duplicated defines from Modules\expat\winconfig.h
* Add WIN32 define to VS9.0 project files to match MSBuild project files.
@jkloth jkloth force-pushed the issue29591-expat-2.2.0 branch from 80aa260 to 153dfe6 Compare June 21, 2017 18:00
@jkloth
Copy link
Contributor Author

jkloth commented Jun 21, 2017

Rebased and modified commit message.

@vstinner
Copy link
Member

The ch1nge looks safe and should at least fix the HAVE_MEMMOVE warning. Thanks.

@vstinner vstinner merged commit ab3b0ad into python:2.7 Jun 21, 2017
@jkloth jkloth deleted the issue29591-expat-2.2.0 branch December 11, 2018 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants