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

`encodeFile` does not truncate an existing file on Windows #178

Closed
mtolly opened this issue Sep 19, 2019 · 7 comments
Closed

`encodeFile` does not truncate an existing file on Windows #178

mtolly opened this issue Sep 19, 2019 · 7 comments

Comments

@mtolly
Copy link

@mtolly mtolly commented Sep 19, 2019

Demo repository at https://github.com/mtolly/yaml-no-truncate
The following program:

module Main where

import qualified Data.Yaml as Y

main :: IO ()
main = do
  Y.encodeFile "test-output" "this is a somewhat long string"
  Y.encodeFile "test-output" "this is shorter"

with this stack.yaml (pointed at current master):

resolver: lts-14.6
packages:
- .
extra-deps:
- git: https://github.com/snoyberg/yaml.git
  commit: 485df7bb08d7ed17a0be363ff493ed27e77fda13
  subdirs: [yaml, libyaml]

produces this file on Windows 7:

this is shorter
at long string

and this on macOS Mojave:

this is shorter

If I replace Y.encodeFile with Y.encode and then B.writeFile (from ByteString) there's no issue.

@snoyberg

This comment has been minimized.

Copy link
Owner

@snoyberg snoyberg commented Sep 22, 2019

Sounds like a missing truncate flag on open file. @Blaisorblade do you think #90 may be relevant?

@mtolly interested in taking a stab at a PR?

@Blaisorblade

This comment has been minimized.

Copy link
Contributor

@Blaisorblade Blaisorblade commented Sep 22, 2019

@snoyberg it seems you're completely right. Line https://github.com/snoyberg/yaml/pull/91/files#diff-fe4d8812ce1a1d403f51795fa362f96bR618 is missing O_TRUNC. I'm not sure why it "works" on macOS, but fdopen("w") could assume O_TRUNC was already passed, so it could be getting confused. (According to docs, this shouldn't work correctly on Linux either).
Beware right now I only checked Linux man pages, so I'm not 100% positive that O_TRUNC works on Windows (but it should, since Windows is POSIXy).

@Blaisorblade

This comment has been minimized.

Copy link
Contributor

@Blaisorblade Blaisorblade commented Sep 22, 2019

Retract: I should have checked the file on master, because that bug you already fixed: 64bf822.

@snoyberg

This comment has been minimized.

Copy link
Owner

@snoyberg snoyberg commented Sep 23, 2019

Huh, interesting. Then I'm not sure what's going on.

@mtolly a great next step here would be a PR that reproduces this problem on CI. One concern I have is that you mention Windows 7 as the test OS, which is not a supported operating system (Microsoft has closed the support window), so I'd rather see a repro that we can see ourselves on a supported OS version.

@snoyberg

This comment has been minimized.

Copy link
Owner

@snoyberg snoyberg commented Nov 6, 2019

I just reproduced this on CI. It seems to only occur with GHC 8.6. I'm guessing there's a GHC bug involved here. Still investigating...

@snoyberg

This comment has been minimized.

Copy link
Owner

@snoyberg snoyberg commented Nov 6, 2019

Sure enough, it seems to be a regression in GHC 8.6. Sample code:

import System.Posix.Internals as Posix
import Control.Exception
import Data.Bits

std_flags = Posix.o_NOCTTY
output_flags = std_flags .|. Posix.o_CREAT .|. Posix.o_TRUNC
write_flags = output_flags .|. Posix.o_WRONLY

main = do
  writeFile "test.txt" "foobarbazbin"
  withFilePath "test.txt" $ \fp -> bracket
    (c_open fp write_flags 0o644)
    c_close
    print
  readFile "test.txt" >>= print

On Windows with GHC 8.6.5, this prints out foobarbazbin as the file contents at the end. On Windows with GHC 8.4.4, or on Linux with GHC 8.6.5, it correctly prints an empty file due to truncation.

I'm going to try a workaround of removing the file before opening for GHC 8.6 and later on Windows.

snoyberg added a commit that referenced this issue Nov 6, 2019
@snoyberg snoyberg closed this in 610a817 Nov 6, 2019
@Blaisorblade

This comment has been minimized.

Copy link
Contributor

@Blaisorblade Blaisorblade commented Nov 6, 2019

FWIW, truncating has subtly different semantics from delete + create (can change permissions, break symlinks, etc.), but it's not obvious how to get to truncate there — maybe c_ftruncate (used
here) is the right API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.