os: add homedir() #1791

cjihrig · 2015-05-25T15:21:08Z

Do not land this yet.

This PR adds os.homedir(), which calls libuv's uv_os_homedir() to retrieve the current user's home directory. This is an alternative to #1670. This PR currently consists of two commits. 6887116 is the libuv update, which has not landed in io.js yet. This commit should be removed once a proper update is in place. e7e5992 is the io.js update.

bnoordhuis · 2015-05-25T16:06:57Z

src/node_os.cc

+  char buf[MAX_PATH];
+#else
+  char buf[PATH_MAX];
+#endif


node.h defines PATH_MAX on Windows so the #ifdef is probably not necessary here.

bnoordhuis · 2015-05-25T18:01:25Z

test/parallel/test-os.js

+} else if (!common.isWindows && process.env.HOME) {
+  assert.equal(home, process.env.HOME);
+  delete process.env.HOME;
+  assert.equal(home, os.homedir());


I'm not sure if this is a good assumption. I'd just check that it still returns something, possibly something that looks like a path.

You're right. Updated.

bnoordhuis · 2015-05-25T21:22:17Z

LGTM. You should probably run the CI before landing this.

cjihrig · 2015-05-25T21:23:32Z

I will once libuv is updated and I can remove the floating patch.

rvagg · 2015-05-25T22:58:00Z

lgtm, the duplication in test-os.js seems unnecessary but I guess it's consistent with what's in there

Fishrock123 · 2015-06-04T02:11:56Z

Libuv 1.6.0 has landed. 👍

thefourtheye · 2015-06-04T02:33:45Z

deps/uv/src/unix/core.c

+    return -EIO;
+
+  uid = getuid();
+  buf = NULL;


At this point, buf would be NULL only.

cjihrig · 2015-06-04T03:13:36Z

Floating patch removed. CI: https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/766/

cjihrig · 2015-06-04T03:34:22Z

@rvagg the CI is still running, but I'm seeing some strange failures. Unable to compile on SmartOS, out of memory errors on FreeBSD, and too many files open on CentOS. Anything odd going on with the build?

rvagg · 2015-06-04T03:57:14Z

@cjihrig eeek, not that I'm aware of, @jbergstroem has been tinkering on CentOS5 but not the others.

jbergstroem · 2015-06-04T04:12:22Z

I'll have a look.

jbergstroem · 2015-06-04T04:33:22Z

Affected os:es seems to hit file-max

freebsd: kern.maxfiles limit exceeded by uid 1003, please see tuning(7).
centos5: VFS: file-max limit 334442 reached

Jenkins has <100 file descriptors open which leads me to believe that ccache might try to. I can't say I've seen it previously, but I'll keep digging.

jbergstroem · 2015-06-04T04:39:47Z

Looking at cat /proc/sys/fs/file-nr (or equivalents) at different machines; can't say I see it grow even close to limits while compiling.

jbergstroem · 2015-06-04T04:42:48Z

(for reference, the run I'm measuring is here: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/767/)

jbergstroem · 2015-06-04T06:19:21Z

So yeah, I'm unable to reproduce what happened at that specific run. Jenkins.

cjihrig · 2015-06-04T14:49:54Z

Added limits.h in https://github.com/cjihrig/io.js/commit/69c1c33348ac1803615c73e42bf7dfef3155a4a7#diff-2b7b259d26f297c87b7d71512e33ccecR14 to get the compile working on SmartOS. The CentOS failure seemed to be flakey. FreeBSD is still throwing ENOMEM. @bnoordhuis any clue why this might be happening (other than the obvious answer that the process is out of memory)?

Updated CI run: https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/769/

bnoordhuis · 2015-06-04T16:39:12Z

I speculate it's because of this. :-)

(IIRC, I pointed that out in the review, that sysconf() can return < 0, but I'm not 100% sure. At any rate, bufsize is a size_t so the -1 wraps around and becomes SIZE_MAX.)

(EDIT: That may have been another PR, come to think of it.)

cjihrig · 2015-06-04T17:18:34Z

@bnoordhuis you did mention that. The data type for bufsize not being switched from size_t to long was my bad (I did add the check for bufsize <= 0, but didn't change the data type). Assuming that this is the problem, I'm curious why this didn't cause a problem with the libuv CI. And, do you think we should ditch the sysconf() call and use something else as the starting point for malloc()?

EDIT: this has a good example that calls sysconf() and falls back to a different value if it fails, although they also use a size_t to hold the return value :-)

cjihrig · 2015-06-04T20:23:38Z

@bnoordhuis you were right, so I added a37072b as a floating patch on libuv, and the CI is much happier now. Let me know how you want this handled in libuv.

bnoordhuis · 2015-06-04T21:55:50Z

@cjihrig Left a comment. Can you file that as a libuv PR? We'll do a patch release for that.

jbergstroem · 2015-06-04T22:24:05Z

Ah, should've tested the branch while searching for issues; I just assumed it was lingering processes or similar.

cjihrig · 2015-06-05T13:53:24Z

@bnoordhuis if I update the floating patch here, is this OK to land?

EDIT: Removed the floating patch since the libuv update PR was opened.

Refs: nodejs#1791

saghul · 2015-06-05T17:49:20Z

@cjihrig FYI: #1905

PR-URL: #1905 Refs: #1791 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>

cjihrig · 2015-06-05T20:35:20Z

Thanks @saghul. Hopefully one last CI run: https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/774/

Fishrock123 · 2015-06-05T20:51:26Z

Hmmm.. freebsd isn't so happy. https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/774/nodes=freebsd101-64/tapTestReport/

os.homedir() calls libuv's uv_os_homedir() to retrieve the current user's home directory. PR-URL: #1791 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rod Vagg <[email protected]>

cjihrig · 2015-06-06T04:45:20Z

It would help if I rebased the branch to include the libuv 1.6.1 update.

https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/775/ is all green.

Landed in 6e78e5f.

@rlidwka

Notable Changes: * libuv: Upgraded to 1.6.0 and 1.6.1, see full ChangeLog for details. (Saúl Ibarra Corretgé) #1905 #1889. Highlights include: - Fix TTY becoming blocked on OS X - Fix UDP send callbacks to not to be synchronous - Add uv_os_homedir() (exposed as os.homedir(), see below) * npm: See full release notes for details. (Kat Marchán) #1899. Highlight: - Use GIT_SSH_COMMAND (available as of Git 2.3) * openssl: - Upgrade to 1.0.2b and 1.0.2c, introduces DHE man-in-the-middle protection (Logjam) and fixes malformed ECParameters causing infinite loop (CVE-2015-1788). See the security advisory for full details. (Shigeki Ohtsu) #1950 #1958 - Support FIPS mode of OpenSSL, see README for instructions. (Fedor Indutny) #1890 * os: Add os.homedir() method. (Colin Ihrig) #1791 * smalloc: Deprecate whole module. (Vladimir Kurchatkin) #1822 * Add new collaborators: - Alex Kocharin (@rlidwka) - Christopher Monsanto (@monsanto) - Ali Ijaz Sheikh (@ofrobots) - Oleg Elifantiev (@Olegas) - Domenic Denicola (@domenic) - Rich Trott (@Trott)

sindresorhus · 2015-06-13T12:30:56Z

I made a polyfill for anyone wanting to use this while still supporting older node/io versions:
https://github.com/sindresorhus/os-homedir

silverwind added os Issues and PRs related to the os subsystem. semver-minor PRs that contain new features and should be released in the next minor version. labels May 25, 2015

bnoordhuis reviewed May 25, 2015
View reviewed changes

Fishrock123 mentioned this pull request May 25, 2015

os: add .homedir() #1670

Closed

bnoordhuis reviewed May 25, 2015
View reviewed changes

Fishrock123 added the libuv Issues and PRs related to the libuv dependency or the uv binding. label May 26, 2015

bnoordhuis force-pushed the master branch from 311c3e4 to b926718 Compare June 2, 2015 17:25

thefourtheye reviewed Jun 4, 2015
View reviewed changes

deps/uv/src/unix/core.c

return -EIO;

uid = getuid();

buf = NULL;

Copy link

Contributor

thefourtheye Jun 4, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, buf would be NULL only.

cjihrig mentioned this pull request Jun 4, 2015

unix: handle invalid _SC_GETPW_R_SIZE_MAX values libuv/libuv#389

Closed

saghul added a commit to saghul/node that referenced this pull request Jun 5, 2015

deps: update libuv to version 1.6.1

d3b69c5

Refs: nodejs#1791

saghul mentioned this pull request Jun 5, 2015

deps: update libuv to version 1.6.1 #1905

Closed

bnoordhuis pushed a commit that referenced this pull request Jun 5, 2015

deps: update libuv to version 1.6.1

a5bd466

PR-URL: #1905 Refs: #1791 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>

os: add homedir()

6e78e5f

os.homedir() calls libuv's uv_os_homedir() to retrieve the current user's home directory. PR-URL: #1791 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rod Vagg <[email protected]>

cjihrig merged commit 6e78e5f into nodejs:master Jun 6, 2015

cjihrig deleted the homedir branch June 6, 2015 04:45

rvagg mentioned this pull request Jun 11, 2015

Release proposal: 2.3.0 #1939

Closed

nfischer mentioned this pull request Jan 19, 2016

feat: add tilde expansion to expand() shelljs/shelljs#293

Merged

renkun-ken mentioned this pull request Jan 10, 2020

Improve session watcher initialization REditorSupport/vscode-R#184

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

os: add homedir() #1791

os: add homedir() #1791

cjihrig commented May 25, 2015

bnoordhuis May 25, 2015

bnoordhuis May 25, 2015

cjihrig May 25, 2015

bnoordhuis commented May 25, 2015

cjihrig commented May 25, 2015

rvagg commented May 25, 2015

Fishrock123 commented Jun 4, 2015

thefourtheye Jun 4, 2015

cjihrig commented Jun 4, 2015

cjihrig commented Jun 4, 2015

rvagg commented Jun 4, 2015

jbergstroem commented Jun 4, 2015

jbergstroem commented Jun 4, 2015

jbergstroem commented Jun 4, 2015

jbergstroem commented Jun 4, 2015

jbergstroem commented Jun 4, 2015

cjihrig commented Jun 4, 2015

bnoordhuis commented Jun 4, 2015

cjihrig commented Jun 4, 2015

cjihrig commented Jun 4, 2015

bnoordhuis commented Jun 4, 2015

jbergstroem commented Jun 4, 2015

cjihrig commented Jun 5, 2015

saghul commented Jun 5, 2015

cjihrig commented Jun 5, 2015

Fishrock123 commented Jun 5, 2015

cjihrig commented Jun 6, 2015

sindresorhus commented Jun 13, 2015

os: add homedir() #1791

os: add homedir() #1791

Conversation

cjihrig commented May 25, 2015

bnoordhuis May 25, 2015

Choose a reason for hiding this comment

bnoordhuis May 25, 2015

Choose a reason for hiding this comment

cjihrig May 25, 2015

Choose a reason for hiding this comment

bnoordhuis commented May 25, 2015

cjihrig commented May 25, 2015

rvagg commented May 25, 2015

Fishrock123 commented Jun 4, 2015

thefourtheye Jun 4, 2015

Choose a reason for hiding this comment

cjihrig commented Jun 4, 2015

cjihrig commented Jun 4, 2015

rvagg commented Jun 4, 2015

jbergstroem commented Jun 4, 2015

jbergstroem commented Jun 4, 2015

jbergstroem commented Jun 4, 2015

jbergstroem commented Jun 4, 2015

jbergstroem commented Jun 4, 2015

cjihrig commented Jun 4, 2015

bnoordhuis commented Jun 4, 2015

cjihrig commented Jun 4, 2015

cjihrig commented Jun 4, 2015

bnoordhuis commented Jun 4, 2015

jbergstroem commented Jun 4, 2015

cjihrig commented Jun 5, 2015

saghul commented Jun 5, 2015

cjihrig commented Jun 5, 2015

Fishrock123 commented Jun 5, 2015

cjihrig commented Jun 6, 2015

sindresorhus commented Jun 13, 2015