process: make process.config read-only

[jasnell]

Checklist

• tests and code linting passes
• a test and/or benchmark is included
• documentation is changed or added
• the commit message follows commit guidelines

Affected core subsystem(s)

process

Description of change

Makes process.config read-only using a getter/setter and freezing the objects on JSON.parse. Does result in a perf hit but process.config is not used in any critical paths (or any path in core) for that matter. There are apparently some places in user land where this is being extended and it causes problems when we want to add new configuration options (see #6115 for an example).

Refs: #6115

/cc @Fishrock123

[claudiorodriguez]

CI: https://ci.nodejs.org/job/node-test-pull-request/2226/
LGTM if CI is happy
Do we have some knowledge of which userland modules extend it currently?

[Fishrock123]

we probably need to do a bit of extended research on this one

[jasnell]

Yep, this one is going to be just a bit problematic. There are a handful of modules that mutate process.config... for instance: https://github.com/falsecz/cson-config/blob/master/index.coffee

Salem (https://www.npmjs.com/package/salem) is a good example. Take a look at the example given:

var salem = require('salem');
var configClient = salem.createClient({
  url: 'http://localhost:8080',
  env: process.env.NODE_ENV || 'development'
});

// 
// Load the configuration for use within the application. 
// 
configClient.loadConfig(function (err, config) {
  if (err) throw err;
  process.config = config;
});

Some of these add keys to the process.config, others replace it outright... which is obviously problematic if we're going to be depending on it in any way.

Digging in further... It also looks like this could break node-gyp which does some stuff with process.config internally.

[Fishrock123]

@jasnell what if we just keep our own internal copy?

[jasnell]

Yep, that would work.

[jasnell]

Essentially, we'd be conceding the process.config to userland which we would need to document. Given the way userland modules override it, users may not be able to depend on it's original purpose.

[claudiorodriguez]

Yep, CI broken all over

gyp: Undefined variable standalone_static_library in binding.gyp while trying to load binding.gyp

[jasnell]

Given the results I'm going to close this and look at tackling the problem a different way.