typelead / eta-hackage

A set of patches to make Hackage compatible with the Eta language.
64 stars 31 forks source link

Patch dhall-1.19.1 with createDirectoryIfMissing #137

Closed jneira closed 5 years ago

jneira commented 5 years ago

Hi, i am getting an strange error in a circleci eta docker env:

 Exception: user error (The import failed but it had to success. Dhall exception: 
        ↳ ./../dhall-lang/tests/import/success/issue553A.dhall sha256:e2d014696fb7d773727ae5aa42dc20bbd2447ea82bcb5971ccbb7763906edace
        JException java.nio.file.FileAlreadyExistsException: /root/.cache)

The code being executed is:

directoryExists <- liftIO (Directory.doesDirectoryExist directory)

if directoryExists
   then do
      permissions <- liftIO (Directory.getPermissions directory)

      guard (accessible permissions)

   else do
      assertDirectory (FilePath.takeDirectory directory)

      liftIO (Directory.createDirectory directory)

      liftIO (Directory.setPermissions directory private)

As it only tries to create the directory when it doesnt exist i guess Directory.doesDirectoryExist is not working correctly. It calls in turn to Files.isDirectory which javadoc states:

Tests whether a file is a directory.
The options array may be used to indicate how symbolic links are handled for the case that the file is a symbolic link. By default, symbolic links are followed and the file attribute of the final target of the link is read. If the option NOFOLLOW_LINKS is present then symbolic links are not followed.

Where is it required to distinguish an I/O exception from the case that the file is not a directory then the file attributes can be read with the readAttributes method and the file type tested with the BasicFileAttributes.isDirectory() method.

It behaves identically to Directory.doesDirectoryExist so dhall itself should fail in circleci as well: it seems in that env Directory.doesDirectoryExist returns false due a IOException (and not cause it really doesnt exist). No time to test it so let's see first if the patch works for etlas and then we can try to apply upstream.

rahulmutt commented 5 years ago

Thanks for the patch @jneira! Yes, it's a good idea to push that upstream because a race condition can happen between doesDirectoryExist and createDirectory if there are multiple processes/threads running it in parallel.

jneira commented 5 years ago

@rahulmutt I am afraid that now circleci test hangs without output more than 10 minutes. However maybe it is cause for another error after creating the cache directory succesfully

rahulmutt commented 5 years ago

So I take it that this occurs only on CI (so linux)? Did you observe this locally on your Windows machine(s)? I can give this a try on my mac too.

jneira commented 5 years ago

Yeah, it doesnt hang in windows but in the file with the import hashed i always get a

Importing fieldOrderA:                                      FAIL
   Exception: user error (The import failed but it had to success. Dhall exception: C:\Users\Javier\AppData\Local\dhall\4a7866b88389e18cf481b525544fd7903325252faf3a86c8fdc981298c788a9b: openBinaryFile: resource busy (file is locked))
rahulmutt commented 5 years ago

@jneira Can you file an issue on this with reproduction steps? It's not clear what exactly I have to run to investigate this.

jneira commented 5 years ago

Dont worry, i put it here incorrecly, it is included in https://github.com/typelead/eta/issues/915