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.

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

Follow symlinks in fs temp paths? #202

Open
glenjamin opened this issue May 28, 2020 · 6 comments
Open

Follow symlinks in fs temp paths? #202

glenjamin opened this issue May 28, 2020 · 6 comments

Comments

@glenjamin
Copy link

On OSX, the default temp dir includes a symlink, this means that if you generate a temporary file, and then pass its path into a function which follows links, then assertions you try and make on matching names won't pass.

Would it be reasonable to have fs.NewDir and fs.NewFile expand to real paths before returning?

@dnephin
Copy link
Member

dnephin commented May 29, 2020

That seems reasonable, but I wonder if it might break other uses where symlinks aren't resolved. I need to think about it a bit more. One option might be to add a PathOp something like https://github.com/gotestyourself/gotest.tools/blob/v3.0.2/fs/path.go#L140-L145, which either sets a new field on the directory struct, or maybe resolves the path itself. It's been a little while since I've looked at the fs package, so I'm not exactly sure how that would work, but I expect something like that should be possible.

@dnephin
Copy link
Member

dnephin commented May 29, 2020

When does this become a problem? Is it when calling fs.Equal() ?

I suspect one workaround is to set the TMPDIR env var to the resolved path, so that it doesn't need to be resolved.

I'm thinking a PathOp may not be a good option after all. A new Dir.EvalSymlinks(t *testing.T) method to resolve the links and set Dir.path would probably be a good option. It could be called by any tests where the actual value is known to eval symlinks.

@glenjamin
Copy link
Author

A PathOp seems like a good approach to me.

I ran into this when setting cwd, as apparently under the hood that follows symlinks.

@dnephin
Copy link
Member

dnephin commented May 29, 2020

I ran into this when setting cwd

Do you mean https://golang.org/pkg/os/#Chdir, or os/exec.Cmd.Dir ? If either of those then resolving the links in the test seems like it would work, no?

@glenjamin
Copy link
Author

This was for some code which creates & sets the working directory.

The flow was:

  • test: generate tempdir
  • code: chdir to the tempdir
  • test: assert cwd = tempdir => failed

@dnephin
Copy link
Member

dnephin commented Jun 20, 2020

For that case, since the code being tested explicitly resolves symlinks, it seems like the test should also resolve symlinks in the expected path. I think the new method would only be necessary if the comparison is done with fs.Equal() because there would be no other way to change the expected path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants