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

Adding files that are symlinks directly #2839

Closed
Kubuxu opened this issue Jun 11, 2016 · 13 comments
Closed

Adding files that are symlinks directly #2839

Kubuxu opened this issue Jun 11, 2016 · 13 comments
Assignees
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@Kubuxu
Copy link
Member

Kubuxu commented Jun 11, 2016

Unix commands when you reference a symlink directly resolve it. IPFS does not.

Example:

mkdir /tmp/hello
echo "World" > /tmp/hello/world
ln -s /tmp/hello /tmp/sym
cd /tmp/sym
ipfs add -r .

cp -r . ../sym1
ls -l .. | grep sym

cp resolved the symlink and copied whole directory, IPFS added an meaningless symlink.

@Kubuxu Kubuxu changed the title Recursive adding from directory that is symlink is broken Adding files that are symlinks directly Jun 11, 2016
@Kubuxu Kubuxu removed the regression label Jun 11, 2016
@whyrusleeping whyrusleeping added the kind/bug A bug in existing code (including security flaws) label Jun 11, 2016
@Kubuxu Kubuxu added this to the Ipfs 0.4.3 milestone Jun 23, 2016
@Kubuxu Kubuxu added the status/in-progress In progress label Jun 23, 2016
@whyrusleeping
Copy link
Member

@Kubuxu this is resolved now, right?

@kevina
Copy link
Contributor

kevina commented Aug 26, 2016

I am reopening this issue because the fix causes problems for me in my filestore code.

cp -r (at least on unix) does not resolve symbolic links unless the path end in a '/'. For example

mkdir tmp
cd tmp
mkdir dira
touch dira/afile
ln -s dira dirb
cp -r dirb dirc # copies the link
cp -r dirb/ dird
ls -l 
drwxr-xr-x 2 kevina kevina 4096 Aug 26 04:15 dira
lrwxrwxrwx 1 kevina kevina    4 Aug 26 04:15 dirb -> dira
lrwxrwxrwx 1 kevina kevina    4 Aug 26 04:15 dirc -> dira
drwxr-xr-x 2 kevina kevina 4096 Aug 26 04:15 dird

The original issue reported is due to a bug in how the special case of . is handled and is easy to fix (and I will happy to do so).

Please also see my comments on the pull request #2897 as it is now seams impossible to add symbolic links to the unixfs protobuf.

The issue comes down to what do you want this to do:

echo "Hello World!" > touch afile
ln -s afile link
ipfs add link

Due you want this at add the symbolic link (the original behavior) or the file the link is pointed to (the new behavior)?

How I deal with this change in the filestore code will depend on the answer to that question.

@jbenet, you might want to take a look at this.

@kevina kevina reopened this Aug 26, 2016
@Kubuxu
Copy link
Member Author

Kubuxu commented Aug 26, 2016

You can add symbolic links inside directories.
We can revert it and think about it more.

Is there any usage pattern where user wants to add a symbolic link, instead of the file referenced by this link if he references the symbolic link directly?

Looks symlinks to files are always resolved.

 …/tmp/syms  echo Hello > file
 …/tmp/syms  ln -s file sym
 …/tmp/syms  ls
total 8.0K
-rw-r--r-- 1 kubuxu kubuxu 6 Aug 26 11:01 file
lrwxrwxrwx 1 kubuxu kubuxu 4 Aug 26 11:01 sym -> file
 …/tmp/syms  cp sym cp_sym
 …/tmp/syms  ls
total 12K
-rw-r--r-- 1 kubuxu kubuxu 6 Aug 26 11:01 cp_sym
-rw-r--r-- 1 kubuxu kubuxu 6 Aug 26 11:01 file
lrwxrwxrwx 1 kubuxu kubuxu 4 Aug 26 11:01 sym -> file
 …/tmp/syms  cat cp_sym
Hello
 …/tmp/syms  mkdir dir
 …/tmp/syms  echo World > dir/f
 …/tmp/syms  ln -s dir sdir
 …/tmp/syms  cp -r sdir cp_sdir
 …/tmp/syms  cp -r sdir/ cp2_sdir
 …/tmp/syms  ls
total 20K
drwxr-xr-x 1 kubuxu kubuxu 2 Aug 26 11:03 cp2_sdir/
lrwxrwxrwx 1 kubuxu kubuxu 3 Aug 26 11:03 cp_sdir -> dir/
-rw-r--r-- 1 kubuxu kubuxu 6 Aug 26 11:01 cp_sym
drwxr-xr-x 1 kubuxu kubuxu 2 Aug 26 11:02 dir/
-rw-r--r-- 1 kubuxu kubuxu 6 Aug 26 11:01 file
lrwxrwxrwx 1 kubuxu kubuxu 3 Aug 26 11:03 sdir -> dir/
lrwxrwxrwx 1 kubuxu kubuxu 4 Aug 26 11:01 sym -> file
 …/tmp/syms  

@jbenet
Copy link
Member

jbenet commented Aug 27, 2016

@Kubuxu

Is there any usage pattern where user wants to add a symbolic link, instead of the file referenced by this link if he references the symbolic link directly?

  • Try git add <symlink>, it adds the symlink, and with good reason. The point is to be able to restore it into another file system, exactly as is.
  • Imagine also usage in the mfs/files part, where i ipfs add <symlink> and then link in that has into the right location in my ipfs files.
  • By default, add should store the symlinks NOT the files. you can add a flag to make the resolution happen if you want it to, and maybe a printed warning for the user (to stderr) to use the flag if they meant to add the file.
  • Consistency is also very important. adding symlinks inside directories, but adding the targets of the links when not in directories is very weird.

@jbenet
Copy link
Member

jbenet commented Aug 27, 2016

This needs to be reverted.

@kevina
Copy link
Contributor

kevina commented Aug 27, 2016

@Kubuxu I am going to go ahead and create a pull request that reverts the fix for this and directly fixes the problem described in the description so adding the current directory works as expected.

@Kubuxu
Copy link
Member Author

Kubuxu commented Aug 27, 2016

@kevina Thanks, can you base it of the v0.4.3-rc3 tag and PR to version/0.4.3-rc4.

@kevina
Copy link
Contributor

kevina commented Aug 27, 2016

@Kubuxu will do

@kevina
Copy link
Contributor

kevina commented Aug 27, 2016

Okay have a look at #3135 .

whyrusleeping added a commit that referenced this issue Sep 1, 2016
@whyrusleeping
Copy link
Member

@kevina @Kubuxu has this been resolved?

@kevina
Copy link
Contributor

kevina commented Sep 14, 2016

The original issue of "Recursive adding from directory that is symlink is broken" is. If you reference a symlink directly it will still be added as a symbolic link, I think this is the desirable behavior. @Kubuxu can we close this?

@kevina kevina assigned Kubuxu and unassigned kevina Sep 14, 2016
@Kubuxu
Copy link
Member Author

Kubuxu commented Sep 14, 2016

Yes.

@sankesireddy
Copy link

Hi,
When I'm trying to install IPFS by using "npm install orbit-db ipfs" I'm encountering this error. ERR! Error: EPERM: operation not permitted, symlink 'C:\Users\Student\OB\node_modules\go-ipfs\go-ipfs\ipfs.exe' ->
However, when I install just the IPFS following "npm install IPFS", there aren't any issues.

Can someone please help me fix this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

No branches or pull requests

5 participants