Remove ‘Path.IO’, switch to ‘path-io’ package

[mrkkrp]

For previous discussion, please see #1701.

---

This PR eliminates Stack's ad-hoc Path.IO module and replaces it with path-io package. The package provides well-typed interface to directory and temporary plus commonly useful functions like recursive copying of directories. IOW, everything that a user of path may need.

With respect to function names, path-io preserves names from directory and temporary, so it's easier for newcomers to get started (one difference is that “directory” is replaced with “dir” like everywhere in path). Stack's Path.IO on the other hand introduces its own terminology for some actions, so we have different names for the same things in path-io and Stack's Path.IO.

Most of the time there is direct correspondence, so simple renaming is enough, but sometimes there is subtle semantic differences, which I'll address shortly to make reviewing easier.

This may look like a massive PR, but is not. We have several recurring patterns that the code base that are just (almost mechanically) replaced with alternative solutions. Here, I list all those patters and explain some subtle moments and why new functions are equivalent (I don't want Stack users to blame me for breaking the tool, after all, so I've approached this with all my care):

• createTree → createDirIfMissing True †
• listDirectory → listDir †
• fileExists → doesFileExist †
• dirExists → doesDirExist †
• removeFileIfExists → forgivingAbsence' . removeFile. forgivingAbsence' is a general way to ignore doesNotExistErrorType exceptions. Other exceptions propagate (we don't want to silence something unexpected and important). Instead of creating a bunch of functions with IfExists suffix, I decided to make a higher-level helper.
• withCanonicalizedSystemTempDirectory → withSystemTempDir. Difference here is that the former function canonicalizes argument of callback, while the replacement just uses parseAbsDir. System temp directory is always Path Abs Dir, so the argument should be already in correct normal form, unless directory is broken, so additional canonicalization is not necessary or will not make difference anyway.
• resolveFileMaybe → forgivingAbsence . resolveFile. forgivingAbsence returns Nothing if its argument throws doesNotExistErrorType, while resolveFileMaybe first checks “manually” whether file exists. I think the new approach is better in a way. The same applies to resolveDirMaybe.
• removeTree → removeDirRecur † Sometimes with above-mentioned forgivingAbsence wrapper that corresponds to removeTreeIfExists.
• copyDirectoryRecursive → copyDirRecur. Almost the same. The latter tries to preserve permissions where possible, while the former does not care.
• getAppUserDataDir is now wrapped, so there is no point in marshalling types to call it.
• getWorkingDir → getCurrentDir †
• parseRelAsAbsDir → resolveDir' (and the same for files). Effectively the same, although code looks a bit different.

† — means that these things are just the same on the source code level.

Somewhere I removed MonadThrow m constraint when it already had MonadCatch m, because the former is super class of the latter.

---

Finally, please note that I've put path-io-0.3.0 in extra-deps for now. As soon as Stackage builds it, you can fix this.

Reference for reviewer (I hope docs will be there by the time you start reviewing this): https://hackage.haskell.org/package/path-io

[mrkkrp]

I rebased on current master and fixed some stuff so GHC 7.8 should be happy now. However for some reason it failed to install hspec-discover-2.2.2 this time. Restart is necessary.

[sjakobi]

@mrkkrp: To fix the Travis build with stack and GHC-7.8, you'll need to add the path-io-0.3 extra-dep to stack-7.8.yaml, too.

Regarding the Travis build with OSX build I'm not sure what's wrong.

[sjakobi]

That's quite a bunch of work, @mrkkrp! I really look forward to using path-io in other places too!

Regarding the replacement for withCanonicalizedSystemTempDirectory I think you should take a look at #1019. It turns out that System.Directory.getTemporaryDirectory can in some cases return uncanonicalized paths.

There are two more things that I feel could be more "ergonomic":

• Because I've never seen anyone write createDirIfMissing False I don't even know what purpose the Bool argument serves (I could look up the documentation of course). Because of this I'd like to have a synonym for createDirIfMissing True. If you don't want to add it to path-io, I think it should go to Path.IO.Extra inside stack.
• I really like forgivingAbsence but I think I'll have a hard time remembering whether I should use that or forgivingAbsence' instead. How about ignoringAbsence or another name that distinguishes the two functions by more than a '?

[mrkkrp]

Great ideas, I will address all these things in path-io-0.3.1 and update the PR.

[mrkkrp]

I've added ignoringAbsence and ensureDir functions in 0.3.1. I have also made functions that can be influenced by environment variables (getHomeDir and getTempDir) smarter in that they use resolveDir' internally instead of parseAbsDir. This however makes getTempDir sensitive to whether the temporary directory actually exists (because of canonicalization). getHomeDir has been checking this always anyway. I wonder if there may be adverse repercussions of such decision. I could use makeAbsolute instead. What do you think, @sjakobi, what is better?

[sjakobi]

I have also made functions that can be influenced by environment variables (getHomeDir and getTempDir) smarter in that they use resolveDir' internally instead of parseAbsDir. This however makes getTempDir sensitive to whether the temporary directory actually exists (because of canonicalization). getHomeDir has been checking this always anyway. I wonder if there may be adverse repercussions of such decision. I could use makeAbsolute instead. What do you think, @sjakobi, what is better?

I think that if you provide functions with similar names and similar types as existing functions in System.Directory you're making an implicit promise that the semantics of the new functions are similar, too. If you want to provide different semantics you should make the distinction clear in the name, too.

IMO, by using parseAbsDir or resolveDir' in getTempDir you'd introduce additional failure modes and end up with very different semantics than those of System.Directory.getTemporaryDirectory. By using makeAbsolute instead, you stop relying on getting an absolute or existing path but you still rely on getCurrentDirectory working… I guess this best compromise between the original semantics of getTemporaryDirectory and ensuring that the result is indeed an absolute path. If you want to avoid the compromise or potential confusion, you should probably use a different name or tell users to use something like withCanonicalizedSystemTempDir instead.

For getHomeDir I think you should expect System.Directory.getHomeDirectory to provide an existing, absolute path. I don't think that it's necessary to add the notion of canonicalization here.

I also think that we should move this discussion to path-io if you would like to continue, @mrkkrp.

[mrkkrp]

The only difference now that temporary directory should exist or doesNotExistErrorType will be thrown. I ended up using resolveDir' because the system temporary directory should exist anyway or otherwise withTempDir (which withSystemTempDir uses) would not be able to create a temporary directory inside it. I mentioned this in the docs of getTempDir explicitely. Also, usually progpammers get temp directory with getTempDir and anyway assume that it exists. So, I guess my sin here is not so big :-)

getHomeDir can be influenced by environment variable HOME as per directory's docs, so canonicalization may be a good thing to do here too.

All in all, everything should work now.

---

Good, it passed all checks this time.

[mrkkrp]

Ping (?) Can someone review it and tell me what to fix or merge it?

[mgsloan]

@mrkkrp Looks good to me, thanks for working on this!

I'm going to go ahead and merge to avoid future merge conflicts. I do have the following thoughts:

• Is ignoreAbsence really necessary? Things that drop the output are usually postfixed with _, like mapM_ vs mapM. However, in this case I don't see this pattern being common enough to warrant a function (may as well use _ <- or void $)
• I prefer ignoreMissing. Here's my reasoning:
• ignore is better than forgiving. forgiving makes it sound like something bad happened. In this case, instead we're just saying that this thing happening isn't bad. Forgiveness also tends to refer to something that happened in the past, not something that happened right now. ignore makes it a little clearer that it's just ignoring that condition.
• missing is better because it's the terminology used by System.Directory, and the exceptions thrown.

[mgsloan]

I don't feel too strongly about the naming, though. Feel free to address these tweaks in another PR.

[mrkkrp]

@mgsloan, I have to admit that forgiving is not most obvious variant technically, it has something to do with my sense of humor too (I hope it's not entirely lost in translation).

The only reason to have ignoringAbsence is to avoid “unused value” warnings in do notation (I think these warnings are a good thing, so we should use different function to mark explicitly where we do not expect useful output). I initially used void . forgivingAbsence idiom, but it's too long to repeat it every time.

I don't know if changing the API is a good idea now, it seems like people are beginning to use the package.

[mgsloan]

The only reason to have ignoringAsence is to avoid “unused value” warnings in do notation (I think these warnings are a good thing, so we should use different function to mark explicitly where we do not expect useful output). I initially used void . forgivingAbsence idiom, but it's too long to repeat it every time.

Ohh, I actually didn't consider that forgivingAbsence . removeFile would return a Maybe (). Hmm, yes, I think this naming is fine. Good stuff!

[mgsloan]

@mrkkrp Also, we've added you to the stack repository! Feel free to make changes you're confident in directly to the repo. So, for example, if path-io's API changes, it'd be quite fine to push a refactoring to stack.

[mrkkrp]

@mgsloan, Thank you!