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

copy.Deep does not follow multiple levels of symbolic links #32

Open
ob opened this issue Dec 8, 2020 · 13 comments
Open

copy.Deep does not follow multiple levels of symbolic links #32

ob opened this issue Dec 8, 2020 · 13 comments
Assignees

Comments

@ob
Copy link

ob commented Dec 8, 2020

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 added a commit to ob/copy that referenced this issue Dec 8, 2020
@ob ob mentioned this issue Dec 8, 2020
@otiai10
Copy link
Owner

otiai10 commented Dec 8, 2020

Hmm, good catch. Thank you!

@otiai10
Copy link
Owner

otiai10 commented Mar 3, 2021

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 ⚠️

@ob
Copy link
Author

ob commented Mar 3, 2021

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?

@otiai10
Copy link
Owner

otiai10 commented Mar 3, 2021

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

@otiai10 otiai10 self-assigned this Mar 3, 2021
@otiai10 otiai10 mentioned this issue Mar 3, 2021
@benmoss
Copy link

benmoss commented Mar 19, 2021

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.

@otiai10
Copy link
Owner

otiai10 commented Mar 22, 2021

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

@benmoss
Copy link

benmoss commented Apr 9, 2021

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 benmoss mentioned this issue Apr 9, 2021
@ob
Copy link
Author

ob commented Apr 9, 2021

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

@enthor
Copy link

enthor commented Apr 17, 2021

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
Copy link

enthor commented Apr 17, 2021

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
Copy link

enthor commented Apr 18, 2021

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
Copy link

enthor commented Apr 20, 2021

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.

@rfay
Copy link

rfay commented Jul 1, 2022

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.

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 a pull request may close this issue.

5 participants