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

Check for npmrc in home directory for linux root users #4047

Merged
merged 8 commits into from
Jul 31, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion __tests__/registries/npm-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,17 @@ function createMocks(): Object {
getScopedOption: jest.fn(),
},
};
const mockReporter = jest.fn();
const mockLog = [];
const mockReporter = {
lang: (...args) => mockLog.push(...args),
Copy link
Member

Choose a reason for hiding this comment

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

I think we can also use the BufferReporter for tests?

verbose: jest.fn(),
};

return {
mockRequestManager,
mockRegistries,
mockReporter,
mockLog,
};
}

Expand Down Expand Up @@ -292,3 +297,16 @@ describe('getScope functional test', () => {
});
});
});

describe('getPossibleConfigLocations', () => {
describe('searches recursively to home directory', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

This should be a test or it call not describe.

const testCwd = './project/subdirectory';
const {mockRequestManager, mockRegistries, mockReporter, mockLog} = createMocks();
const npmRegistry = new NpmRegistry(testCwd, mockRegistries, mockRequestManager, mockReporter);
await npmRegistry.getPossibleConfigLocations('npmrc', mockReporter);

expect(mockLog.indexOf('project/subdirectory/.npmrc')).toBeGreaterThan(-1);
expect(mockLog.indexOf('project/.npmrc')).toBeGreaterThan(-1);
expect(mockLog.indexOf(`${homeDir}/.npmrc`)).toBeGreaterThan(-1);
});
});
17 changes: 11 additions & 6 deletions src/registries/npm-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ import envReplace from '../util/env-replace.js';
import Registry from './base-registry.js';
import {addSuffix} from '../util/misc';
import {getPosixPath, resolveWithHome} from '../util/path';

const normalizeUrl = require('normalize-url');
const userHome = require('../util/user-home-dir').default;
const path = require('path');
const url = require('url');
const ini = require('ini');
import normalizeUrl from 'normalize-url';
import {default as userHome, home} from '../util/user-home-dir';
import path from 'path';
Copy link
Member

Choose a reason for hiding this comment

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

Nice! I think the convention is to list stdlib imports at the top, right?

import url from 'url';
import ini from 'ini';

const DEFAULT_REGISTRY = 'https://registry.npmjs.org/';
const REGEX_REGISTRY_PREFIX = /^https?:/;
Expand Down Expand Up @@ -178,6 +177,12 @@ export default class NpmRegistry extends Registry {
[false, path.join(getGlobalPrefix(), 'etc', filename)],
];

// When home directory for global install is different from where $HOME/npmrc is stored,
// E.g. /usr/local/share vs /root on linux machines, check the additional location
if (home !== userHome) {
possibles.push([true, path.join(home, localfile)]);
Copy link
Member

Choose a reason for hiding this comment

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

I think we should change this data structure to be a normal object instead of an array sometime in the future: {isHomeDir: true, path: path.join(home, localfile)}

}

// npmrc --> ../.npmrc, ../../.npmrc, etc.
const foldersFromRootToCwd = getPosixPath(this.cwd).split('/');
while (foldersFromRootToCwd.length > 1) {
Expand Down
5 changes: 3 additions & 2 deletions src/util/user-home-dir.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import ROOT_USER from './root-user.js';

const path = require('path');

const userHomeDir =
process.platform === 'linux' && ROOT_USER ? path.resolve('/usr/local/share') : require('os').homedir();
export const home = require('os').homedir();

const userHomeDir = ROOT_USER ? path.resolve('/usr/local/share') : home;

export default userHomeDir;