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

cp_r loops #154

Open
andreasabel opened this issue Aug 18, 2017 · 3 comments
Open

cp_r loops #154

andreasabel opened this issue Aug 18, 2017 · 3 comments

Comments

@andreasabel
Copy link
Collaborator

cp_r "A" "A/B" loops, creating an infinitely deep directory structure.
In contrast, cp -r A A/B on the shell will report an error.

cp: cannot copy a directory, 'A', into itself, 'A/B/A'

The problem with cp_r is that copied directories will be copied again. This can be solved by separating the procedure into two phases: making a list what has to be copied, and then copying.
I just fixed the same problem in Agda, see: agda/agda@443e603 .

Here is the test program for cp_r I used to confirm the bug:

{-# LANGUAGE LambdaCase #-}

import System.Environment (getArgs)
import System.Exit
import Filesystem.Path.CurrentOS (decodeString)

import Shelly (shelly, cp_r)

main :: IO ()
main = getArgs >>= \case
  [src , dest] -> shelly $ cp_r (decodeString src) (decodeString dest)
  _ -> do
   putStrLn "Usage: ShellyCpR src dest\nShould behave like cp -r src dest"
   exitFailure
@ulikoehler
Copy link

I can confirm this issue. Although it probably would be considered a corner case (as you can't do it using cp -r, see below), I think it should probably be fixed (in the sense that it should behave equivalently to cp -r, i.e. it should fail).

If you run cp -r A A/B, you get this error message:

'A' -> 'A/B'
cp: cannot copy a directory, 'A', into itself, 'A/B'

I'd like to hear your thoughts on whether you think this should actually copy (by building a list first) or rather fail similarly to cp -r.

@andreasabel
Copy link
Collaborator Author

The cp man page does not specify the behavior in this case.
I opted for doing the copy in a safe way rather than failing, since it is compatible with what the alternative packages do that I list at agda/agda#2705 (comment) .
Actually ,cp -r does the copying, in addition to giving the error message.

@ulikoehler
Copy link

@andreasabel I can confirm that in my manpage doesnt list anything, but I can not confirm that copying is actually done. My cp --version is cp (GNU coreutils) 8.25. Here's a test script:

#!/bin/sh
# Cleanup
rm -rf A
# Create test dir
mkdir -p A/B
echo "foo" > A/bar.txt
# Test copy
cp -r A A/B
# This should not error if copying is actually performed
cat A/B/A/bar.txt # <-- error
# NOTE: A/B/A is created
# Print version
cp --version 

What's your output?

Note that A/B/A is created (but A/B/A/bar.txt is not!). I'm inclined to considering it a bug in cp, but it's probably not very critical for most (if not all) people.

Although I get your point to other Haskell packages, as Shellys function is not called copyDir etc as the examples you pointed to but cp_r I'd expect it to have a behaviour very similar or identical to cp_r (which includes that ininite loops must not happen). Even besides that, I'd consider cp as the standard for file copying on unixoid systems and I believe most people would compare copyDir functions to cp as well. That doesn't prohibit a specific function like copyDirSelfCopy (name TBD) which lists files prior to copying.

Note that there are corner cases where listing the files is not feasible due to RAM usage (i.e. a huge number of files), although I'd say they are pretty rare in reality.

You can find the relevant section in the source code here:
https://git.savannah.gnu.org/cgit/coreutils.git/tree/src/copy.c#n2267
It clearly lists goto un_backup;, which I'd consider a sign of aborting the copy considering what un_backup lists: https://git.savannah.gnu.org/cgit/coreutils.git/tree/src/copy.c#n2881

Also, due to the comment, I'm pretty sure the current behaviour is actually intended by the coreutils authors.

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

No branches or pull requests

2 participants