classification
Title: Unix-only crypt should not be present on Windows.
Type: behavior Stage: needs patch
Components: Library (Lib), Windows Versions: Python 3.9, Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: jafo, miss-islington, paul.moore, shireenrao, steve.dower, terry.reedy, tim.golden, xdegaye, zach.ware
Priority: normal Keywords: easy, newcomer friendly, patch

Created on 2015-09-19 00:49 by terry.reedy, last changed 2019-11-19 15:48 by xdegaye.

Pull Requests
URL Status Linked Edit
PR 15149 merged shireenrao, 2019-08-06 17:54
PR 15182 merged miss-islington, 2019-08-08 20:03
PR 15252 merged shireenrao, 2019-08-13 14:38
PR 15263 merged miss-islington, 2019-08-13 21:28
Messages (35)
msg251046 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2019-08-13 14:57
I added the label for you.
msg349569 - (view) Author: Paul Moore (paul.moore) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python triager) 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
History
Date User Action Args
2019-11-19 15:48:10xdegayesetstatus: closed -> open
resolution: fixed ->
stage: resolved -> needs patch
2019-11-18 21:43:47xdegayesetnosy: + xdegaye
messages: + msg356914
2019-08-13 22:18:52shireenraosetmessages: + msg349624
2019-08-13 21:52:23miss-islingtonsetmessages: + msg349623
2019-08-13 21:30:21steve.dowersetstatus: open -> closed
resolution: fixed
messages: + msg349621

stage: patch review -> resolved
2019-08-13 21:28:02miss-islingtonsetpull_requests: + pull_request14982
2019-08-13 21:27:37steve.dowersetmessages: + msg349620
2019-08-13 21:27:18miss-islingtonsetnosy: + miss-islington
messages: + msg349619
2019-08-13 20:22:23shireenraosetmessages: + msg349615
2019-08-13 14:59:04paul.mooresetmessages: + msg349569
2019-08-13 14:57:51paul.mooresetmessages: + msg349568
2019-08-13 14:42:42shireenraosetmessages: + msg349566
2019-08-13 14:38:36shireenraosetpull_requests: + pull_request14973
2019-08-09 06:08:42shireenraosetmessages: + msg349275
2019-08-08 22:26:17steve.dowersetmessages: + msg349265
2019-08-08 22:00:26shireenraosetmessages: + msg349262
2019-08-08 21:38:29steve.dowersetmessages: + msg349259
2019-08-08 21:36:40shireenraosetmessages: + msg349258
2019-08-08 21:16:08steve.dowersetmessages: + msg349257
2019-08-08 21:11:50steve.dowersetmessages: + msg349256
2019-08-08 21:07:55paul.mooresetmessages: + msg349254
2019-08-08 20:31:24shireenraosetmessages: + msg349253
2019-08-08 20:07:14paul.mooresetmessages: + msg349252
2019-08-08 20:03:08miss-islingtonsetpull_requests: + pull_request14914
2019-08-08 20:02:58paul.mooresetmessages: + msg349251
2019-08-07 00:41:19terry.reedysetmessages: + msg349143
2019-08-06 22:39:00shireenraosetmessages: + msg349138
2019-08-06 19:40:05paul.mooresetmessages: + msg349130
2019-08-06 18:14:01shireenraosetmessages: + msg349129
2019-08-06 17:54:30shireenraosetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request14884
2019-08-06 17:45:05paul.mooresetmessages: + msg349125
2019-08-06 17:23:25steve.dowersetmessages: + msg349118
2019-08-06 17:20:56shireenraosetmessages: + msg349117
2019-08-06 17:12:06shireenraosetmessages: + msg349116
2019-08-06 17:09:42steve.dowersetmessages: + msg349115
2019-08-06 16:53:33shireenraosetmessages: + msg349114
2019-08-06 15:17:06steve.dowersetmessages: + msg349111
2019-08-06 03:35:52shireenraosetnosy: + shireenrao
messages: + msg349097
2019-08-02 23:25:16steve.dowersetkeywords: + easy, newcomer friendly

messages: + msg348935
versions: + Python 3.8, Python 3.9, - Python 3.6, Python 3.7
2017-10-24 20:55:02serhiy.storchakasetstage: needs patch
type: behavior
versions: + Python 3.7, - Python 3.4, Python 3.5
2015-09-19 00:49:31terry.reedycreate