Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

Use os.tmpdir() for temp-dirs #4

Closed

Conversation

robertkowalski
Copy link
Contributor

The former implementation preferred /home/{username}/tmp on unix
which caused errors for users that had different permissions on
their tmp-folder in their home, like root as owner from a previous
sudo-command.

The new implementation uses a plain os.tmpdir() from node.core.

os.tmpdir in node core seems to be essentially the same as this function, with the small difference that not every process.env-variable are used on windows

I first thought about checking for the existence of the tmp-dir and then fallback to the /home/{username}/tmp folder but this makes no sense as the system must have the sytsem-/tmp dirs.

This fixes: isaacs/npm#3470

@robertkowalski
Copy link
Contributor Author

ping

maybe we should close the issue here and in npm if the change is not wanted which is okay for me.

@just-boris
Copy link

I have same issue, vote for merge.
+1

@LinusU
Copy link

LinusU commented Feb 7, 2014

@isaacs Could we please get this merged, this is causing trouble for me when using forever. This bug affects most big node.js applications, e.g. npm, forever, bower...

@allaire
Copy link

allaire commented Mar 2, 2014

👍

The former implementation preferred /home/{username}/tmp on unix
which caused errors for users that had different permissions on
their tmp-folder in their home, like root as owner from a previous
sudo-command.

The new implementation uses os.tmpdir()

This fixes: isaacs/npm#3470
@robertkowalski
Copy link
Contributor Author

merged as d6eddbc and released as 0.1.0

@robertkowalski robertkowalski deleted the tmp-dir--isaacs/npm#3470-v2 branch May 26, 2014 18:00
@allaire
Copy link

allaire commented May 26, 2014

@robertkowalski yay, less code, less problems 😄

How can we get this fix? Update npm?

Thank you!

@robertkowalski
Copy link
Contributor Author

@allaire will keep you posted in npm/npm#3470 - should be included in one of the next versions of npm.

@allaire
Copy link

allaire commented May 26, 2014

@robertkowalski Good thanks!!

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

Successfully merging this pull request may close these issues.

4 participants