Skip to content

Full unicode import system #47330

Closed
Closed

Description

@amauryfa
BPO 3080
Nosy @brettcannon, @birkenfeld, @terryjreedy, @amauryfa, @ncoghlan, @abalkin, @pitrou, @vstinner, @benjaminp, @merwok, @bitdancer, @asvetlov
Files
  • issue3080-5.patch
  • issue3080.py
  • typo.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/vstinner'
    closed_at = <Date 2011-03-21.00:01:35.123>
    created_at = <Date 2008-06-11.18:24:56.218>
    labels = ['interpreter-core', 'type-bug']
    title = 'Full unicode import system'
    updated_at = <Date 2011-03-23.15:58:18.116>
    user = 'https://github.com/amauryfa'

    bugs.python.org fields:

    activity = <Date 2011-03-23.15:58:18.116>
    actor = 'vstinner'
    assignee = 'vstinner'
    closed = True
    closed_date = <Date 2011-03-21.00:01:35.123>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2008-06-11.18:24:56.218>
    creator = 'amaury.forgeotdarc'
    dependencies = []
    files = ['20477', '21296', '21305']
    hgrepos = []
    issue_num = 3080
    keywords = ['patch']
    message_count = 60.0
    messages = ['68005', '68015', '109844', '112028', '119107', '123963', '123993', '124756', '125752', '126514', '126515', '126516', '126606', '126608', '126612', '126613', '126672', '126673', '126676', '126678', '126680', '126681', '126695', '126705', '126706', '126708', '126752', '126755', '126756', '126760', '127591', '127674', '129141', '129143', '129185', '129196', '130050', '130473', '130492', '130507', '130645', '130935', '131457', '131464', '131472', '131473', '131474', '131483', '131484', '131516', '131547', '131571', '131572', '131575', '131606', '131612', '131625', '131664', '131711', '131890']
    nosy_count = 14.0
    nosy_names = ['brett.cannon', 'georg.brandl', 'terry.reedy', 'amaury.forgeotdarc', 'ncoghlan', 'belopolsky', 'pitrou', 'vstinner', 'benjamin.peterson', 'eric.araujo', 'Arfrever', 'r.david.murray', 'asvetlov', 'python-dev']
    pr_nums = []
    priority = 'high'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue3080'
    versions = ['Python 3.3']

    Activity

    1. amauryfa commented on Jun 11, 2008

      @amauryfa
      ContributorAuthor

      This is the most difficult part of bpo-1342:
      """
      On Windows, don't use the FileSystemEncoding on Windows for sys.path items.
      Instead, it should use the wide API to perform all system calls. Py3k
      shouldn't ever use the file system encoding for anything on Windows.
      """

      This imply to rewrite all functions in import.c, and replace all char*
      arguments with unicode variables.

    2. benjaminp commented on Jun 11, 2008

      @benjaminp
      Contributor

      I suspect importlib may help with this.

    3. added
      type-bugAn unexpected behavior, bug, or error
      on Aug 9, 2008
    4. birkenfeld commented on Jul 10, 2010

      @birkenfeld
      Member

      Victor is working on this.

    5. vstinner commented on Jul 30, 2010

      @vstinner
      Member

      I posted a patch: bpo-9425.

    6. vstinner commented on Oct 19, 2010

      @vstinner
      Member

      With bpo-8611 and bpo-9425, I patched a lot of functions and modules, including the NullImporter and zipimport, but not the core of the import machinery.

      In my import_unicode SVN branch, I patched the import machinery to manipulate unicode strings, instead of bytes strings. But the patch is huge and the import machinery is fragile. Since Python 3.2 now works in a non-ASCII directory with an ASCII locale (fileystem) encoding, I don't plan to merge the patch into py3k.

      The patch is still useful on Windows, because Python uses the mbcs encoding to encode/decode filenames, and this encoding is usually a very small subset of Unicode (eg. cp1252 is 256 codes wheres unicode 6.0 has 109,449 characters).

    7. bitdancer commented on Dec 14, 2010

      @bitdancer
      Member

      With bpo-1342 fixed, it seems that this issue is no longer critical (Haypo describes his complicated patch as "useful on Windows", but not critical. So I'm downgrading it to 'high'. Perhaps it is even 'normal'. It also seems as though it is currently languishing unless someone wants to pick it up.

    8. vstinner commented on Dec 14, 2010

      @vstinner
      Member

      Haypo describes his complicated patch as "useful on Windows",
      but not critical

      Usecase on Windows: your japanese friend gives you an USB key (eg. created on Windows with code page 932) with his Python project, you cannot run it on your english speaking Windows (eg. code page 1252), because it loads Python modules with japanese characters in their paths.

      It works if all paths are encodable to your ANSI code page. It doesn't work if a least one character of one path is not encodable to your ANSI code page.

      I don't know if this usecase is common or not.

      Note: the FAT file system of the USB key stores filenames as UTF-16 (and not in the user code page).

    9. vstinner commented on Dec 28, 2010

      @vstinner
      Member

      Issue bpo-10785 prepares the work for this issue: store input filename as a unicode string, instead of a byte string, in the parser.

    10. terryjreedy commented on Jan 8, 2011

      @terryjreedy
      Member

      If I edit a file with IDLE, save it, and successfully run it (perhaps to test it), then when I edit a second file that imports the first, I expect the import to work. It does not always (see bpo-10828).

      Import is part of the core definition of the language. Unicode identifiers are supposedly part of Python3. Given the existence of <identifier>.py in the current directory, 'import identifier' should work. If it does not, the 3.1 message '<identifier> not found' is more truthful than the current 'no module named <identifier>', when there is one.

      The doc says "identifier ::= (identifier ".")* identifier". As long as that is not true, some indication of the restriction that most people can understand would be nice. (And I suspect that a majority of Windows users, at least in the US, have no idea of what an 'ANSI code page' is.)

    11. vstinner commented on Jan 19, 2011

      @vstinner
      Member

      Here is a work-in-progress patch: bpo-3080-3.patch. The patch is HUGE and written for Python 3.3.

      $ diffstat issue3080-3.patch 
       Doc/c-api/module.rst   |   24 
       Include/import.h       |   73 +
       Include/moduleobject.h |    2 
       Include/pycapsule.h    |    4 
       Modules/zipimport.c    |  272 +++

      Objects/moduleobject.c | 52 -
      PC/import_nt.c | 84 +-
      Python/dynload_aix.c | 2
      Python/dynload_dl.c | 2
      Python/dynload_hpux.c | 2
      Python/dynload_next.c | 4
      Python/dynload_os2.c | 2
      Python/dynload_shlib.c | 2
      Python/dynload_win.c | 2
      Python/import.c | 1910 +++++++++++++++++++++++++++----------------------
      Python/importdl.c | 79 +-
      Python/importdl.h | 2
      bpo-3080.py | 29
      18 files changed, 1484 insertions(+), 1063 deletions(-)

      As expected, most of the work in done in import.c.

      Decode the module name earlier and encode it later. Try to manipulate PyUnicodeObject objects instead of char* buffers (so we have directly the string length).

      Split the huge and very complex find_module() function into 3 functions (find_module, find_module_filename and find_module2) and document them. Drop OS/2 support in find_module() (it can be kept, but it was easier for me to drop it and the OS/2 maintainer wrote that Python 3 is far from being compatible with OS/2).

      The patch creates some functions: PyModule_GetNameObject(), PyImport_ExecCodeModuleUnicode(), PyImport_AddModuleUnicode(), PyImport_ImportFrozenModuleUnicode(), PyModule_NewUnicode(), ...

      Use "U" format to parse a module name, and "%R" to format a module name (to escape surrogates characters and add quotes, instead of "... '%.200s' ...").

      PyWin_FindRegisteredModule() is now private. Remove fqname argument from _PyImport_GetDynLoadFunc(), it wasn't used.

      Replace open_exclusive() by fopen(name, "wb") on Windows: is it correct?

      TODO:

      • rename xxxobj => xxx to keep original names and have a short patch (eg. I renamed name to nameobj during the transition to detect bugs)
      • catch encoding errors in case_ok()
      • don't encode in case_ok() if case_ok() does nothing (eg. on Linux)
      • find a better name for find_module2()

      The patch contains a tiny script, bpo-3080.py, to test the patch using an ISO-8859-1 locale.

      I will open a thread on the mailing list (python-dev) to decide if this patch is needed or not. If we agree that this issue should be fixed, I will split the patch into smaller parts and start a review process.

    12. vstinner commented on Jan 19, 2011

      @vstinner
      Member

      This patch changes more lines of code than my previous crazy unicode patch (msg103663, issue bpo-8242 bpo-8611 bpo-9425), but it changes less files.

    13. vstinner commented on Jan 19, 2011

      @vstinner
      Member
    14. 35 remaining items

    15. vstinner commented on Mar 20, 2011

      @vstinner
      Member

      http://www.python.org/dev/buildbot/all/builders/PPC%20Tiger%203.x/builds/1599/steps/test/logs/stdio

      ======================================================================
      ERROR: testImpWrapper (test.test_importhooks.ImportHooksTestCase)
      ----------------------------------------------------------------------

      Traceback (most recent call last):
        File "/Users/buildbot/buildarea/3.x.parc-tiger-1/build/Lib/test/test_importhooks.py", line 239, in testImpWrapper
          m = __import__(mname, globals(), locals(), ["__dummy__"])
        File "/Users/buildbot/buildarea/3.x.parc-tiger-1/build/Lib/test/test_importhooks.py", line 132, in load_module
          mod = imp.load_module(fullname, self.file, self.filename, self.stuff)
        File "/Users/buildbot/buildarea/3.x.parc-tiger-1/build/Lib/distutils/core.py", line 19, in <module>
          from distutils.cmd import Command
        File "/Users/buildbot/buildarea/3.x.parc-tiger-1/build/Lib/test/test_importhooks.py", line 132, in load_module
          mod = imp.load_module(fullname, self.file, self.filename, self.stuff)
        File "/Users/buildbot/buildarea/3.x.parc-tiger-1/build/Lib/distutils/cmd.py", line 11, in <module>
          from distutils import util, dir_util, file_util, archive_util, dep_util
        File "/Users/buildbot/buildarea/3.x.parc-tiger-1/build/Lib/test/test_importhooks.py", line 132, in load_module
          mod = imp.load_module(fullname, self.file, self.filename, self.stuff)
        File "/Users/buildbot/buildarea/3.x.parc-tiger-1/build/Lib/distutils/dir_util.py", line 8, in <module>
          import errno
        File "/Users/buildbot/buildarea/3.x.parc-tiger-1/build/Lib/test/test_importhooks.py", line 132, in load_module
          mod = imp.load_module(fullname, self.file, self.filename, self.stuff)
      TypeError: 'NoneType' object is not iterable

    16. vstinner commented on Mar 20, 2011

      @vstinner
      Member
      mod = imp.load_module(fullname, self.file, self.filename, self.stuff)
      

      TypeError: 'NoneType' object is not iterable

      The problem is that imp.find_module() now returns None as the filename, but imp.load_module() doesn't support None.

    17. merwok commented on Mar 20, 2011

      @merwok
      Member

      Attached patch fixes a typo in Doc/c-api/import.rst. You can merge it in your next commit.

    18. python-dev commented on Mar 20, 2011

      python-devmannequin
      Mannequin

      New changeset 7f4a4e393058 by Victor Stinner in branch 'default':
      Issue bpo-3080: imp.load_module() accepts None for the module path
      http://hg.python.org/cpython/rev/7f4a4e393058

    19. vstinner commented on Mar 20, 2011

      @vstinner
      Member

      Ok. Python 3.3 does now support non-ASCII characters in module paths and names on Windows, but only characters encodable to the ANSI code page. To support the full Unicode range, we should remove all calls to PyUnicode_EncodeFSDefault() on Windows:

      a) parse_source_module() has to encode the filename because the parser has no function expecting a filename as a Python object. It uses currently PyParser_ASTFromFile().

      b) write_compiled_module() encodes the filename to call open_exclusive(). I don't know how to implement open_exclusive() for Windows using Unicode filename: open() expects the filename as a byte string. Can we use _Py_fopen() (_wfopen)? Or do you need the O_EXCL flag?

      c) _PyImport_LoadDynamicModule() encodes the filename for _PyImport_GetDynLoadFunc(). The prototype should be changed, but only on Windows, to accept a filename as a Unicode string.

      Issue bpo-10785 is the right fix to (a). When bpo-10785 will be fixed, it will be easier to fix bpo-9319 crash.

    20. vstinner commented on Mar 20, 2011

      @vstinner
      Member

      c) _PyImport_LoadDynamicModule() encodes the filename for
      _PyImport_GetDynLoadFunc(). The prototype should be changed,
      but only on Windows, to accept a filename as a Unicode string.

      Hum, the difficult part is to use Unicode in _PyImport_GetDynLoadFunc() for:

      hDLL = LoadLibraryEx(pathname, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);

      There is a LoadLibraryW() function, but it doesn't have a flag argument. And I suppose that the LOAD_WITH_ALTERED_SEARCH_PATH option is important.

    21. vstinner commented on Mar 21, 2011

      @vstinner
      Member

      Ok, I think that the most important part is now implemented in Python 3.3: use Unicode for module names and paths in the import machinery. Remaing parts are specific to Windows, and so I opened a new issue: bpo-11619. Let's close this 3 years old issue.

    22. python-dev commented on Mar 21, 2011

      python-devmannequin
      Mannequin

      New changeset ee4e780a6b7a by Éric Araujo in branch 'default':
      Fix a typo (see bpo-3080)
      http://hg.python.org/cpython/rev/ee4e780a6b7a

    23. asvetlov commented on Mar 21, 2011

      @asvetlov
      Contributor

      As I see Victor has dropped OS/2 support from Python/import.c
      Perhaps file Python/dynload_os2.c should be removed also.
      Not sure about other dynload_* files.

    24. vstinner commented on Mar 21, 2011

      @vstinner
      Member

      As I see Victor has dropped OS/2 support from Python/import.c
      Perhaps file Python/dynload_os2.c should be removed also.
      Not sure about other dynload_* files.

      340f76a6a792 just removes few lines in import.c: they can easily be rewritten. And this commit doesn't drop completly the support of OS/2 from the import machinery, as you wrote: dynload_os2.c still exists.

      If we drop completly the support of OS/2, it should be done completly using a PEP (I don't remember its number), and it should be discussed. At least with Andrew I MacIntyre :-)

    25. asvetlov commented on Mar 21, 2011

      @asvetlov
      Contributor

      Understood. Sorry.
      I thought Python support only Windows and posix (Linux, BSD, MacOSX etc) systems now, all other OSes are not maintained.

      Anyway please don't care about that.

    26. python-dev commented on Mar 22, 2011

      python-devmannequin
      Mannequin

      New changeset 15f9eca5e956 by Victor Stinner in branch 'default':
      Issue bpo-3080: On DJGPP, case_bytes() returns -1 to signal an error if the file
      http://hg.python.org/cpython/rev/15f9eca5e956

    27. vstinner commented on Mar 23, 2011

      @vstinner
      Member

      test the fixed nosy list

    28. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    interpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions