classification
Title: backport of #32305 causes regressions in various packages
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: barry, brett.cannon, doko, eric.smith, ncoghlan, ned.deily
Priority: Keywords: 3.6regression, patch

Created on 2018-02-19 09:03 by doko, last changed 2018-03-28 18:27 by ned.deily. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 5911 merged barry, 2018-02-26 18:24
PR 6276 merged ned.deily, 2018-03-28 05:39
PR 6277 merged miss-islington, 2018-03-28 05:57
PR 6278 merged miss-islington, 2018-03-28 05:58
Messages (14)
msg312344 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2018-02-19 09:03
The backport of issue #32305 causes regressions in several packaged namespace packages:

https://bugs.debian.org/890621
https://bugs.debian.org/890754

while the change is intended, is it appropriate to backport it to 3.6?  Please could you have a look, you might still have an appropriate chroot laying around ;)
msg312351 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2018-02-19 13:41
Both of those upstreams should be using `if getattr(module, '__file__', None)` instead.  The old behavior was an undocumented quirk.
msg312358 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-02-19 18:27
Now that we know that this change *does* break some existing code, I think it is worth having that talk as mentioned in PR 5481:

"I suppose it's possible that this will break existing code, but I'd argue that because current behavior runs counter to the documentation and makes no sense given the inconsistencies, it is better to fix them. I propose this change be applied to 3.7 and 3.6, although if you, my friendly reviewer, disagrees about 3.6, we can talk about it!"

The question I have is: is the problem the backport is trying to fix severe enough to cause package regressions in the middle of a maintenance release cycle?  Sure, the third-party packages should be fixed and such a change is fine for 3.7 but we also kinda promise that installing a maintenance release should be painless.  I don't know what is the right answer and I don't want to spend a lot of time on this but I'd like to get a bit more input on this before we go ahead with releasing it in 3.6.5.
msg312366 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2018-02-19 20:19
I'm personally fine if the change gets reverted. I don't think the odd behaviour is severe enough to justify breaking projects in a maintenance release. Besides, they will learn soon enough about their code breaking in Python 3.7.
msg312397 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-02-20 03:57
+1 from me for making the change 3.7.0+ only - 3.6 isn't doing the right thing, but given folks are relying on it doing the wrong thing, then let's leave it alone given where it is in its lifecycle.
msg312839 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-02-25 19:11
OK, I agree with Brett and Nick.  Barry, are you OK with reverting this change for 3.6?  If so, can you do the honors?
msg312844 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2018-02-25 19:21
I'd personally prefer to keep the fix (I ran into some problems w/3.6), but I'll defer to the RM.  I'll revert the change for 3.6, but I want to test it with importlib_resources first, since I'll probably have to spin a new release of that package too.
msg312845 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-02-25 19:29
Thanks, Barry!  I don't see that either action is ideal but I am concerned about breaking third-party packages and 2 (already known) breakages is worrisome.  And thanks, @doko, for bringing the matter up.
msg314131 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2018-03-20 06:40
reopening.

Lib/test/libregrtest/setup.py still needs fixing at least on 3.6, I didn't check the trunk.

libregrtest/setup.py:

60c60
<         if hasattr(module, '__file__'):
---
>         if getattr(module, '__file__', None):
msg314132 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2018-03-20 06:40
reopening.

Lib/test/libregrtest/setup.py still needs fixing at least on 3.6, I didn't check the trunk.

libregrtest/setup.py:

60c60
<         if hasattr(module, '__file__'):
---
>         if getattr(module, '__file__', None):
msg314564 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-03-28 05:57
New changeset e52ac045972a4f75d7f52e4ee0d6de128259134d by Ned Deily in branch 'master':
bpo-32872: Avoid regrtest compatibility issue with namespace packages. (GH-6276)
https://github.com/python/cpython/commit/e52ac045972a4f75d7f52e4ee0d6de128259134d
msg314565 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-03-28 06:39
New changeset d2d5bd8bc4f58b572702d572dc8491f0a50144e6 by Ned Deily (Miss Islington (bot)) in branch '3.7':
bpo-32872: Avoid regrtest compatibility issue with namespace packages. (GH-6276) (#6277)
https://github.com/python/cpython/commit/d2d5bd8bc4f58b572702d572dc8491f0a50144e6
msg314566 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-03-28 06:43
New changeset a93662cf8fb98e41f2b7e7c032b680eee834d290 by Ned Deily (Miss Islington (bot)) in branch '3.6':
bpo-32872: Avoid regrtest compatibility issue with namespace packages. (GH-6276) (GH-6278)
https://github.com/python/cpython/commit/a93662cf8fb98e41f2b7e7c032b680eee834d290
msg314617 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-03-28 18:27
New changeset 7f554c536cfe2e66a1ff0a36c8896040be1e5aee by Ned Deily (Miss Islington (bot)) in branch '3.6':
bpo-32872: Avoid regrtest compatibility issue with namespace packages. (GH-6276) (GH-6278)
https://github.com/python/cpython/commit/7f554c536cfe2e66a1ff0a36c8896040be1e5aee
History
Date User Action Args
2018-03-28 18:27:17ned.deilysetmessages: + msg314617
2018-03-28 06:46:54ned.deilysetpriority: release blocker ->
status: open -> closed
type: crash ->
resolution: fixed
stage: patch review -> resolved
2018-03-28 06:43:54ned.deilysetmessages: + msg314566
2018-03-28 06:39:21ned.deilysetmessages: + msg314565
2018-03-28 05:58:24miss-islingtonsetpull_requests: + pull_request6005
2018-03-28 05:57:43miss-islingtonsetpull_requests: + pull_request6004
2018-03-28 05:57:15ned.deilysetmessages: + msg314564
2018-03-28 05:39:18ned.deilysetpull_requests: + pull_request6003
2018-03-20 06:40:57dokosetstatus: open

messages: + msg314132
stage: patch review
2018-03-20 06:40:26dokosetstatus: closed -> (no value)
resolution: fixed -> (no value)
messages: + msg314131

stage: resolved -> (no value)
2018-02-26 19:25:05barrysetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-02-26 18:24:35barrysetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request5682
2018-02-25 19:29:41ned.deilysetmessages: + msg312845
2018-02-25 19:21:44barrysetmessages: + msg312844
2018-02-25 19:11:52ned.deilysetmessages: + msg312839
stage: needs patch
2018-02-20 03:57:42ncoghlansetmessages: + msg312397
2018-02-19 20:19:23brett.cannonsetmessages: + msg312366
2018-02-19 18:31:39ned.deilysetpriority: critical -> release blocker
2018-02-19 18:27:18ned.deilysetstatus: closed -> open

nosy: + brett.cannon, ncoghlan, ned.deily
messages: + msg312358

resolution: wont fix -> (no value)
stage: resolved -> (no value)
2018-02-19 17:57:25barrysetstatus: open -> closed
resolution: wont fix
stage: resolved
2018-02-19 13:41:05barrysetmessages: + msg312351
2018-02-19 09:03:48dokocreate