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.

Sign up for GitHub

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

Jump to bottom

Resolve symlink if it is directly referenced in cli #2897

Merged
merged 2 commits into from
Jun 28, 2016
Merged

Resolve symlink if it is directly referenced in cli #2897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dafeb3e
Resolve symlink if it is directly referenced in cli
Kubuxu Jun 23, 2016
f35c53e
sharness: add test for symlink in the middle
Kubuxu Jun 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion commands/cli/parse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,10 @@ func appendFile(fpath string, argDef *cmds.Argument, recursive, hidden bool) (fi
}

fpath = filepath.ToSlash(filepath.Clean(fpath))

fpath, err := filepath.EvalSymlinks(fpath)
Copy link
Member

Choose a reason for hiding this comment

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

I think that this doesnt just resolve top level symlinks.

You could check if the final path element refers to a symlink and resolve it only in that case

Copy link
Member Author

@Kubuxu Kubuxu Jun 23, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Top level as directly referenced by user.

if err != nil {
return nil, err
}
stat, err := os.Lstat(fpath)
if err != nil {
return nil, err
Expand Down
23 changes: 17 additions & 6 deletions test/sharness/t0044-add-symlink.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@ test_description="Test add -w"
test_expect_success "creating files succeeds" '
mkdir -p files/foo &&
mkdir -p files/bar &&
mkdir -p files/badin
echo "some text" > files/foo/baz &&
ln -s files/foo/baz files/bar/baz &&
ln -s files/does/not/exist files/bad
ln -s ../foo/baz files/bar/baz &&
ln -s files/does/not/exist files/badin/bad &&
mkdir -p files2/a/b/c &&
echo "some other text" > files2/a/b/c/foo &&
ln -s b files2/a/d
'

test_add_symlinks() {
Expand All @@ -23,27 +27,34 @@ test_add_symlinks() {
'

test_expect_success "output looks good" '
echo QmWdiHKoeSW8G1u7ATCgpx4yMoUhYaJBQGkyPLkS9goYZ8 > filehash_exp &&
echo QmQRgZT6xVFKJLVVpJDu3WcPkw2iqQ1jqK1F9jmdeq9zAv > filehash_exp &&
test_cmp filehash_exp filehash_out
'

test_expect_success "adding a symlink adds the link itself" '
test_expect_success "adding a symlink adds the file itself" '
ipfs add -q files/bar/baz > goodlink_out
'

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the intent of this test was really to add a symbolic link as there is limited support for them in the unixfs format.

Copy link
Member

Choose a reason for hiding this comment

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

i think so too. we do want to be able to store symlinks eventually

test_expect_success "output looks good" '
echo "QmdocmZeF7qwPT9Z8SiVhMSyKA2KKoA2J7jToW6z6WBmxR" > goodlink_exp &&
echo QmcPNXE5zjkWkM24xQ7Bi3VAm8fRxiaNp88jFsij7kSQF1 > goodlink_exp &&
test_cmp goodlink_exp goodlink_out
'

test_expect_success "adding a broken symlink works" '
ipfs add -q files/bad > badlink_out
ipfs add -qr files/badin | head -1 > badlink_out
Copy link
Member

Choose a reason for hiding this comment

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

does this still work? even with the changes youre making?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because in this case we are adding symlink in recursive tree (not referenced directly by the command) this means that this symlink will not be resolved and added in the old way.

'

test_expect_success "output looks good" '
echo "QmWYN8SEXCgNT2PSjB6BnxAx6NJQtazWoBkTRH9GRfPFFQ" > badlink_exp &&
test_cmp badlink_exp badlink_out
'

test_expect_success "adding with symlink in middle of path is same as\
adding with no symlink" '
ipfs add -rq files2/a/b/c > no_sym &&
ipfs add -rq files2/a/d/c > sym &&
test_cmp no_sym sym
'
}

test_init_ipfs
Expand Down