msg251046 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2015-09-19 00:49 |
Except for crypt, all the modules labeled 'Unix' or 'Linux' on the module index https://docs.python.org/3/py-modindex.html are absent from /Lib on Windows. 'import xyz' fails with "ImportError: no module named 'zyz'". (I presume the same is true on unix for Windows-only modules.)
However, crypt is present, and 'import crypt' fails with "...'_crypt'", leading one to think that the C accelerator should be present but is not. Assuming that _crypt should (cannot) not be present, please add crypt to the list of modules omitted from the Windows installer, however this is done. And if 'unix-only' is obsolete and _crypt should be present, please fix this instead. ;-)
This is 3.x issue only, as crypt is not present in my Windows 2.7.10 /Lib. The fact that is was somehow added to some 3.x prompted me to look and see if there is anything useful without _crypt present. I think not.
|
msg348935 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2019-08-02 23:25 |
Marking this easy/newcomer friendly.
This should catch the ModuleNotFoundError raised when _crypt is missing and raise a more informative ImportError saying that crypt is unsupported.
All the other modules that are missing are native extension modules that are not built - this is the only one that is a .py file. We don't exclude any other Lib\*.py files from the distribution.
|
msg349097 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-06 03:35 |
Hello
I would like to work on this issue. First time contributor here. Would putting the import _crypt statement on line 3 in crypt.py inside a try/except block and checking to see if platform is windows then give an appropriate message be a good fix?
Thank you
-Srinivas
|
msg349111 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2019-08-06 15:17 |
That sounds good to me, though you may want to propose the error message here first so we can get the wording right - probably not everyone will be watching github PR.
It probably makes sense to raise a different error message on platforms where we do expect it to have been built - the same confusion could arise there.
Some starting points (that will likely need improvement):
win32: raise ImportError("The crypt module is not supported on Windows")
not win32: raise ImportError("The required _crypt module was not built as part of CPython")
|
msg349114 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-06 16:53 |
What is the recommended way to check for platform in cpython? Is it using the sys module or os module? As for the error messages I couldn't think of anything better then what you suggested :).
win32: raise ImportError("The crypt module is not supported on Windows")
not win32: raise ImportError("The required _crypt module was not built as part of CPython")
|
msg349115 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2019-08-06 17:09 |
`sys.platform` is the value that reflects how Python was compiled - `sys.platform == 'win32'` means compiled for Windows. And since this issue is related to compilation, it makes the most sense to use sys.
(`os.name` is also based on compilation, but it really only reflects the POSIX implementation in use, which is why sys is better.)
|
msg349116 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-06 17:12 |
Sounds good. Can I submit my PR now?
|
msg349117 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-06 17:20 |
I forgot to ask about tests? I see there is test_crypt.py under Lib\test. Do you have any thoughts on how to test this?
|
msg349118 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2019-08-06 17:23 |
> Can I submit my PR now?
Sure, though if others want to weigh in on wording they should feel free to do it here.
> I forgot to ask about tests? I see there is test_crypt.py under Lib\test. Do you have any thoughts on how to test this?
It should continue to "successfully" skip tests when _crypt isn't built. So as long as you raise ImportError, this should be fine.
|
msg349125 - (view) |
Author: Paul Moore (paul.moore) *  |
Date: 2019-08-06 17:45 |
The proposed wording seems good to me.
|
msg349129 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-06 18:14 |
Sorry, I should have waited 1 day after submitting the CLA before submitting the PR. The system is waiting for my CLA to be cleared.
|
msg349130 - (view) |
Author: Paul Moore (paul.moore) *  |
Date: 2019-08-06 19:40 |
It might be worth adding a test (running only on Windows) to confirm that if you try to import crypt you get an import error with a message that includes the phrase "not supported". That will ensure that we don't get regressions here in future, while still not tying us too strictly to the exact wording.
|
msg349138 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-06 22:39 |
How do I write tests which only run for a given platform? Looking at test_crypt.py, I see that the crypt module is imported using test.support module so the exception will be raised even before the tests run. Is there a way to test imports?
|
msg349143 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2019-08-07 00:41 |
I agree with Paul about the wording. Note that the proposed platform-specific catch and raise are for crypt.py, not test_crypt.py.
If a test module should be skipped entirely, import_module is correct. For more refinement, test_idle has, for example,
tk = import_module('tkinter') # Also imports _tkinter.
if tk.TkVersion < 8.5:
raise unittest.SkipTest("IDLE requires tk 8.5 or later.")
Testing only continues if tkinter and _tkinter and tk >= 8.5.
I presume that crypt = import_module('crypt') will only continue if crypt and _crypt, which is what you want. It is apparently not an error for _crypt to be missing on unix, just an inconvenience for people who expect it.
FYI, Individual test classes and methods can be skipped with
@unittest.skipIf(condition, message) # example
@unittest.skipIf(sys.platform != 'darwin', 'test macOS-only code')
See the unittest doc for more, though apparently not needed here.
|
msg349251 - (view) |
Author: Paul Moore (paul.moore) *  |
Date: 2019-08-08 20:02 |
New changeset f4e725f224b864bf9bf405ff7f863cda46fca1cd by Paul Moore (shireenrao) in branch 'master':
bpo-25172: Raise appropriate ImportError msg when crypt module used on Windows (GH-15149)
https://github.com/python/cpython/commit/f4e725f224b864bf9bf405ff7f863cda46fca1cd
|
msg349252 - (view) |
Author: Paul Moore (paul.moore) *  |
Date: 2019-08-08 20:07 |
On reflection, adding a test for the Windows-specific behaviour looks like it could be more complex than I'd anticipated, so I'm happy for this to go in without an extra test. I'll look at adding a test in a follow-up PR (or if someone else wants to, that's fine as well).
|
msg349253 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-08 20:31 |
I am curious how one would create a test for this? Would this be a new test script? The new script would skip if platform not win32 and the test case would be to import the crypt module and check for "not supported" string in the ImportError?
|
msg349254 - (view) |
Author: Paul Moore (paul.moore) *  |
Date: 2019-08-08 21:07 |
That's where I decided it was too complex for now :-)
The whole test_crypt.py file is skipped if you can't import crypt, via the use of the test.support.import_module() function. To be able to write a test for a platform where the whole point is that you can't import crypt, you'd have to rewrite that. So you'd get something like
try:
import crypt
except ImportError as e:
if sys.platform == "win32":
assert "not supported" in e.msg
raise unittest.SkipTest("crypt is not present on this platform")
But you can't use "assert" like that in top-level code (or at least I don't think you can - I'm not that familiar with unittest, but I think assertions have to live in classes in unittest).
At this point I gave up. It starts to be quite a big rewrite for a relatively minor benefit.
The alternative would be, if there was a "Windows-specific tests" test module, we could have put a test for this situation in there. But I don't think there is such a module.
|
msg349256 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2019-08-08 21:11 |
> The alternative would be, if there was a "Windows-specific tests" test module, we could have put a test for this situation in there
Maybe test___all__?
|
msg349257 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2019-08-08 21:16 |
Or better yet (since my last suggestion was bad), add a second test class to test_crypt
try:
import crypt
IMPORT_ERROR = None
except ImportError as ex:
crypt = None
IMPORT_ERROR = str(ex)
@skipIf(crypt)
class TestWhyCryptDidNotImport(TestCase):
...
@skipUnless(crypt)
class CryptTestCase(TestCase):
...
|
msg349258 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-08 21:36 |
Are you suggesting using the try/except block instead of import_module in test_crypt.py
|
msg349259 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2019-08-08 21:38 |
Yes. support.import_module is going to raise a skip when the module can't be imported, but we want to handle ImportError differently.
|
msg349262 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-08 22:00 |
Can I have a go at it? Do I need to open a new issue to post a new PR?
|
msg349265 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2019-08-08 22:26 |
Sure. You can post a new PR with the same bug number (and it won't need a NEWS file this time).
|
msg349275 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-09 06:08 |
When I try to put the skipUnless decorator on CryptTestCase, I am still seeing a failure when I try to run this. The error is -
File "C:\Users\srao\projects\cpython\lib\test\test_crypt.py", line 59, in CryptTestCase
@unittest.skipUnless(crypt.METHOD_SHA256 in crypt.methods or
AttributeError: 'NoneType' object has no attribute 'METHOD_SHA256'
Line 59 is the following decorator and method -
@unittest.skipUnless(crypt.METHOD_SHA256 in crypt.methods or
crypt.METHOD_SHA512 in crypt.methods,
'requires support of SHA-2')
def test_sha2_rounds(self):
I tried a simple @skip decorator and that too fails on the same error. The class level skip is not helping here. Does it make sense to create a different test file for windows?
|
msg349566 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-13 14:42 |
I submitted a new PR for the Windows test case. Please take a look. Also how do I attach the label "skip news" to this new PR?
|
msg349568 - (view) |
Author: Paul Moore (paul.moore) *  |
Date: 2019-08-13 14:57 |
I added the label for you.
|
msg349569 - (view) |
Author: Paul Moore (paul.moore) *  |
Date: 2019-08-13 14:59 |
To clarify, I think adding the label needs core dev (or maybe triager) rights on github, so I don't think it's something you can do yourself.
|
msg349615 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-13 20:22 |
Thank you @paul.moore
|
msg349619 - (view) |
Author: miss-islington (miss-islington) |
Date: 2019-08-13 21:27 |
New changeset 7f7f74734acd729d1f82b7cf672e064c9525fced by Miss Islington (bot) in branch '3.8':
bpo-25172: Raise appropriate ImportError msg when crypt module used on Windows (GH-15149)
https://github.com/python/cpython/commit/7f7f74734acd729d1f82b7cf672e064c9525fced
|
msg349620 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2019-08-13 21:27 |
New changeset 243a73deee4ac61fe06602b7ed56b6df01e19f27 by Steve Dower (shireenrao) in branch 'master':
bpo-25172: Add test for crypt ImportError on Windows (GH-15252)
https://github.com/python/cpython/commit/243a73deee4ac61fe06602b7ed56b6df01e19f27
|
msg349621 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2019-08-13 21:30 |
Thanks for the patches, Srinivas!
|
msg349623 - (view) |
Author: miss-islington (miss-islington) |
Date: 2019-08-13 21:52 |
New changeset e7ec9e04c82be72aef621fdfba03f41cbd8599aa by Miss Islington (bot) in branch '3.8':
bpo-25172: Add test for crypt ImportError on Windows (GH-15252)
https://github.com/python/cpython/commit/e7ec9e04c82be72aef621fdfba03f41cbd8599aa
|
msg349624 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2019-08-13 22:18 |
Thank you Steve for accepting my pull requests.
I was surprised to see the methods in the class and its decorators getting evaluated and causing the failures initially, but I then realized that the code is parsed before execution and that's when I was seeing the failure.
|
msg356914 - (view) |
Author: Xavier de Gaye (xdegaye) *  |
Date: 2019-11-18 21:43 |
test_crypt fails on android following last changes made at 243a73deee4ac61fe06602b7ed56b6df01e19f27.
The android libc does not have a crypt() function and the _crypt module is not built.
generic_x86_64:/data/local/tmp/python $ python
Python 3.9.0a0 (heads/abifa-dirty:cf805c25e6, Nov 18 2019, 16:40:26)
[Clang 8.0.2 (https://android.googlesource.com/toolchain/clang 40173bab62ec7462 on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import crypt
Traceback (most recent call last):
File "/data/local/tmp/python/lib/python3.9/crypt.py", line 6, in <module>
import _crypt
ModuleNotFoundError: No module named '_crypt'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/data/local/tmp/python/lib/python3.9/crypt.py", line 11, in <module>
raise ImportError("The required _crypt module was not built as part of CPython")
ImportError: The required _crypt module was not built as part of CPython
>>>
generic_x86_64:/data/local/tmp/python $ python -m test -v test_crypt
== CPython 3.9.0a0 (heads/abifa-dirty:cf805c25e6, Nov 18 2019, 16:40:26) [Clang 8.0.2 (https://andro
id.googlesource.com/toolchain/clang 40173bab62ec7462
== Linux-3.10.0+-x86_64-with-libc little-endian
== cwd: /data/local/tmp/python/tmp/test_python_3523
== CPU count: 2
== encodings: locale=UTF-8, FS=utf-8
0:00:00 Run tests sequentially
0:00:00 [1/1] test_crypt
test_blowfish_rounds (test.test_crypt.CryptTestCase) ... skipped 'Not supported on Windows'
test_crypt (test.test_crypt.CryptTestCase) ... skipped 'Not supported on Windows'
test_invalid_rounds (test.test_crypt.CryptTestCase) ... skipped 'Not supported on Windows'
test_methods (test.test_crypt.CryptTestCase) ... skipped 'Not supported on Windows'
test_salt (test.test_crypt.CryptTestCase) ... skipped 'Not supported on Windows'
test_saltedcrypt (test.test_crypt.CryptTestCase) ... skipped 'Not supported on Windows'
test_sha2_rounds (test.test_crypt.CryptTestCase) ... skipped 'Not supported on Windows'
test_failure_only_for_windows (test.test_crypt.TestWhyCryptDidNotImport) ... FAIL
test_import_failure_message (test.test_crypt.TestWhyCryptDidNotImport) ... FAIL
======================================================================
FAIL: test_failure_only_for_windows (test.test_crypt.TestWhyCryptDidNotImport)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/data/local/tmp/python/lib/python3.9/test/test_crypt.py", line 16, in test_failure_only_for_
windows
self.assertEqual(sys.platform, 'win32')
AssertionError: 'linux' != 'win32'
- linux
+ win32
======================================================================
FAIL: test_import_failure_message (test.test_crypt.TestWhyCryptDidNotImport)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/data/local/tmp/python/lib/python3.9/test/test_crypt.py", line 19, in test_import_failure_message
self.assertIn('not supported', IMPORT_ERROR)
AssertionError: 'not supported' not found in 'The required _crypt module was not built as part of CPython'
----------------------------------------------------------------------
Ran 9 tests in 0.008s
FAILED (failures=2, skipped=7)
test test_crypt failed
test_crypt failed
== Tests result: FAILURE ==
1 test failed:
test_crypt
Total duration: 165 ms
Tests result: FAILURE
|
msg358170 - (view) |
Author: Xavier de Gaye (xdegaye) *  |
Date: 2019-12-10 07:48 |
Not interested anymore in android stuff.
|
msg359467 - (view) |
Author: Srinivas Nyayapati (shireenrao) * |
Date: 2020-01-06 20:29 |
@steve.dower - Does this fix need more work for android? @xdegaye says he does not need this for android anymore.
|
msg359469 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2020-01-06 21:27 |
I started creating an example of improving the skip cases, and ended up just making a PR. It should:
* expect a helpful error on Windows
* work fine on other platforms with crypt
* skip all tests on other platforms without crypt
|
msg359686 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2020-01-09 17:00 |
New changeset ed367815eeb9329c48a86a8a7fa3186e27a10f2c by Steve Dower in branch 'master':
bpo-25172: Reduce scope of crypt import tests (GH-17881)
https://github.com/python/cpython/commit/ed367815eeb9329c48a86a8a7fa3186e27a10f2c
|
msg359688 - (view) |
Author: miss-islington (miss-islington) |
Date: 2020-01-09 17:20 |
New changeset 8c08518c255747a06d00479f21087f0c934d0ad6 by Miss Islington (bot) in branch '3.8':
bpo-25172: Reduce scope of crypt import tests (GH-17881)
https://github.com/python/cpython/commit/8c08518c255747a06d00479f21087f0c934d0ad6
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:21 | admin | set | github: 69359 |
2020-01-09 17:40:12 | steve.dower | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
2020-01-09 17:20:06 | miss-islington | set | messages:
+ msg359688 |
2020-01-09 17:01:06 | miss-islington | set | pull_requests:
+ pull_request17330 |
2020-01-09 17:00:47 | steve.dower | set | messages:
+ msg359686 |
2020-01-06 21:28:11 | steve.dower | set | stage: needs patch -> patch review pull_requests:
+ pull_request17297 |
2020-01-06 21:27:55 | steve.dower | set | messages:
+ msg359469 |
2020-01-06 20:29:49 | shireenrao | set | messages:
+ msg359467 |
2019-12-10 07:56:18 | xdegaye | set | nosy:
- xdegaye
|
2019-12-10 07:48:36 | xdegaye | set | nosy:
terry.reedy, paul.moore, jafo, tim.golden, xdegaye, zach.ware, steve.dower, miss-islington, shireenrao messages:
+ msg358170 |
2019-11-19 15:48:10 | xdegaye | set | status: closed -> open resolution: fixed -> (no value) stage: resolved -> needs patch |
2019-11-18 21:43:47 | xdegaye | set | nosy:
+ xdegaye messages:
+ msg356914
|
2019-08-13 22:18:52 | shireenrao | set | messages:
+ msg349624 |
2019-08-13 21:52:23 | miss-islington | set | messages:
+ msg349623 |
2019-08-13 21:30:21 | steve.dower | set | status: open -> closed resolution: fixed messages:
+ msg349621
stage: patch review -> resolved |
2019-08-13 21:28:02 | miss-islington | set | pull_requests:
+ pull_request14982 |
2019-08-13 21:27:37 | steve.dower | set | messages:
+ msg349620 |
2019-08-13 21:27:18 | miss-islington | set | nosy:
+ miss-islington messages:
+ msg349619
|
2019-08-13 20:22:23 | shireenrao | set | messages:
+ msg349615 |
2019-08-13 14:59:04 | paul.moore | set | messages:
+ msg349569 |
2019-08-13 14:57:51 | paul.moore | set | messages:
+ msg349568 |
2019-08-13 14:42:42 | shireenrao | set | messages:
+ msg349566 |
2019-08-13 14:38:36 | shireenrao | set | pull_requests:
+ pull_request14973 |
2019-08-09 06:08:42 | shireenrao | set | messages:
+ msg349275 |
2019-08-08 22:26:17 | steve.dower | set | messages:
+ msg349265 |
2019-08-08 22:00:26 | shireenrao | set | messages:
+ msg349262 |
2019-08-08 21:38:29 | steve.dower | set | messages:
+ msg349259 |
2019-08-08 21:36:40 | shireenrao | set | messages:
+ msg349258 |
2019-08-08 21:16:08 | steve.dower | set | messages:
+ msg349257 |
2019-08-08 21:11:50 | steve.dower | set | messages:
+ msg349256 |
2019-08-08 21:07:55 | paul.moore | set | messages:
+ msg349254 |
2019-08-08 20:31:24 | shireenrao | set | messages:
+ msg349253 |
2019-08-08 20:07:14 | paul.moore | set | messages:
+ msg349252 |
2019-08-08 20:03:08 | miss-islington | set | pull_requests:
+ pull_request14914 |
2019-08-08 20:02:58 | paul.moore | set | messages:
+ msg349251 |
2019-08-07 00:41:19 | terry.reedy | set | messages:
+ msg349143 |
2019-08-06 22:39:00 | shireenrao | set | messages:
+ msg349138 |
2019-08-06 19:40:05 | paul.moore | set | messages:
+ msg349130 |
2019-08-06 18:14:01 | shireenrao | set | messages:
+ msg349129 |
2019-08-06 17:54:30 | shireenrao | set | keywords:
+ patch stage: needs patch -> patch review pull_requests:
+ pull_request14884 |
2019-08-06 17:45:05 | paul.moore | set | messages:
+ msg349125 |
2019-08-06 17:23:25 | steve.dower | set | messages:
+ msg349118 |
2019-08-06 17:20:56 | shireenrao | set | messages:
+ msg349117 |
2019-08-06 17:12:06 | shireenrao | set | messages:
+ msg349116 |
2019-08-06 17:09:42 | steve.dower | set | messages:
+ msg349115 |
2019-08-06 16:53:33 | shireenrao | set | messages:
+ msg349114 |
2019-08-06 15:17:06 | steve.dower | set | messages:
+ msg349111 |
2019-08-06 03:35:52 | shireenrao | set | nosy:
+ shireenrao messages:
+ msg349097
|
2019-08-02 23:25:16 | steve.dower | set | keywords:
+ easy, newcomer friendly
messages:
+ msg348935 versions:
+ Python 3.8, Python 3.9, - Python 3.6, Python 3.7 |
2017-10-24 20:55:02 | serhiy.storchaka | set | stage: needs patch type: behavior versions:
+ Python 3.7, - Python 3.4, Python 3.5 |
2015-09-19 00:49:31 | terry.reedy | create | |