Fixed: On windows 'ipfs add -r .' used the full path

[palesz]

Previously 'ipfs add -r .' created an ipfs node with the full
path of the directory as the root node (e.g.
C:\Users\someone\yourdatadirtoshare\ ), from now on, it won't
include the full path of the root directory in the ipfs nodes.

[GitCop]

There were the following issues with your Pull Request

• Commit: 23858ab
  • Invalid signoff. Commit message must end with
    License: MIT
    Signed-off-by: .* <.*>

Guidelines and a script are available to help. Your feedback on GitCop is welcome on this issue.

---

This message was auto-generated by https://gitcop.com

[djdv]

Good catch, that's the one test case that didn't come up before.
Could this be simplified by only changing the one line to:
arg, err := files.NewSerialFile(filepath.Base(fpath), fpath, stat)
or does that cause issues in other cases? That change seems to work for me in the cases I tried but maybe I'm missing something.

[palesz]

filepath.Base(fpath) does not have the same effect. Let's say we have the following dir structure:

D:/public
  /index.html

Using the code in this PR, running ipfs add -r . or ipfs add -r D:/public/:

added Qm__ index.html
added Qm__ .
added Qm__

Using the filepath.Base(fpath) version:

added Qm__ public/index.html
added Qm__ public/
added Qm__

[palesz]

If the latter is the intention, I'm okay changing it to the filepath.Base version.

[djdv]

I believe when using the dot, the directory we're inside should be added the same as with any other command targeting it, if the contents inside the directory are the target I think ipfs add -r * or maybe ipfs add -r ./* should be used. Here's a session from a BSD jail using the latest official release which should show the intended behaviour of all the commands for that tree.
(sorry for the formatting I'm pasting from a phone, out of town)

root@test:/tmp/ipfstest # tree
.
└── topdir
    ├── file1
    ├── file2
    ├── file3
    └── subdir
        ├── subfile1
        ├── subfile2
        └── subfile3

2 directories, 6 files
root@test:/tmp/ipfstest # 

root@test:/tmp/ipfstest/topdir # ipfs-go add -r .
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/file1
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/file2
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/file3
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/subdir/subfile1
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/subdir/subfile2
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/subdir/subfile3
added QmaJu4Xy1FHKf3csaFmwYctBv6qSShzdTnuBhwYQgwVCY4 topdir/subdir
added QmPXQGsymh84jxBaaXqgZRXZLNvfY53SjtxsGExW4aB9nA topdir
root@test:/tmp/ipfstest/topdir # ipfs-go add -r /tmp/ipfstest/topdir/
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/file1
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/file2
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/file3
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/subdir/subfile1
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/subdir/subfile2
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/subdir/subfile3
added QmaJu4Xy1FHKf3csaFmwYctBv6qSShzdTnuBhwYQgwVCY4 topdir/subdir
added QmPXQGsymh84jxBaaXqgZRXZLNvfY53SjtxsGExW4aB9nA topdir
root@test:/tmp/ipfstest/topdir # cd ..
root@test:/tmp/ipfstest # ipfs-go add -r topdir/
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/file1
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/file2
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/file3
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/subdir/subfile1
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/subdir/subfile2
added QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH topdir/subdir/subfile3
added QmaJu4Xy1FHKf3csaFmwYctBv6qSShzdTnuBhwYQgwVCY4 topdir/subdir
added QmPXQGsymh84jxBaaXqgZRXZLNvfY53SjtxsGExW4aB9nA topdir
root@test:/tmp/ipfstest # 

[whyrusleeping]

@palesz could you sign off your commit? (see the instructions posted by gitcop)

[djdv]

@palesz
For clarity I believe the latter is the intended output.
Is that right @whyrusleeping ?

[whyrusleeping]

@djdv yes, the output in the second half of your post is as expected.

[djdv]

Correction:
arg, err := files.NewSerialFile(path.Base(fpath), fpath, stat)
would need to be changed to
arg, err := files.NewSerialFile(filepath.Base(path.Base(fpath)), fpath, stat)
Using just filepath alone works for all cases I tested except UNC path targets which breaks them (resolves all UNC paths to \), using path alone causes the initial issue, stacking both of them doesn't cause any issue in the cases I tested.

For clarity only that line (https://github.com/ipfs/go-ipfs/blob/master/commands/cli/parse.go#L397) was changed from the origin in my local tests, the stat checking doesn't seem to be needed.

Edit:
Stacking the calls like that doesn't seem like the right solution, something else is going on when calling filepath, needs more testing. (Maybe an implicit Clean()? The string should already be cleaned from input before though which would allow just path.Base() to work, not sure.)

[whyrusleeping]

@palesz update here?

[whyrusleeping]

closing due to inactivity, please reopen as needed