cli: do ToSlash after EvalSymlinks to remove platform specific slashes

[Kubuxu]

Resolves #3010

License: MIT
Signed-off-by: Jakub Sztandera kubuxu@protonmail.ch

[whyrusleeping]

typo here truns

[whyrusleeping]

@Kubuxu what is a repro for the original problem? Since this is a windows issue and we don't have windows CI, i will manually verify this fixes the problem on a windows server box

[Kubuxu]

I don't really know.
@liliuhai @djdv can you explain the issues on hands.

[whyrusleeping]

@Kubuxu why does this contain changelog updates?

[Kubuxu]

because I screwed up my local master sorry
fixing it

[djdv]

This is a subset of the problem outlined in issue #1922 and pr #1933

Absolute paths are added but mangled fake directories are also added to the root directory of the ipfs object, if I remember correctly it always returns a directory object as well even when -r and -w are not supplied.
The images in the issue may better describe the problem.

Sans the comment this patch mirrors my own (djdv@31b4cbb) which works on my machine to fix this issue, I believe @liliuhai is saying the same in the previous pr as well (#3010 (comment)).

Adding any Windows file via its absolute path will result in these malformed directory objects. i.e. `ipfs add "C:\somewhere\file.ext" Would result in a structure like this:

/*hash*
    C:\somewhere\file.ext 6B
    file.ext *actual size*

Assuring that paths are / standardized prevents this from happening.

[djdv]

Following up to #3010 (comment) here

@whyrusleeping
I'm not actually familiar with the IPFS tests, if there is a case for adding file(s) via their absolute path then this should have been caught on Windows, but I don't think any of the CI's are testing on Windows anymore, since this is a platform specific problem I don't think it would be caught unless running on a platform that uses path delimiters that are not /. The pr that reintroduced it (#2897) passed 3/5 checks and I think they were all run on *nix containers.

[whyrusleeping]

Tested this branch on my windows box. adding a dir using an absolute path creates the same object as using a relative path now. This LGTM

[jbenet]

This should have test cases-- we will have a regression otherwise.

@djdv what is a good way to setup test cases in windows?

• we had a CI thing. not sure if it still is alive.
• we could also have some os-specific tests to run with sharness.

[djdv]

@jbenet
tl;dr:
Current sharness tests seem fine but CI's can't use them on Windows. We should probably open an issue for getting those tests working properly on Windows and then make CI's use them. go test "./..." works fine on Windows currently though.

Long form:
I think this would have been caught with the current test cases but they would have to have been run on Windows or any OS that uses different standards for file paths. filepath returning platform normalized path strings is what caused this, if the tests are run on a platform that already uses paths with the same standards IPFS expects then there will be no issue if we're using path or filepath on those platforms.

Testing the resulting hash of an add via an absolute path such as C:\tmp\file would have caught this previously, that case would fail where /tmp/file would not, however that case would only show up in a Windows environment so I think it must be tested there. A mock filesystem could possibly be used instead for testing paths but I think that sidesteps testing for other platform issues that could be present.

At one point go-ipfs was building and testing with Appveyor on Windows but I don't think that's the case anymore. Relevant: #1982
The CI's that can, should be building and testing on Windows, that would help prevent regressions like this.

Testing with go via go test "./..." works fine natively but the sharness tests will not work without reworking them a bit to take care of path quirks and glob patterns like "*.go"

Maybe an issue should be raised on this and we can make an effort towards making those scripts more Windows aware.

[whyrusleeping]

@djdv we disabled the appveyor tests because they were always failing and none of the devs had windows boxes to test any of this with. I have an AWS windows server box right now but thats still far from ideal for developing on. If you're interested, we would love some help getting things running nicely on windows (CI and all). We're also looking into a better CI solution over at ipfs/infra#100

[djdv]

@whyrusleeping
Absolutely, I'll come to the IRC channel when I get some time here. See if I can help out in any way.