Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

importing from UNC roots doesn't work #47927

Closed
kristjanvalur mannequin opened this issue Aug 25, 2008 · 19 comments
Closed

importing from UNC roots doesn't work #47927

kristjanvalur mannequin opened this issue Aug 25, 2008 · 19 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-windows type-bug An unexpected behavior, bug, or error

Comments

@kristjanvalur
Copy link
Mannequin

kristjanvalur mannequin commented Aug 25, 2008

BPO 3677
Nosy @loewis, @kristjanvalur, @benjaminp
Files
  • not_use_stat_in_import_on_windows.patch: trunk
  • not_use_stat_in_import_on_windows.patch: reviewed by Kristjan, fixes this issue and
  • 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 = None
    closed_at = <Date 2009-01-09.20:11:28.215>
    created_at = <Date 2008-08-25.15:00:26.607>
    labels = ['interpreter-core', 'type-bug', 'OS-windows']
    title = "importing from UNC roots doesn't work"
    updated_at = <Date 2009-01-24.13:57:44.811>
    user = 'https://github.com/kristjanvalur'

    bugs.python.org fields:

    activity = <Date 2009-01-24.13:57:44.811>
    actor = 'ocean-city'
    assignee = 'none'
    closed = True
    closed_date = <Date 2009-01-09.20:11:28.215>
    closer = 'kristjan.jonsson'
    components = ['Interpreter Core', 'Windows']
    creation = <Date 2008-08-25.15:00:26.607>
    creator = 'kristjan.jonsson'
    dependencies = []
    files = ['11740', '12591']
    hgrepos = []
    issue_num = 3677
    keywords = ['patch']
    message_count = 19.0
    messages = ['71932', '71941', '74478', '74490', '74491', '74495', '74496', '74500', '74501', '79118', '79500', '80397', '80399', '80434', '80442', '80443', '80444', '80446', '80448']
    nosy_count = 5.0
    nosy_names = ['loewis', 'ggenellina', 'kristjan.jonsson', 'ocean-city', 'benjamin.peterson']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue3677'
    versions = ['Python 2.5']

    @kristjanvalur
    Copy link
    Mannequin Author

    kristjanvalur mannequin commented Aug 25, 2008

    When executing a script from a UNC path, e.g. //myhost/exports/a.py,
    r"\\myhost\exports" gets prepended to sys.path. But it doesn't work.
    This means that in a.py, "import b" will fail even though b.py is
    present in the same directory.

    The workaround is to manually prepend a backslass to the UNC path in
    sys.path.

    Note the related defect: http://bugs.python.org/issue954115
    This intdoruces two functions for testing UNC paths, but it appears
    that these functions are nowhere used!

    @kristjanvalur kristjanvalur mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Aug 25, 2008
    @kristjanvalur
    Copy link
    Mannequin Author

    kristjanvalur mannequin commented Aug 25, 2008

    Correction: The workaround is to _append_ a backslash.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Oct 7, 2008

    Kristjan, you suggested this issue for consideration for 2.5.3.

    Is there an actual patch to apply?

    If not, the issue should get forwarded to 2.7 (and then to 2.8, and so
    on, until somebody actually comes up with a patch).

    @kristjanvalur
    Copy link
    Mannequin Author

    kristjanvalur mannequin commented Oct 7, 2008

    No, not really.
    Again, I refer to defect 954115 by glchapman. And note that those
    functions added there are actually not used.
    I was hoping that there was someone here more familiar with importing
    on PC.
    I'll go and see if the old defect still applies and if there is perhaps
    a more general fix to be made.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Oct 7, 2008

    Ok, un-targetting this from 2.5.3 then.

    Usage of IsUNCRoot disappeared as part of r42230, which dropped usage of
    the C library for stat implementations. This only affects os.stat,
    though, so I don't see the relation to this issue.

    For the record: What *exactly* is the problem? I.e. specifically, what
    do you do, what happens, what do you expect to happen instead?

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Oct 7, 2008

    I think this is stat(2) problem on windows. Please try following test
    program.

    #include <stdio.h>
    #include <sys/stat.h>
    #include <windows.h>
    
    void test(const char *path)
    {
        struct stat st;
    
        printf("%s %d %d\n", path, stat(path, &st), GetFileAttributes(path));
    }
    
    int main()
    {
        test("e:\\shared");
        test("e:\\shared\\"); /* issue1293 */
        test("e:\\shared\\a.py");
        test("\\\\whiterab-c2znlh\\shared"); /* this issue */
        test("\\\\whiterab-c2znlh\\shared\\");
        test("\\\\whiterab-c2znlh\\shared\\a.py");
    }

    And this is result.

    e:\shared 0 16
    e:\shared\ -1 16
    e:\shared\a.py 0 32
    \\whiterab-c2znlh\shared -1 16
    \\whiterab-c2znlh\shared\ 0 16
    \\whiterab-c2znlh\shared\a.py 0 32

    As discussed in bpo-1293, stat(2) fails for the normal folder path with
    trailing slash, and also fails for the UNC folder path without trailing
    slash. I'm using VC6, but same behavior on VC9?

    trunk/Python/import.c(3160) and trunk/Modules/zipimport.c(99) is using
    stat(2).

    I'll create patch to prove my observation. :-)

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Oct 7, 2008

    Ah, of cource, please change path to fit you environment. > test prog

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Oct 8, 2008

    zipimport.c seems fine, because stat(2) succeeds for UNC file. It fails
    for UNC folder. (zip file is absolutely file)

    Already fix for bpo-1293 was in, so maybe I should create the patch for
    that direction but... anyway this issue seems to be fixed by my patch. I
    also confirmed the test script in http://bugs.python.org/msg56523 works.

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Oct 8, 2008

    Just humble thought...
    If we can replace stat(2) + S_ISDIR/S_ISREG check with
    GetFileAttributesEx, maybe we are safe for this kind of problem, and can
    solve bpo-3099 same time? :-)

    @ocean-city ocean-city mannequin added the OS-windows label Oct 10, 2008
    @kristjanvalur
    Copy link
    Mannequin Author

    kristjanvalur mannequin commented Jan 5, 2009

    Okay, I've tested the patch and made some beauty fixes.
    Can we have this in, please?

    @kristjanvalur
    Copy link
    Mannequin Author

    kristjanvalur mannequin commented Jan 9, 2009

    Checked in as revision: 68457

    @kristjanvalur kristjanvalur mannequin closed this as completed Jan 9, 2009
    @benjaminp
    Copy link
    Contributor

    This fix needs to be ported to the py3k branch. Can somebody please do it?

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Jan 23, 2009

    Merged in r68873(py3k).

    @ggenellina
    Copy link
    Mannequin

    ggenellina mannequin commented Jan 24, 2009

    The path variable should be PyMem_Free'd (in both trunk and py3k)
    (also, I don't see any specific test - is there any?)

    @kristjanvalur
    Copy link
    Mannequin Author

    kristjanvalur mannequin commented Jan 24, 2009

    added the mem release in r68882.
    I'll try to add test cases too.

    @kristjanvalur
    Copy link
    Mannequin Author

    kristjanvalur mannequin commented Jan 24, 2009

    Added tests for UNC path imports in r68883 and r68884

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Jan 24, 2009

    The path variable should be PyMem_Free'd

    Sorry if I'm missing something, but is this really needed?
    Other PyArg_ParseTuple(args, "s... doesn't seem to have
    corresponding PyMem_Free.

    static PyObject *
    imp_new_module(PyObject *self, PyObject *args)
    {
    	char *name;
    	if (!PyArg_ParseTuple(args, "s:new_module", &name))
    		return NULL;
    	return PyModule_New(name);
    }

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Jan 24, 2009

    Sorry if I'm missing something, but is this really needed?

    Yes, the "es" converter allocates memory.

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Jan 24, 2009

    Ah, "es" is used in py3k, thanks. And sorry about my merge which had
    memory leak.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-windows type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant