Closed Bodigrim closed 1 year ago
Excellent.
What I wonder is whether some of this can be backported to the string-only filepath or whether the new API introduced performance regressions (a few functions were slightly rewritten).
I get compilation error on 9.2.6:
/x86_64-linux/ghc-9.2.6/filepath-1.4.100.0/t/abstract-filepath/build/abstract-filepath/abstract-filepath-tmp/OsPathSpec.dyn_o )
tests/abstract-filepath/OsPathSpec.hs:56:64: error: [-Wincomplete-uni-patterns, -Werror=incomplete-uni-patterns]
Pattern match(es) are non-exhaustive
In a lambda abstraction:
Patterns of type ‘Either EncodingException String’ not matched:
Left _
|
56 | property $ \(padEven -> bs) -> (Posix.encodeWith ucs2le . (\(Right r) -> r) . Posix.decodeWith ucs2le . OS.PS . toShort) bs === Right (OS.PS . toShort $ bs))
| ^^^^^^^^^^^^^^^
tests/abstract-filepath/OsPathSpec.hs:58:66: error: [-Wincomplete-uni-patterns, -Werror=incomplete-uni-patterns]
Pattern match(es) are non-exhaustive
In a lambda abstraction:
Patterns of type ‘Either EncodingException String’ not matched:
Left _
|
58 | property $ \(padEven -> bs) -> (Windows.encodeWith ucs2le . (\(Right r) -> r) . Windows.decodeWith ucs2le . OS.WS . toShort) bs
| ^^^^^^^^^^^^^^^
Error: cabal: Failed to build test:abstract-filepath from filepath-1.4.100.0.
What I wonder is whether some of this can be backported to the string-only filepath or whether the new API introduced performance regressions (a few functions were slightly rewritten).
The changes improve performance both for the legacy API and for the new one.
I get compilation error on 9.2.6:
It's a warning for me, and present in the master
branch as well.
If you rebase against master and then push to this repo instead of your fork, we can benefit from the fixed CI.
@hasufell any more comments / suggestions?
@Bodigrim we have a regression:
With this patch (1.4.100.2):
> W.splitFileName "\\\\?\\A:\\fred"
("\\\\?\\A:\\fred", "")
With 1.4.100.0:
> W.splitFileName "\\\\?\\A:\\fred"
("\\\\?\\A:\\", "fred")
This was by chance found by a property test (they are rather unreliable): https://github.com/haskell/filepath/actions/runs/4263893204/jobs/7421218609#step:5:1112
However, W.splitDrive
match.
@hasufell That regression was fixed in https://github.com/haskell/filepath/commit/6a1c98dc5eb8021107d59903bbbb60137a6f9dbd, correct?
I noticed at least one other change in behavior which I discovered by chance in the filepath-bytestring
equivalency test suite. It appears e.g. when comparing filepath-1.4.100.4
(GHC 9.8.1) to filepath-1.4.2.2
(GHC 9.4.8):
$ ghci-9.4.8
GHCi, version 9.4.8: https://www.haskell.org/ghc/ :? for help
ghci> import qualified System.FilePath.Windows as W
ghci> W.takeDirectory "//?/a:"
"//?/a:"
$ ghci-9.8.1
GHCi, version 9.8.1: https://www.haskell.org/ghc/ :? for help
ghci> W.takeDirectory "//?/a:"
"//?/"
Can you comment whether this is also to be considered a regression?
//?/
rarely behaves correctly, but yes, it's a regression.
In one of my projects I've been trying to migrate from home-grown low-level helpers to
filepath
API (e. g.,takeExtension fn == ".foo"
instead of".foo" `B.isSuffixOf` fn
) but failed miserably: performance penalties were too harsh, especially on Windows.I fixed some low-hanging fruits, and I think results are quite good: many functions are twice faster now, and
System.OsPath
on Windows is often 10x-20x faster. Here is a detailed report: