Resolve symlink if it is directly referenced in cli #2897
Conversation
| @@ -418,7 +418,10 @@ func appendFile(fpath string, argDef *cmds.Argument, recursive, hidden bool) (fi | |||
| } | |||
|
|
|||
| fpath = filepath.ToSlash(filepath.Clean(fpath)) | |||
|
|
|||
| fpath, err := filepath.EvalSymlinks(fpath) | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Top level as directly referenced by user.
Kubuxu
force-pushed
the
feature/root-is-symlink
branch
2 times, most recently
from
00c3271
to
4ce81d6
Compare
Kubuxu
added
need/review
|
I'd like to see a test for a path like: |
| 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 |
There was a problem hiding this comment.
does this still work? even with the changes youre making?
There was a problem hiding this comment.
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.
|
@whyrusleeping I don't understand. |
|
Something like this: mkdir -p a/b/c
ln -s a/b a/d
ipfs add a/d/cJust to put some coverage on resolving a symlink thats in the middle of a path. |
test: Directly referenced symlink should be resolved License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
Kubuxu
force-pushed
the
feature/root-is-symlink
branch
from
c7ab3fb
to
f35c53e
Compare
|
@whyrusleeping added the test and squashed test fixing with changes in the code. |
|
Cool, LGTM |
|
(waiting on tests to complete) |
| 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 | ||
| ' | ||
|
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
i think so too. we do want to be able to store symlinks eventually
This reverts commit fe7b01f. Conflicts: commands/cli/parse.go License: MIT Signed-off-by: Kevin Atkinson <[email protected]>
This reverts commit fe7b01f. Conflicts: commands/cli/parse.go Revert "Merge pull request ipfs#3023 from ipfs/feature/eval-symlink-windows" This reverts commit 16c5a89, reversing changes made to 8c77ff8. Conflicts: commands/cli/parse.go License: MIT Signed-off-by: Kevin Atkinson <[email protected]>
|
This needs to be reverted. See #2839 (comment) |
|
Use a flag instead to get the behavior of resolving the link first. When in doubt, do what git does. |
This reverts commit fe7b01f. Conflicts: commands/cli/parse.go Revert "Merge pull request #3023 from ipfs/feature/eval-symlink-windows" This reverts commit 16c5a89, reversing changes made to 8c77ff8. Conflicts: commands/cli/parse.go License: MIT Signed-off-by: Kevin Atkinson <[email protected]>
Add back still useful test from reverted commit: fe7b01f Resolve symlink if it is directly referenced in cli (#2897) License: MIT Signed-off-by: Kevin Atkinson <[email protected]>
Add back still useful test from reverted commit: fe7b01f Resolve symlink if it is directly referenced in cli (#2897) License: MIT Signed-off-by: Kevin Atkinson <[email protected]>
Resolves #2839
License: MIT
Signed-off-by: Jakub Sztandera [email protected]