haskell / directory

Platform-independent library for basic file system operations
https://hackage.haskell.org/package/directory
Other
58 stars 47 forks source link

on Windows, copyFile makes empty source and destination files when the source file is absent #177

Closed hsenag closed 5 months ago

hsenag commented 5 months ago

on Windows,

copyFile nonexistent shouldnotexist

succeeds and leaves 0-byte files for both nonexistent and shouldnotexist.

I would expect it to throw an exception and not create either source or destination file, which is what happens on Linux and MacOS.

The behaviour seems to have started in directory-1.3.8.0 and I bisected it to 78b3e59. I'm fairly sure it's directory because I did things like holding Win32 and trying with a range of GHCs. With default versions we only noticed it starting with GHC 9.6, but I get the same behaviour with GHC 9.4.8+Win32 2.13.3.0+directory 1.3.8.0 (for example).

I used this test program to track down where the problem started:

module Main where

import Control.Exception
import System.Directory
import System.IO.Error (isDoesNotExistError)

main :: IO ()
main = do
  removeFile "nonexistent.source" `catch` (\e -> if isDoesNotExistError e then return () else throwIO e)
  removeFile "shouldnotexist.target" `catch` (\e -> if isDoesNotExistError e then return () else throwIO e)
  (copyFile "nonexistent.source" "shouldnotexist.target" >> putStrLn "BAD: copyFile did not throw") `catch` (\e -> if isDoesNotExistError e then putStrLn "OK: copyFile threw" else throwIO e)
  doesFileExist "nonexistent.source" >>= \b -> if b then putStrLn "BAD: nonexistent.source exists" else putStrLn "OK: nonexistent.source does not exist"
  doesFileExist "shouldnotexist.target" >>= \b -> if b then putStrLn "BAD: shouldnotexist.target exists" else putStrLn "OK: shouldnotexist.target does not exist"
  removeFile "nonexistent.source" `catch` (\e -> if isDoesNotExistError e then return () else throwIO e)
  removeFile "shouldnotexist.target" `catch` (\e -> if isDoesNotExistError e then return () else throwIO e)
  return ()

ref: https://bugs.darcs.net/issue2721

hasufell commented 5 months ago

I can confirm. The relevant code is here:

https://github.com/haskell/directory/blob/a97a8a8f30d652f972192122fd5f459a147c13e5/System/Directory/OsPath.hs#L746-L772

hasufell commented 5 months ago

Fix is here: https://github.com/haskell/directory/pull/178

However, I suggest @Rufflewind that we consider to use the package file-io for this in the future so that we can consolidate such low-level functionality. Having complicated Win32 code spread around different packages is problematic.

The openExistingFile function from that package does not suffer from this issue.

Rufflewind commented 5 months ago

Released: https://hackage.haskell.org/package/directory-1.3.8.5

hasufell commented 5 months ago

@bgamari do you have visibility which GHC versions ship with this bug?

hsenag commented 5 months ago

Thanks for the quick fix/release!

Rufflewind commented 5 months ago

This affects directory 1.3.8.0 through 1.3.8.4, which maps to GHC 9.6.1 through 9.10.1 according to https://gitlab.haskell.org/ghc/ghc/-/wikis/commentary/libraries/version-history

bgamari commented 5 months ago

I have opened GHC#24843 to ensure that this bump is performed in future minor releases.

andreasabel commented 5 months ago

This affects directory 1.3.8.0 through 1.3.8.4,

Would be good to add this info in the release notes.