Description

barneygale

opened on May 2, 2023

Issue body actions

As part of removing "flavour" classes in #31691, I changed pathlib's glob() implementation: previously it used re.IGNORECASE to implement case-insensitive matches, whereas after it called os.path.normcase() on the pattern and the paths. The new behaviour is a little slower, and I think we should restore the previous implementation.

Linked PRs

• GH-104104: Optimize pathlib.Path.glob() by avoiding repeated calls to os.path.normcase() #104105

Activity

1. barneygale

   added

   type-featureA feature request or enhancementA feature request or enhancement

   on May 2, 2023
2. barneygale

   added a commit that references this issue on May 2, 2023

   pythonGH-104104: Optimize `pathlib.Path.glob()` by avoiding repeated …

   93ad73f
3. barneygale

   added

   performancePerformance or resource usagePerformance or resource usage

   3.12only security fixesonly security fixes

   topic-pathlib

   on May 2, 2023
4. barneygale

   added a commit that references this issue on May 2, 2023

   GH-104104: Optimize `pathlib.Path.glob()` by avoiding repeated calls …

   47770a1
5. barneygale

   closed this as completedon May 2, 2023
6. carljm

   added a commit that references this issue on May 2, 2023

   Merge branch 'main' into cached_property_docs

   b9c39e3
7. eryksun commented on May 3, 2023

   

   eryksun

   on May 3, 2023

   Contributor

   More actions

   Using re.IGNORECASE is wrong on Windows. Correct case-insensitive filename comparisons on Windows have to use os.path.normcase(). The implementation calls WinAPI LCMapStringEx() with the invariant locale to get a simple lowercase conversion of characters in the basic multilingual plane (BMP). This case mapping is always a one-to-one character mapping, and no mapping is implemented for characters beyond the BMP. This basically matches how filesystems on Windows implement case-insensitive name comparisons, except they use an uppercase conversion, which doesn't matter for checking equality. (It matters a bit for the sort order of filenames.)

   For example, Python's lowercase mapping of "İ" (U+0130) is a two-character string:

   >>> print(ascii('İ'.lower()))
   'i\u0307'

   To a Windows filesystem, "İ" and "i\u0307" are different filenames.

   >>> open('i\u0307', 'w').close()
   >>> open('\u0130', 'w').close()
   >>> names = os.listdir()
   >>> names
   ['i̇', 'İ']
   >>> os.path.normcase(names[0])
   'i̇'
   >>> os.path.normcase(names[1])
   'İ'
   >>> os.path.normcase(names[0]) == os.path.normcase(names[1])
   False

   Using re.IGNORECASE yields a false positive match:

   >>> re.match(names[1], names[0], re.IGNORECASE)
   <re.Match object; span=(0, 1), match='i'>

   Reacted by Barney Gale
8. eryksun

   reopened this on May 3, 2023
9. barneygale commented on May 3, 2023

   

   barneygale

   on May 3, 2023

   ContributorAuthor

   More actions

   That bug existed in all previous versions of pathlib; I didn't intentionally fix it IIRC. Personally I consider it pretty minor. I'm hoping to add a case_sensitive argument to glob() in #102710, and so I'd rather keep the case normalization rules reasonably generic.
10. eryksun commented on May 3, 2023

    

    eryksun

    on May 3, 2023

    Contributor

    More actions

    Okay, I didn't review the code in detail. I just wanted you to be aware that Windows paths should not be case-insensitively compared for equality using Python's lowercase mapping. The system's locale-invariant, non-linguistic case mapping has to be used when comparing paths.
11. eryksun

    closed this as completedon May 3, 2023
12. eryksun commented on May 3, 2023

    

    eryksun

    on May 3, 2023

    Contributor

    More actions

    This case mapping is always a one-to-one character mapping, and no mapping is implemented for characters beyond the BMP.

    Of course when I actually checked this just now I discovered a bug. When os.path.normcase() was modified to use WinAPI LCMapStringEx() instead of str.lower(), I only verified that it worked correctly for characters in the BMP and that using str.lower() was wrong for 200 characters in the BMP. Unfortunately the documented behavior of LCMapStringEx() is wrong for non-BMP characters. It's supposed to default to "file system" rules, for which the surrogate pair representation of a non-BMP character should be handled as two UCS-2 ordinals. But LCMapStringEx() actually implements a case mapping for 40 non-BMP characters:

    >>> os.listdir()
    []
    >>> chars = [c for i in range(65536, sys.maxunicode) if normcase(c:=chr(i)) != c]
    >>> len(chars)
    40
    >>> [(c, normcase(c)) for c in chars]
    [('𐐀', '𐐨'), ('𐐁', '𐐩'), ('𐐂', '𐐪'), ('𐐃', '𐐫'), ('𐐄', '𐐬'), ('𐐅', '𐐭'), ('𐐆', '𐐮'), ('𐐇', '𐐯'), ('𐐈',
     '𐐰'), ('𐐉', '𐐱'), ('𐐊', '𐐲'), ('𐐋', '𐐳'), ('𐐌', '𐐴'), ('𐐍', '𐐵'), ('𐐎', '𐐶'), ('𐐏', '𐐷'), ('𐐐', '𐐸'),
     ('𐐑', '𐐹'), ('𐐒', '𐐺'), ('𐐓', '𐐻'), ('𐐔', '𐐼'), ('𐐕', '𐐽'), ('𐐖', '𐐾'), ('𐐗', '𐐿'), ('𐐘', '𐑀'), ('𐐙',
     '𐑁'), ('𐐚', '𐑂'), ('𐐛', '𐑃'), ('𐐜', '𐑄'), ('𐐝', '𐑅'), ('𐐞', '𐑆'), ('𐐟', '𐑇'), ('𐐠', '𐑈'), ('𐐡', '𐑉'),
     ('𐐢', '𐑊'), ('𐐣', '𐑋'), ('𐐤', '𐑌'), ('𐐥', '𐑍'), ('𐐦', '𐑎'), ('𐐧', '𐑏')]

    To the filesystem, each of these normalized names is unique:

    >>> len(os.listdir())
    0
    >>> for c in chars: open(c, 'w').close(); open(normcase(c), 'w').close()
    ...
    >>> len(os.listdir())
    80

    I don't know how to efficiently resolve this bug, short of calling a lower-level NTAPI function instead of LCMapStringEx(). I think we need a new path comparison function in os.path. On Windows, it can call CompareStringOrdinal() and default to a case-insensitive comparison. CompareStringOrdinal() uses the same uppercase mapping as the filesystem.

Issue actions