Skip to content

[3.4] bpo-31170: Fix inclusion of expat in Windows build projects #3785

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 5 commits into from
Nov 29, 2017
Merged

[3.4] bpo-31170: Fix inclusion of expat in Windows build projects #3785

merged 5 commits into from
Nov 29, 2017

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Sep 27, 2017

Co-Authored-By: Steve Dower <steve.dower@microsoft.com>
@vstinner vstinner requested a review from zooba September 27, 2017 08:51
@vstinner vstinner changed the title [3.3] bpo-31170: Fix inclusion of expat in Windows build projects [3.4] bpo-31170: Fix inclusion of expat in Windows build projects Sep 27, 2017
@vstinner
Copy link
Member Author

I wanted to test my PR on Windows, but VS 2015 fails to build Python 3.4, and my VS 2010 license is not more valid.

My PR #3353 upgraded expat to 2.2.4, @larryhastings merged it 3 days ago.

Problem: AppVeyor didn't run on PR #3353, whereas on this PR, AppVeyor runs and fails on building expat.

Travis CI and AppVeyor were enabled on Python 3.4 by the PR #2475. This one was merged at 22 Jul 2017.

The standard header stdbool.h is not available
with old Visual Studio compilers

Cherry-picked from libexpat commit b4b89c2ab0cc5325a41360c25ef9d2ccbe617e5c.

expat: Add artificial scopes in xmltok.c utf8_toUtf8() to fix c89 compilation.

Cherry-picked from libexpat commit e0b290eb3d8f4c4b45137a7d7f4f8db812145bd2
Remove the following defines:

* BYTEORDER=1234
* HAVE_MEMMOVE
* USE_PYEXPAT_CAPI
* XML_CONTEXT_BYTES=1024
* XML_DTD
* XML_NS
* XML_STATIC
@vstinner
Copy link
Member Author

Ok. I found a VS 2010 license in my MSDN subscription. I fixed C defines in PCbuild/ and PC/VS9.0/ projects (_elementtree and pyexpat). I also cherry-picked the 2 fixes for Visual Studio older than 2015 from libexpat, I used code from Python 2.7 (already merged).

I built _elementtree and pyexpat projects in Debug|x64 mode: they build correctly without any warning.

@vstinner vstinner requested a review from zware September 27, 2017 13:34
@vstinner
Copy link
Member Author

Cool, CIs are happy (green). AppVeyor succeeded to build expat and to run the test suite ("351 tests OK.") :-)

@vstinner
Copy link
Member Author

@larryhastings: Basically, this PR makes Python 3.4 compilable again on Windows.

Copy link
Member

@zware zware left a comment

Choose a reason for hiding this comment

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

This looks fine to me, though I haven't actually tested it.

FTR, we only care about PC/VS9.0/ on 2.7; in any other branch it may or may not work at all.

align_limit_to_full_utf8_characters(*fromP, &fromLim);
if (fromLim < fromLimBefore) {
input_incomplete = true;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize I'm nit-picking, but: why were these two paragraphs of code (this section, and the next starting at 425) indented inside blocks? Is this change really a security fix?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the second commit of this PR and it was cherry-picked from upstream.

Without this code, compilation fails on Visual Studio 2010 which uses C89, not C99. So Python 3.4 users on Windows cannot get the latest Python 3.4 fixes.

@vstinner
Copy link
Member Author

This PR isn't directly a security fix. It only allows to build Python 3.4 with latest security fixes on Windows.

@larryhastings
Copy link
Contributor

Okay, but is that change actually relevant?

Please tell me how it can't build on Windows right now, but adding curly braces and indenting allows it to build. It looks like someone added curly braces and indented a block as an experiment during debugging, then forgot to undo that change before creating the PR.

@vstinner
Copy link
Member Author

Please tell me how it can't build on Windows right now,

ISO C89 disallows the declaration of variables "in the middle of the code". "int a; a = 1; int b;" in illegal in C89.

but adding curly braces and indenting allows it to build.

"int a; a = 1; { int b; }" is legal in C89.

It looks like someone added curly braces and indented a block as an experiment during debugging, then forgot to undo that change before creating the PR.

I cherry-picked the commit from upstream:
libexpat/libexpat@e0b290e

@larryhastings
Copy link
Contributor

Okay, I'll buy that.

@larryhastings larryhastings merged commit 8b11e8d into python:3.4 Nov 29, 2017
@vstinner vstinner deleted the expat_pcbuild34 branch May 29, 2018 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants