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

Fix cptree to handle symlinks correctly #344

Merged
merged 3 commits into from
Feb 7, 2019
Merged

Conversation

Gabriella439
Copy link
Owner

Fixes #343

If cptree encounters a symlink, just copy the symlink instead of copying
the file it points to or descending into any directory it points to.

Fixes #343

If `cptree` encounters a symlink, just copy the symlink instead of copying
the file it points to or descending into any directory it points to.
@@ -1098,17 +1098,39 @@ symlink a b = liftIO $ createSymbolicLink (fp2fp a) (fp2fp b)
-- | Copy a directory tree
cptree :: MonadIO io => FilePath -> FilePath -> io ()
cptree oldTree newTree = sh (do
oldPath <- lstree oldTree
let isNotSymbolicLink path = do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about

let isNotSymbolicLink = fmap (not . PosixCompat.isSymbolicLink) . lstat

-- The `system-filepath` library treats a path like "/tmp" as a file and not
-- a directory and fails to strip it as a prefix from `/tmp/foo`. Adding
-- `(</> "")` to the end of the path makes clear that the path is a
-- directory
Just suffix <- return (Filesystem.stripPrefix (oldTree </> "") oldPath)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not

let Just suffix = Filesystem.stripPrefix (oldTree </> "") oldPath

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mberndt123: They are not equivalent. The reason why is that Haskell interprets an irrefutable pattern match in a let binding differently than when binding a value in a do block.

An irrefutable pattern match in a let binding:

let Just x = foo

... is the same as:

let x = case foo of
        Just y -> y
        Nothing -> error "…: Irrefutable pattern failed for pattern Just x"

... whereas an irrefutable pattern match in the bind of a do block:

Just x <- return foo

... is the same as:

r <- return foo

x <- case r of
    Just y  -> return y
    Nothing -> fail "Pattern match failure in do expression at …"

These differ because fail might not be the same as error (in fact, it's very common for fail to not be error). For example, for any list-like Monad (including Shell), fail _ = empty so an irrefutable pattern match when binding a value in a do block actually gracefully degrades to returning a total (albeit empty) result, whereas an irrefutable pattern match in a let binding gives a partial result (i.e. error) which is not intended to be recoverable.

So that's why that codde does the whole dance of wrapping the value in return before attempting an irrefutable pattern match. Another way it could have been written is to explicitly desugar it, like this:

x <- case Filesystem.stripPrefix (oldTree </> "") oldPath of
    Nothing -> empty
    Just x  -> return x

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks for explaining!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're welcome!

@Gabriella439 Gabriella439 merged commit e985498 into master Feb 7, 2019
@Gabriella439 Gabriella439 deleted the gabriel/fix_cptree branch February 7, 2019 13:31
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

Successfully merging this pull request may close these issues.

2 participants