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 XDG OS conventions for storing data #1570

Merged
merged 2 commits into from Oct 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -170,7 +170,7 @@ Some additional resources for addons and writing `gyp` files:
| `--thin=yes` | Enable thin static libraries
| `--arch=$arch` | Set target architecture (e.g. ia32)
| `--tarball=$path` | Get headers from a local tarball
| `--devdir=$path` | SDK download directory (default is `~/.node-gyp`)
| `--devdir=$path` | SDK download directory (default is OS cache directory)
| `--ensure` | Don't reinstall headers if already present
| `--dist-url=$url` | Download header tarball from custom URL
| `--proxy=$url` | Set HTTP proxy for downloading header tarball
Expand Down
8 changes: 4 additions & 4 deletions bin/node-gyp.js
Expand Up @@ -2,10 +2,10 @@

process.title = 'node-gyp'

var envPaths = require('env-paths')
var gyp = require('../')
var log = require('npmlog')
var osenv = require('osenv')
var path = require('path')
var os = require('os')

/**
* Process and execute the selected commands.
Expand All @@ -16,11 +16,11 @@ var completed = false
prog.parseArgv(process.argv)
prog.devDir = prog.opts.devdir

var homeDir = osenv.home()
var homeDir = os.homedir()
if (prog.devDir) {
prog.devDir = prog.devDir.replace(/^~/, homeDir)
} else if (homeDir) {
prog.devDir = path.resolve(homeDir, '.node-gyp')
prog.devDir = envPaths('node-gyp', { suffix: '' }).cache
Copy link
Contributor Author

@Siilwyn Siilwyn Oct 13, 2018

Choose a reason for hiding this comment

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

The suffix is 'nodejs' by default, another option is: envPaths('gyp') which will result in gyp-nodejs.
I opted for .cache because I think it fits the type of data more than .data since it is data being cached to prevent unneeded requests.

} else {
throw new Error(
"node-gyp requires that the user's home directory is specified " +
Expand Down
4 changes: 2 additions & 2 deletions lib/configure.js
Expand Up @@ -8,7 +8,7 @@ module.exports.test = {
var fs = require('graceful-fs')
, path = require('path')
, log = require('npmlog')
, osenv = require('osenv')
, os = require('os')
, which = require('which')
, semver = require('semver')
, mkdirp = require('mkdirp')
Expand Down Expand Up @@ -46,7 +46,7 @@ function configure (gyp, argv, callback) {

if (gyp.opts.nodedir) {
// --nodedir was specified. use that for the dev files
nodeDir = gyp.opts.nodedir.replace(/^~/, osenv.home())
nodeDir = gyp.opts.nodedir.replace(/^~/, os.homedir())

log.verbose('get node dir', 'compiling against specified --nodedir dev files: %s', nodeDir)
createBuildDir()
Expand Down
6 changes: 3 additions & 3 deletions lib/install.js
Expand Up @@ -11,7 +11,7 @@ module.exports.test = {
exports.usage = 'Install node development files for the specified node version.'

var fs = require('graceful-fs')
, osenv = require('osenv')
, os = require('os')
, tar = require('tar')
, path = require('path')
, crypto = require('crypto')
Expand Down Expand Up @@ -400,9 +400,9 @@ function install (fs, gyp, argv, callback) {
function eaccesFallback (err) {
var noretry = '--node_gyp_internal_noretry'
if (-1 !== argv.indexOf(noretry)) return cb(err)
var tmpdir = osenv.tmpdir()
var tmpdir = os.tmpdir()
gyp.devDir = path.resolve(tmpdir, '.node-gyp')
log.warn('EACCES', 'user "%s" does not have permission to access the dev dir "%s"', osenv.user(), devDir)
log.warn('EACCES', 'user "%s" does not have permission to access the dev dir "%s"', os.userInfo().username, devDir)
log.warn('EACCES', 'attempting to reinstall using temporary dev dir "%s"', gyp.devDir)
if (process.cwd() == tmpdir) {
log.verbose('tmpdir == cwd', 'automatically will remove dev files after to save disk space')
Expand Down
4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -22,20 +22,20 @@
"bin": "./bin/node-gyp.js",
"main": "./lib/node-gyp.js",
"dependencies": {
"env-paths": "^1.0.0",
"glob": "^7.0.3",
"graceful-fs": "^4.1.2",
"mkdirp": "^0.5.0",
"nopt": "2 || 3",
"npmlog": "0 || 1 || 2 || 3 || 4",
"osenv": "0",
"request": "^2.87.0",
"rimraf": "2",
"semver": "~5.3.0",
"tar": "^3.1.3",
"which": "1"
},
"engines": {
"node": ">= 4.0.0"
"node": ">= 6.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is tricky. We need more buy-in from other contributors.
@nodejs/node-gyp

Copy link
Member

Choose a reason for hiding this comment

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

So far there have been no objections to dropping support for Node.js <6 for node-gyp v4 (#1519). I also suggested dropping support for Node.js 6 which will be EOL in April.

},
"devDependencies": {
"babel-eslint": "^8.2.5",
Expand Down