python / cpython

The Python programming language
https://www.python.org
Other
62.75k stars 30.07k forks source link

Refactoring Python/import.c #41378

Closed theller closed 19 years ago

theller commented 19 years ago
BPO 1093253
Nosy @mwhudson, @theller
Files
  • import.c.patch: Refactoring of Python/import.c
  • import.c.patch2: Second patch
  • import.c: Complete, patched Python/import.c
  • 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: ```python assignee = None closed_at = created_at = labels = ['interpreter-core'] title = 'Refactoring Python/import.c' updated_at = user = 'https://github.com/theller' ``` bugs.python.org fields: ```python activity = actor = 'theller' assignee = 'none' closed = True closed_date = None closer = None components = ['Interpreter Core'] creation = creator = 'theller' dependencies = [] files = ['6398', '6399', '6400'] hgrepos = [] issue_num = 1093253 keywords = ['patch'] message_count = 9.0 messages = ['47413', '47414', '47415', '47416', '47417', '47418', '47419', '47420', '47421'] nosy_count = 2.0 nosy_names = ['mwh', 'theller'] pr_nums = [] priority = 'normal' resolution = 'rejected' stage = None status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue1093253' versions = ['Python 2.5'] ```

    theller commented 19 years ago

    This patch refactores Python/import.c.

    find_module() was changed to return an PyObject* pointer which contains the module's pathname, instead of filling out a char* buffer.

    load_module() accepts the PyObject pathname instead of a char.

    The patch is probably missing some error checking, and the 8 character hack for loading extensions on OS2 is not implemented, but the test case runs without errors on Windows XP pro.

    If a change in spirit of this patch is accepted, I'm willing to further work on it so that eventually unicode entries on sys.path, which can not be encoded with the default file system encodings, will work as expected (currently they don't).

    See also: http://mail.python.org/pipermail/python-list/2004-December/256969.html

    mwhudson commented 19 years ago

    Logged In: YES user_id=6656

    Applied the patch and built on OS X. This was the result:

    $ ./python.exe 
    'import site' failed; use -v for traceback
    ../Python/import.c:1496: failed assertion `dirlen <= MAXPATHLEN'
    Abort trap

    dirlen is 796092779, which seems fishy :) An uninitialized variable, maybe? Haven't looked, really...

    theller commented 19 years ago

    Logged In: YES user_id=11105

    Yes, I overlooked that the initialization of the variables is inside an #if defined(MS_WINDOWS) block. Probably it would be better to leave the signature of case_ok() as before and call it through a wrapper which converts the arguments.

    I will prepare a new patch in a few days.

    mwhudson commented 19 years ago

    Logged In: YES user_id=6656

    Perhaps there should be multiple implementations of case_ok ... i.e.

    #if PLAT1
    int case_ok(...)
    {
    ...
    }
    #elif PLAT2
    int case_ok(...)
    {
    ...
    }
    #endif

    the current spaghetti is confusing, even by the standards of import.c...

    theller commented 19 years ago

    Logged In: YES user_id=11105

    New patch attached with multiple implementations of case_ok, and more error checking: import.c.patch2

    Slightly tested on OSX, Linux, Windows.

    The case_ok function still needs to be fixed for RISCOS (which I cannot test).

    theller commented 19 years ago

    Logged In: YES user_id=11105

    For easier reading, I've attached the complete, new Python/import.c file.

    theller commented 19 years ago

    Logged In: YES user_id=11105

    Martin, this is the first step for making unicode entries on sys.path work the way that is expected on Windows. You requested patches, so please take a look, if you have time. Otherwise, unassign yourself.

    theller commented 19 years ago

    Logged In: YES user_id=11105

    Unassigning from MvL.

    theller commented 19 years ago

    Logged In: YES user_id=11105

    Self rejecting. The patch is out of date now, and there seems zero interest in it.