classification
Title: [Windows] Warnings in elementtree due to new expat
Type: compile error Stage: resolved
Components: Extension Modules, Windows Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Segev Finer, larry, ned.deily, paul.moore, steve.dower, tim.golden, vstinner, zach.ware
Priority: normal Keywords:

Created on 2017-06-21 22:18 by Segev Finer, last changed 2019-05-10 18:13 by ned.deily. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 2319 merged Segev Finer, 2017-06-21 22:27
PR 2348 merged vstinner, 2017-06-23 07:46
PR 2349 merged vstinner, 2017-06-23 08:11
PR 2350 merged vstinner, 2017-06-23 08:11
PR 2368 merged vstinner, 2017-06-23 21:26
PR 2375 merged Segev Finer, 2017-06-24 10:31
PR 2570 merged Segev Finer, 2017-07-04 18:04
PR 2571 merged Segev Finer, 2017-07-04 18:05
PR 2572 merged Segev Finer, 2017-07-04 18:16
Messages (30)
msg296590 - (view) Author: Segev Finer (Segev Finer) * Date: 2017-06-21 22:18
We are getting:

Warning	C4005	'HAVE_MEMMOVE': macro redefinition	_elementtree	c:\users\segev\prj\python\cpython\modules\expat\winconfig.h	34	
Warning	C4267	'=': conversion from 'size_t' to 'unsigned char', possible loss of data	_elementtree	c:\users\segev\prj\python\cpython\modules\expat\siphash.h	316	
Warning	C4996	'getenv': This function or variable may be unsafe. Consider using _dupenv_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.	_elementtree	C:\Users\Segev\prj\python\cpython\Modules\expat\xmlparse.c	796	
Warning	C4005	'HAVE_MEMMOVE': macro redefinition	_elementtree	c:\users\segev\prj\python\cpython\modules\expat\winconfig.h	34	
Warning	C4005	'HAVE_MEMMOVE': macro redefinition	_elementtree	c:\users\segev\prj\python\cpython\modules\expat\winconfig.h	34

And in 64-bit:

c:\users\segev\prj\python\cpython\modules\expat\siphash.h(201): warning C4244: 'initializing': conversion from '__int64' to 'char', possible loss of data

I'm not sure how many Python versions this affects.
msg296688 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-23 08:09
New changeset c8fb58bd7917151e63398587a7fc2126db7c26de by Victor Stinner in branch 'master':
bpo-30726: PCbuild _elementtree: remove duplicate defines (#2348)
https://github.com/python/cpython/commit/c8fb58bd7917151e63398587a7fc2126db7c26de
msg296690 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-23 08:13
My commit c8fb58bd7917151e63398587a7fc2126db7c26de (co-written with Jeremy Kloth) fixes the "macro redefinition".

There are still warnings in the siphash code, but I suggest to report them upstream, and *then* propose to cherry-pick fixes from libexpat (as I did for Visual Studio 2008 support in Python 2.7 when I upgraded libexpat to 2.2.1, but I was lucky, the fix was already made in libexpat).
msg296691 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-23 08:23
Oh... I'm sorry Segev Finer, I didn't see that you proposed a PR :-(
msg296693 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-23 08:42
For siphash.h warnings, I created a PR on libexpat:
https://github.com/libexpat/libexpat/pull/58
msg296694 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-23 08:49
> Oh... I'm sorry Segev Finer, I didn't see that you proposed a PR :-(

Please rebase and rewrite your PR to just add _CRT_SECURE_NO_WARNINGS. Once merged, I will include this change to my 3.6 and 3.5 backports.
msg296696 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-23 10:45
New changeset 87c65550730a8f85ce339ba197bce4fb7e836619 by Victor Stinner (Segev Finer) in branch 'master':
bpo-30726: Fix elementtree warnings on Windows due to expat upgrade (#2319)
https://github.com/python/cpython/commit/87c65550730a8f85ce339ba197bce4fb7e836619
msg296712 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-23 13:14
New changeset f42ce179c8aaa7e211ac4123c58fa3dd9a452004 by Victor Stinner in branch '3.5':
[3.5] bpo-30726: PCbuild _elementtree: remove duplicate defines (#2348) (#2350)
https://github.com/python/cpython/commit/f42ce179c8aaa7e211ac4123c58fa3dd9a452004
msg296715 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-23 13:21
New changeset d32a05953130fb5cc2d3c0c9fcb20ad0859353f3 by Victor Stinner in branch '3.6':
[3.6] bpo-30726: PCbuild _elementtree: remove duplicate defines (#2348) (#2349)
https://github.com/python/cpython/commit/d32a05953130fb5cc2d3c0c9fcb20ad0859353f3
msg296744 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-23 23:08
New changeset 8f525882fa43209d52afdb99753de2f5111d7433 by Victor Stinner in branch 'master':
bpo-30726: expat: Fix compiler warnings on Windows 64-bit (#2368)
https://github.com/python/cpython/commit/8f525882fa43209d52afdb99753de2f5111d7433
msg296762 - (view) Author: Segev Finer (Segev Finer) * Date: 2017-06-24 10:39
It seems that the issue repeats in pyexpat.vcxproj. I submitted a PR for that one as well.
msg297236 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2017-06-28 22:18
New changeset 7526cadd64566725ffc56071a7208828a46ddbd8 by Steve Dower (Segev Finer) in branch 'master':
bpo-30726: Also fix pyexpat.vcxproj (#2375)
https://github.com/python/cpython/commit/7526cadd64566725ffc56071a7208828a46ddbd8
msg297287 - (view) Author: Segev Finer (Segev Finer) * Date: 2017-06-29 17:36
All that remains is back porting https://github.com/python/cpython/pull/2375 and than this issue is fixed.
msg297290 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2017-06-29 17:55
How far back does it need to be ported? Just to 3.5?
msg297299 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-29 19:44
> How far back does it need to be ported? Just to 3.5?

Up to 2.7, expat 2.2.1 was updated to 2.7 as well. I even proposed a PR for 3.3 and 3.4 (not merged yet). But since this issue only fixes a warning, I don't think that it's worth it to backport it to 3.3 and 3.4.
msg297305 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2017-06-29 20:04
It doesn't even fix a warning - only suppresses it. Since there is no behavior change at all, I'm not inclined to backport any further than is trivial.
msg297591 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-07-03 14:20
Segev Finer: Would you mind to backport PR 2375 to 3.6 and 3.5 branches?

Maybe also to 2.7, at least in PCbuild, maybe also PC/VS9.0/.
msg297680 - (view) Author: Segev Finer (Segev Finer) * Date: 2017-07-04 18:07
Python 2.7 diverged https://github.com/python/cpython/commit/ab3b0ade505ce07a3d5ec4fbc991a154242732e6. It's only missing _CRT_SECURE_NO_WARNINGS. :P
msg297682 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-07-04 18:38
New changeset d02c8416fe1b29b3322004b73133bf6c8a2e353a by Victor Stinner (Segev Finer) in branch '3.6':
[3.6] bpo-30726: Also fix pyexpat.vcxproj (GH-2375) (#2570)
https://github.com/python/cpython/commit/d02c8416fe1b29b3322004b73133bf6c8a2e353a
msg297683 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-07-04 18:39
New changeset 320ee67f981f766ff55c55ef87d5ef17ce297824 by Victor Stinner (Segev Finer) in branch '3.5':
[3.5] bpo-30726: Also fix pyexpat.vcxproj (GH-2375) (#2571)
https://github.com/python/cpython/commit/320ee67f981f766ff55c55ef87d5ef17ce297824
msg297684 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-07-04 18:40
New changeset d0ae4be443332d63fafb304944842fbaece734a3 by Victor Stinner (Segev Finer) in branch '2.7':
bpo-30726: Add _CRT_SECURE_NO_WARNINGS to _elementtree and pyexpat projects (#2572)
https://github.com/python/cpython/commit/d0ae4be443332d63fafb304944842fbaece734a3
msg297686 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-07-04 20:49
Thanks Segev Finer for backports.
msg297687 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-07-04 21:10
> It doesn't even fix a warning - only suppresses it. Since there is no behavior change at all, I'm not inclined to backport any further than is trivial.

I like decreasing the number of warnings. It helps to detect new warnings, and so to prevent bugs.
msg297688 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2017-07-04 21:57
> I like decreasing the number of warnings. It helps to detect new warnings, and so to prevent bugs.

The approach used to suppress this warning will also suppress other warnings that might be fixable. Hard to say it's definitely a good thing in this case, though in general I agree that selectively suppressing warnings that cannot be fixed is a good thing.
msg297689 - (view) Author: Segev Finer (Segev Finer) * Date: 2017-07-04 22:45
> The approach used to suppress this warning will also suppress other warnings that might be fixable. Hard to say it's definitely a good thing in this case, though in general I agree that selectively suppressing warnings that cannot be fixed is a good thing.

Most secure CRT warnings are a bit useless in portable code since you are unlikely to really use strcpy_s and friends...
msg297705 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-07-05 06:44
Steve Dower: "The approach used to suppress this warning will also suppress other warnings that might be fixable. Hard to say it's definitely a good thing in this case, though in general I agree that selectively suppressing warnings that cannot be fixed is a good thing."

First of all, Modules/expat/ is a copy of https://github.com/libexpat/libexpat/. In the past, it contained significant patches. Today, it almost contains zero patch, the major addition is the pyexpatns.h file:

https://github.com/python/cpython/blob/master/Modules/expat/pyexpatns.h

If someone considers that the following warning deserves a fix, please report it upstream:

Warning	C4996	'getenv': This function or variable may be unsafe. Consider using _dupenv_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.	_elementtree	C:\Users\Segev\prj\python\cpython\Modules\expat\xmlparse.c	796	

I read the code, and I don't think that it deserves a fix. Using non-portable function _dupenv_s() makes the code a little bit more complex for a little benefit.

I agree that if libexpat is upgraded again, suppressing warnings can hide a future real bug. But right now, we embed a copy of libexpat 2.1.1 and I don't want to modify our downstream copy.
msg297761 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2017-07-05 14:11
I hope you notice I'm not against this particular change. I'm just voicing a general belief that suppressing an entire category of warnings for a whole project is not necessarily an improvement.

In future, and when applied to our own project files, expect me to push back against this kind of fix.
msg297937 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2017-07-08 04:51
New changeset 5777e79ecbd1f2adf36456e09f210608ee221691 by Ned Deily (Victor Stinner) in branch '3.6':
[3.6] bpo-30726: PCbuild _elementtree: remove duplicate defines (#2348) (#2349)
https://github.com/python/cpython/commit/5777e79ecbd1f2adf36456e09f210608ee221691

New changeset b6012f982fabed6029d7e2db2a509a8b28b4f6e1 by Ned Deily (Segev Finer) in branch '3.6':
[3.6] bpo-30726: Also fix pyexpat.vcxproj (GH-2375) (#2570)
https://github.com/python/cpython/commit/b6012f982fabed6029d7e2db2a509a8b28b4f6e1
msg298207 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2017-07-12 12:41
New changeset 71572bbe82aa0836c036d44d41c8269ba6a321be by larryhastings (Victor Stinner) in branch '3.4':
[3.4] bpo-29591, bpo-30694: Upgrade Modules/expat to libexpat 2.2.1 (#2164) (#2203)
https://github.com/python/cpython/commit/71572bbe82aa0836c036d44d41c8269ba6a321be
msg298426 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2017-07-16 08:48
New changeset ab90986600ba7dea2aa41e5c1773791070725453 by Ned Deily (Victor Stinner) in branch '3.3':
[3.3] bpo-29591, bpo-30694: Upgrade Modules/expat to libexpat 2.2.1 (#2164) (#2204)
https://github.com/python/cpython/commit/ab90986600ba7dea2aa41e5c1773791070725453
History
Date User Action Args
2019-05-10 18:13:34ned.deilysetmessages: - msg342097
2019-05-10 17:36:39ned.deilysetmessages: + msg342097
2017-07-16 08:48:07ned.deilysetmessages: + msg298426
2017-07-12 12:41:36larrysetnosy: + larry
messages: + msg298207
2017-07-08 04:51:41ned.deilysetnosy: + ned.deily
messages: + msg297937
2017-07-05 14:11:26steve.dowersetmessages: + msg297761
2017-07-05 06:44:55vstinnersetmessages: + msg297705
2017-07-04 22:45:30Segev Finersetmessages: + msg297689
2017-07-04 21:57:14steve.dowersetmessages: + msg297688
2017-07-04 21:10:08vstinnersetmessages: + msg297687
2017-07-04 21:08:57vstinnersetstatus: open -> closed
resolution: fixed
stage: backport needed -> resolved
2017-07-04 20:49:59vstinnersetmessages: + msg297686
2017-07-04 18:40:06vstinnersetmessages: + msg297684
2017-07-04 18:39:31vstinnersetmessages: + msg297683
2017-07-04 18:38:49vstinnersetmessages: + msg297682
2017-07-04 18:16:34Segev Finersetpull_requests: + pull_request2643
2017-07-04 18:07:57Segev Finersetmessages: + msg297680
2017-07-04 18:05:08Segev Finersetpull_requests: + pull_request2642
2017-07-04 18:04:59Segev Finersetpull_requests: + pull_request2641
2017-07-03 14:20:44vstinnersetmessages: + msg297591
2017-06-29 20:04:51steve.dowersetmessages: + msg297305
2017-06-29 19:45:00vstinnersetversions: + Python 2.7
2017-06-29 19:44:54vstinnersetmessages: + msg297299
2017-06-29 17:55:05steve.dowersetmessages: + msg297290
stage: backport needed
2017-06-29 17:36:11Segev Finersetmessages: + msg297287
2017-06-28 22:18:30steve.dowersetmessages: + msg297236
2017-06-24 11:01:11Segev Finersetversions: + Python 3.5, Python 3.6
2017-06-24 10:39:54Segev Finersetmessages: + msg296762
2017-06-24 10:31:39Segev Finersetpull_requests: + pull_request2422
2017-06-23 23:08:59vstinnersetmessages: + msg296744
2017-06-23 21:26:15vstinnersetpull_requests: + pull_request2418
2017-06-23 13:21:35vstinnersetmessages: + msg296715
2017-06-23 13:14:20vstinnersetmessages: + msg296712
2017-06-23 10:45:03vstinnersetmessages: + msg296696
2017-06-23 08:49:57vstinnersetmessages: + msg296694
2017-06-23 08:42:31vstinnersetmessages: + msg296693
2017-06-23 08:23:14vstinnersetmessages: + msg296691
2017-06-23 08:13:26vstinnersetmessages: + msg296690
2017-06-23 08:11:33vstinnersetpull_requests: + pull_request2395
2017-06-23 08:11:19vstinnersetpull_requests: + pull_request2393
2017-06-23 08:09:37vstinnersetmessages: + msg296688
2017-06-23 07:46:56vstinnersetpull_requests: + pull_request2391
2017-06-22 04:31:12louielusetnosy: + vstinner
2017-06-22 04:19:58Segev Finersettype: compile error
2017-06-21 22:27:27Segev Finersetpull_requests: + pull_request2369
2017-06-21 22:18:03Segev Finercreate