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

Trouble with non-ASCII filenames on Windows #90

Closed
Blaisorblade opened this issue Aug 14, 2016 · 4 comments
Closed

Trouble with non-ASCII filenames on Windows #90

Blaisorblade opened this issue Aug 14, 2016 · 4 comments

Comments

@Blaisorblade
Copy link
Contributor

I just minimized commercialhaskell/stack#2491 to a bug in the Yaml library. Quoting myself there:

René/foo.yaml

true
...

testYaml.hs:

import qualified Data.Yaml as Yaml

loadYaml :: FilePath -> IO (Either Yaml.ParseException Bool)
loadYaml = Yaml.decodeFileEither

main :: IO ()
main = do
  print =<< loadYaml "René\\foo.yaml"
  return ()
$ stack runhaskell testYaml.hs
Left (InvalidYaml (Just (YamlException "Yaml file not found: Ren\233\\foo.yaml")))

(That stack runhaskell is inside a project with yaml installed, in particular stack master on lts-6.0, hence it uses https://www.stackage.org/lts-6.0/package/yaml-0.8.17.1; the changelog at https://hackage.haskell.org/package/yaml-0.8.18.1/changelog does not mention this issue, nor does this issue tracker).

What I've shown is on a fresh Win10 VM, insideGit Bash coming from Git for Windows.
Already minimized to a bug in the Yaml library.

Analysys

I expect we end up here:

yaml/Text/Libyaml.hs

Lines 525 to 533 in f88b1bf

$ withCString file $ \file' -> withCString "r" $ \r' ->
c_fopen file' r'
if file' == nullPtr
then do
c_fclose_helper file'
c_yaml_parser_delete ptr
free ptr
throwIO $ YamlException
$ "Yaml file not found: " ++ file

That uses withCString and fopen, and withCString is documented here as "using a locale-dependent encoding".
https://www.stackage.org/haddock/lts-6.0/base-4.8.2.0/Foreign-C-String.html

Also note: the source code is UTF8, and that's required to have it accepted by stack runhaskell, whether I use codepage 437 (the default) or 65001.

@Blaisorblade
Copy link
Contributor Author

Also checked the diff, yaml/0.8.17.1...yaml/0.8.18.1, nothing relevant; and retested with runhaskell after adding - yaml-0.8.18.1 to stack.yaml's extra-deps, with the same result.

@snoyberg
Copy link
Owner

Good catch. Do you want to take a stab at a PR to resolve?

On Sun, Aug 14, 2016 at 6:11 PM, Paolo G. Giarrusso <
notifications@github.com> wrote:

Also checked the diff, yaml/0.8.17.1...yaml/0.8.18.1
yaml/0.8.17.1...yaml/0.8.18.1,
nothing relevant; and retested with runhaskell after adding -
yaml-0.8.18.1 to stack.yaml's extra-deps, with the same result.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#90 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADBByu4YEzOok2zs-iqab4FCCPKguuvks5qfzAYgaJpZM4Jj6Y0
.

Blaisorblade added a commit to Blaisorblade/yaml that referenced this issue Aug 14, 2016
Working on snoyberg#90. Unfinished.

Idea: reuse low-level API from GHC internals, since it has already
tested low-level code to deal with non-ASCII filenames on different
platforms.

Current status:
- Only for reading files.
- This still passes tests on OS X. To test on Windows.
Blaisorblade added a commit to Blaisorblade/yaml that referenced this issue Aug 14, 2016
Working on snoyberg#90. Unfinished.

Idea: reuse low-level API from GHC internals, since it has already
tested low-level code to deal with non-ASCII filenames on different
platforms.

Current status:
- Only for reading files.
- This still passes tests on OS X (on lts-6.0). To test on Windows.
@Blaisorblade
Copy link
Contributor Author

Good catch. Do you want to take a stab at a PR to resolve?

OK. I'm taking a look at (ab)using GHC's internals for this—base has tested code dealing with this.
But shouldn't you be on vacation 😃 ?

@snoyberg
Copy link
Owner

Heh, indeed

On Sun, Aug 14, 2016, 8:32 PM Paolo G. Giarrusso notifications@github.com
wrote:

Good catch. Do you want to take a stab at a PR to resolve?

OK. I'm taking a look at (ab)using GHC's internals for this—base has
tested code dealing with this.
But shouldn't you be on vacation 😃 ?


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#90 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADBB7RTPwDVMFWuKyeVB0Rc71pWcDJiks5qf1EsgaJpZM4Jj6Y0
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants