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

path in caching backends should be absolute #65

Closed
wch opened this issue Apr 18, 2018 · 3 comments
Closed

path in caching backends should be absolute #65

wch opened this issue Apr 18, 2018 · 3 comments
Labels
bug an unexpected problem or unintended behavior

Comments

@wch
Copy link
Member

wch commented Apr 18, 2018

For example, with cache_filesystem, if path is relative and the working directory changes, then the caching operations will happen in the wrong directory.

@jimhester
Copy link
Member

Agreed, I even suggested this in #51 (comment), but apparently never did anything about it :)

@jimhester jimhester added the bug an unexpected problem or unintended behavior label May 4, 2018
@xhdong-umd
Copy link
Contributor

If I'm understanding correctly, https://github.com/xhdong-umd/memoise/commit/b237c97f8f68e2b3aa44351cbaeb8482bf828db1 should work. I run the tests in package and all passed.

?normalizePath mentioned about this, but I'm not sure if this will cause a problem.

The canonical form of paths may not be what you expect. For example, on macOS absolute paths such as ‘/tmp’ and ‘/var’ are symbolic links.

If you think it's OK I can create a pull request.

@jimhester
Copy link
Member

Just using normalizePath is ok, thanks for offering to do a pull request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

3 participants