copy icon indicating copy to clipboard operation
copy copied to clipboard

copy.Deep does not follow multiple levels of symbolic links

Open ob opened this issue 3 years ago • 13 comments

Run the following bash script:

#!/bin/bash

rm -rf src dst
mkdir -p src dst
cd src
ln -s adir/afile a
ln -s bdir adir
mkdir bdir
touch bdir/a

to create the following structure:

❯ tree src
src
├── a -> adir/afile
├── adir -> bdir
└── bdir
    └── a

2 directories, 2 files

Then use the following program:

❯ cat main.go
package main

import (
        "log"

        "github.com/otiai10/copy"
)

func main() {
        opt := copy.Options{OnSymlink: func(string) copy.SymlinkAction { return copy.Deep }}
        if err := copy.Copy("src/a", "dst/b", opt); err != nil {
                log.Fatal(err)
        }

}

finally, run it:

❯ go run main.go
2020/12/07 18:29:30 lstat adir/afile: no such file or directory
exit status 1

ob avatar Dec 08 '20 02:12 ob

Hmm, good catch. Thank you!

otiai10 avatar Dec 08 '20 07:12 otiai10

I understand this problem as follows, do you think it's right? @ob

src of link \ action Deep Shallow
Absolute Copy all src files physically Create linking to the src
Relative ⚠️ Same but src files might NOT exist YET ⚠️ Same but src files might NOT exist YET ⚠️

otiai10 avatar Mar 03 '21 00:03 otiai10

I think the problem is that Deep copy should keep resolving the symlink until it gets to a file that is not a symlink no?

So maybe a loop here?

ob avatar Mar 03 '21 04:03 ob

I might need to check and structure our issue first. Thank you for your response and reporting this issue ;)

otiai10 avatar Mar 03 '21 06:03 otiai10

I think the symlink copying might be broken completely. https://github.com/otiai10/copy/blob/main/test_setup.go#L13 creates an invalid symlink:

$ ls -al test/data/case03
total 12
drwxrwxr-x  2 cesium cesium 4096 Mar 19 18:20 .
drwxrwxr-x 15 cesium cesium 4096 Mar 19 18:08 ..
lrwxrwxrwx  1 cesium cesium   16 Mar 19 18:20 case01 -> test/data/case01
-rw-rw-r--  1 cesium cesium   28 Mar 19 17:55 README.md

it should be pointing to ../case01.

I started trying to fix this here but haven't fully worked out how to get shallow copies to work correctly. If they're relative anyway, they need to fix the relative path to the destination.

benmoss avatar Mar 19 '21 23:03 benmoss

Thanks! Just FYI : https://golang.org/pkg/path/filepath/#EvalSymlinks

otiai10 avatar Mar 22 '21 07:03 otiai10

There's a very difficult issue with the Shallow copy that I'm not sure how to solve.

With case09, you have a symlink to a file in the same directory:

$ ls -l test/data/case09/symlink
lrwxrwxrwx 1 cesium cesium 9 Mar 19 17:55 test/data/case09/symlink -> README.md

With case03 you have a symlink to a file in a different directory:

$ ls -al test/data/case03/case01
lrwxrwxrwx 1 cesium cesium 16 Apr  9 12:54 test/data/case03/case01 -> test/data/case01

In case09's case, I think you would expect the new symlink to be pointing to the copied README.md.

With case03 the target of the symlink isn't being copied, so what do you do? Create a broken symlink to test/data.copy/case01? Create a valid symlink to test/data/case01? Error?

benmoss avatar Apr 09 '21 16:04 benmoss

I think for shallow copies creating broken symlinks is the right answer. I believe that's how rsync works.

ob avatar Apr 09 '21 20:04 ob

The issue with Shallow copies can be fixed by either making the link src relative to the location of the link itself -- not curent working directory -- or by making the link src absolute.

Try making this change in setup.go:

`// os.Symlink("test/data/case01", "test/data/case03/case01")

os.Symlink("../case01", "test/data/case03/case01")

`

I also theorized that the link above, being a directory link, might be different, so I created a file link, with the same result (problem + fix):

`// os.Symlink("test/data/case01/README.md", "test/data/case03/case01readme")

os.Symlink("../case01/README.md", "test/data/case03/case01readme")

`

The fix would also be needed in lcopy() but computing a relative location dynamically from the destination directory for the link isn't simple to me. It would be simpler to just make it absolute.

Also note that test/data.copy/case03/case01 will not be pointing to test/data.copy/case01 because they are not related -- unless you use a relative link. An absolute link would point to test/data/case01. The semantics of copying a directory containing a symbolic link are ambiguous, but could be interpreted as favoring a relative link, so that the structure of the copied directory is the same as the copied directory.

enthor avatar Apr 17 '21 21:04 enthor

P.S. NOTE: I had to modify the algorith when the link source path name is already 'relocatable', in the form "../case01", for example: add

if strings.HasPrefix(origLinkSrc, ".") then just use origLinkSrc as-is, don't rebase it or change to abs, it is good already.

Here is Playground solution for Shallow copy: first, try to re-base the link source as a relative path to the new Symlink location. Then if that does not work, make it an absolute path:

https://play.golang.org/p/QlH-y9MM7zC

` package main

import ( "fmt" "path/filepath" "os" )

func main() { fmt.Println("Hello, playground") origLinkSrc := "test/data/case01" pOrigSymlink := "test/data/case03/case01" pNewSymlink := "test/data.copy/case03/case01" relOrigLinkSrc, err := filepath.Rel(filepath.Dir(pOrigSymlink), origLinkSrc) if err == nil { fmt.Printf("\n relOrigLinkSrc=%v", relOrigLinkSrc) err = os.Symlink(relOrigLinkSrc, pNewSymlink) } else { absOrigLinkSrc, err := filepath.Abs(origLinkSrc) if err == nil { err = os.Symlink(absOrigLinkSrc, pNewSymlink) } } if err != nil { fmt.Printf("\n well...file not found expected on playground!") fmt.Printf("\n err=%v", err) } }

`

enthor avatar Apr 17 '21 22:04 enthor

Regarding Deep copies, I need to test this to be sure, but the option in my fork called "SymlinkBlissfulIgnorance" actually does what I think you want "Deep" to do: it just treats a Symlink as a regular file and copies the linked-to file. And I read in the Unix/Linux manual (somewhere) that link following halts at 250 to prevent malicous linking.

So now that we have established the correct link source path name -- should be relative to the Symlink itself -- everything is set and good to go without Deep having to manually follow a chain of Symlinks. I think that happens automatically when you refer to a Symlink (called "Alias" in MacOS) as a file.

Here is my latest, tested code for "Shallow" Symlink (which I think fixes everything):

`

var absOrigLinkSrc, relOrigLinkSrc, newLinkSrc string
switch {
case filepath.IsAbs(origLinkSrc):
	absOrigLinkSrc = origLinkSrc
case strings.HasPrefix(origLinkSrc, ".") || filepath.Dir(origLinkSrc) == ".":
	// don't mess with these, they're good. 
	// for example: origLinkSrc "../case01" and "README.md" are already
	// relocatable!
	//
	relOrigLinkSrc = origLinkSrc
default:
	relOrigLinkSrc, err = filepath.Rel(filepath.Dir(src), origLinkSrc)
	if err != nil {
		absOrigLinkSrc, err = filepath.Abs(src)
		if err != nil {
			err = errors.New(fmt.Sprintf(
				"Kopy.copyEntityIsSymlink(): error from filepath.Abs(), origLinkSrc=%v, src=%v, dest=%v, err = \n %v",
				origLinkSrc, src, dest, err))
			if kopy.SkipBrokenSymlinks {
				log.Printf("Kopy.copyEntityIsSymlink(): Bypassing broken Symlink. Original err = \n %v",
					err)
				kopy.KopyStats.SymlinksSkipped += 1
				err = nil
			}
			return err
		}
	}
}
newLinkSrc = relOrigLinkSrc
if newLinkSrc == "" {
	newLinkSrc = absOrigLinkSrc
}

`

enthor avatar Apr 18 '21 03:04 enthor

I uploaded my fork: github.com/enthor/kmdrv0 -- my testing is nearing conclusion, leaving tidy up and packaging, etc. See doc.txt for a very thorough review of the differences between it and github.com/otiai10/copy.

I view this as experimental code, though I will use it in my own project. It definitely validates the otiai10/copy algorithm and code except for the minor issues mentioned so far. And also, today I confirmed that while PreserveTimes does not (cannot) work for Symlink files, it does work with Named Pipes. So, as a scientific experiment and proof of the otiai10 approach, this is a success. And also very humbling. Working at this low-level of system details is quite different from my work in Applications Programming.

I welcome any suggestions (except re: verbosity) within the next couple of weeks. Then back to my project: a bespoke, nano-relational self-defining database system (which is already working except for a few features, like an SQL parser, transaction file updates, and a few other minor details :-) After another 6 months on that I plan to return to kmdrv0 and add compares and compression to the repertoire.

BTW, I know bupkus about git and github so I am prone to reloading the entire repository if anything goes wrong. Take care to fork kmdrv0 and and comments if you want to study them because I might mess things up and have to start fresh in github.

Thank you for your great work. It has helped educate me, and I appreciate your substantial efforts thus far.

enthor avatar Apr 20 '21 04:04 enthor

I guess this is still an issue. Ran into it trying to copy a symlink to a ~/.oh-my-zsh, which itself contains a couple of symlinks down there deep.

rfay avatar Jul 01 '22 01:07 rfay