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

Trailing slash consistency between __dirname and os.tmpdir() #715

Closed
hacksparrow opened this issue Feb 4, 2015 · 11 comments
Closed

Trailing slash consistency between __dirname and os.tmpdir() #715

hacksparrow opened this issue Feb 4, 2015 · 11 comments
Labels
os Issues and PRs related to the os subsystem.

Comments

@hacksparrow
Copy link

console.log(os.tmpdir()) // trailing slash
console.log(__dirname) // no trailing slash

Is there a good reason why os.tmpdir() ends with a trailing slash, while __dirname does not? If there is none, it might make sense to make the return values consistent.

@micnic
Copy link
Contributor

micnic commented Feb 4, 2015

it may be that this is a platform specific issue, for me they are both without trailing slash, what operating system are you using?

@cjihrig
Copy link
Contributor

cjihrig commented Feb 4, 2015

os.tmpdir() comes directly from environment variables. __dirname uses path.dirname(), which strips trailing slashes.

@hacksparrow
Copy link
Author

Mac OS.

@tellnes
Copy link
Contributor

tellnes commented Feb 4, 2015

Maybe we should make os.tmpdir() strip trailing slashes?

@cjihrig
Copy link
Contributor

cjihrig commented Feb 4, 2015

It would be nice to be consistent, but it could potentially break existing code.

@tellnes
Copy link
Contributor

tellnes commented Feb 4, 2015

Yes it will probably break some existing code, but I still see this as a bug.

@cjihrig
Copy link
Contributor

cjihrig commented Feb 4, 2015

I would be OK with this for a major version bump, but I'd like to hear what others think.

@brendanashworth brendanashworth added the os Issues and PRs related to the os subsystem. label Feb 5, 2015
@hacksparrow
Copy link
Author

Right now I am getting through by adding a mandatory '/' everywhere I suspect the slash may or may not be found, and using path.normalize(). But it is an inelegant solution.

Probably iojs should work on normalizing the trailing slash to provide for a consistent interface. It is a pain when something works on Mac and fails on Linux, and vice versa.

@micnic
Copy link
Contributor

micnic commented Feb 6, 2015

I was looking on Github for modules which use os.tmpdir() and most of them use path methods to deal with it, only about 4 modules out of ~50 that I found were using raw string concatenation which can lead to some issues

@tellnes
Copy link
Contributor

tellnes commented Feb 6, 2015

This is a quick fix in io.js and a quick fix in those modules if they break. I'll go ahead and create a pull request and then the only thing left is to choose whether to merge or not.

tellnes added a commit to tellnes/io.js that referenced this issue Mar 21, 2015
This commit makes `os.tmpdir()` behave consistently on all platforms. It
changes `os.tmpdir()` to always return a path without trailing slash.

Semver: major
Fixes: nodejs#715
PR-URL: nodejs#747
Reviewed-By: Ben Noordhuis <[email protected]>
@tellnes
Copy link
Contributor

tellnes commented Mar 22, 2015

Landed in next branch as bb97b70

@tellnes tellnes closed this as completed Mar 22, 2015
tellnes added a commit that referenced this issue Apr 28, 2015
This commit makes `os.tmpdir()` behave consistently on all platforms. It
changes `os.tmpdir()` to always return a path without trailing slash.

Semver: major
Fixes: #715
PR-URL: #747
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
os Issues and PRs related to the os subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants