Add symlinks support

[darshildpatel]

Please fill in the fields below to submit a pull request. The more
information that is provided, the better.

Fixes issue #: #32

Description of pull request: Handles the directory symlinks. The support for recursive symlinks is WIP.

Please verify and check that the pull request fulfills the following
requirements:

• Tests have been added for the bug fix or new feature
• Docs have been added for the bug fix or new feature

[JustinCappos]

Here be dragons! We need to really review this carefully with all of the race conditions related to symlinks / dirs in mind.

[lukpueh]

Thanks for working on this, @darshildpatel! Can you please remove unrelated files? doc.html and in_toto/.runlib_test.go.swp need not be part of your PR. Also go.sum and go.mod seem to be outdated. I reckon, they are leftovers from your playing around with fastwalk? Take a look at the go modules documentation for more infos (hint: go mod tidy should do the trick).

Also, and this is mostly for future reference, please make sure to follow our development guidelines, e.g. more descriptive commit messages would be very helpful.

Same goes for the code documentation. Given that there are several important caveats, such as race conditions and infinite recursions, it would be good to mention them in the function docstring and/or inline comments.

The implementation itself looks good. :) I will give you a more thorough review on the code once you have fixed above, i.e. remove obsolete files and add code documentation.

[shibumi]

@lukpueh I think It makes sense to touch this PR here as well, when I have finished the exclude and error handling one. Following symlinks is very useful for the actual run implementation, that we are planning.

[shibumi]

I had a closer look on this PR and I think we can use filepath's EvalSymlink() method instead of checking for symlinks via the os module:

https://golang.org/pkg/path/filepath/#EvalSymlinks

This PR also diverges with the current master a lot, I think a complete rewrite would be necessary :S

[shibumi]

@darshildpatel can you explain why you didn't use filepath.EvalSymLink() here?
I am not sure if you used os.ReadLink() on purpose for validating all "steps" in a symlink or if you just don't know about EvalSymLink().

[shibumi]

@lukpueh I am not sure about the behavior of this PR. Would be nice if you could tell me what we actually want here...

Let's imagine we have the following symlink chain:

/tmp/test3 -> /tmp/test2 -> /tmp/test1

Then, filepath.EvalSymlinks("/tmp/test3") will output: /tmp/test1
If we use os.Readlink("/tmp/test3") we will have output: test2.

Right now I think filepath.EvalSymlinks is the better solution here, because we wouldn't rely on a recursion for solving this issue + it's less error-prone. What do you think?

[lukpueh]

Right now I think filepath.EvalSymlinks is the better solution here, because we wouldn't rely on a recursion for solving this issue + it's less error-prone. What do you think?

Apologies for not replying earlier. I think either approach are fine as long as it is documented. It looks like the recursion argument no longer holds true, because you do use a recursive approach in #55 anyways, right?

Regardless, we still need to decide if link metadata stores the path of the source or the target of a symlink. Did you have a chance to look at the python reference implementation and see how it's done there?

[shibumi]

I think we can close this PR. Thanks for your effort @darshildpatel, your PR was a key to our new symlink support ❤️

[shibumi]

Regardless, we still need to decide if link metadata stores the path of the source or the target of a symlink. Did you have a chance to look at the python reference implementation and see how it's done there?

uff.. just saw this now.. I will open a new issue for this, so that we keep track of it.

[lukpueh]

uff.. just saw this now.. I will open a new issue for this, so that we keep track of it.

Thanks! I forgot to cross-check too. :)

[lukpueh]

Thanks for the solid spadework, @darshildpatel!