Skip to content

Commit

Permalink
Fix: Check for npmrc in home directory for linux root users (#4047)
Browse files Browse the repository at this point in the history
**Summary**
Fix for #3920.

In this [commit](https://github.com/wtgtybhertgeghgtwtg/yarn/commit/f4fe7431ed992862a1c1d2357e6521c268bad7a7), `userHome` for linux users running as root was changed to `/usr/local/share`, mainly to allow for other users to run globally added bins. However this introduced a bug where npmrc wasn't being looked in the root directory after checking `/user/local/share`. So this change pushes another location to check in case `userHome` is different from native home directory returned by `os.homedir()`

**Test plan**
I've added a test to generally test the `getPossibleConfigLocations` method, but I haven't been able to properly mock out running it as a root user on linux. Suggestions welcome!
  • Loading branch information
kaylieEB authored and BYK committed Jul 31, 2017
1 parent cd86ba4 commit 0ef3bf1
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 11 deletions.
25 changes: 22 additions & 3 deletions __tests__/registries/npm-registry.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
/* @flow */

import {resolve, join as pathJoin} from 'path';

import NpmRegistry from '../../src/registries/npm-registry.js';
import {resolve} from 'path';
import {BufferReporter} from '../../src/reporters/index.js';
import homeDir from '../../src/util/user-home-dir.js';

describe('normalizeConfig', () => {
Expand Down Expand Up @@ -50,12 +52,10 @@ function createMocks(): Object {
getScopedOption: jest.fn(),
},
};
const mockReporter = jest.fn();

return {
mockRequestManager,
mockRegistries,
mockReporter,
};
}

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

describe('getPossibleConfigLocations', () => {
test('searches recursively to home directory', async () => {
const testCwd = './project/subdirectory';
const {mockRequestManager, mockRegistries} = createMocks();
const reporter = new BufferReporter({verbose: true});
const npmRegistry = new NpmRegistry(testCwd, mockRegistries, mockRequestManager, reporter);
await npmRegistry.getPossibleConfigLocations('npmrc', reporter);

const logs = reporter.getBuffer().map(logItem => logItem.data);
expect(logs).toEqual(
expect.arrayContaining([
expect.stringContaining(JSON.stringify(pathJoin('project', 'subdirectory', '.npmrc'))),
expect.stringContaining(JSON.stringify(pathJoin('project', '.npmrc'))),
expect.stringContaining(JSON.stringify(pathJoin(homeDir, '.npmrc'))),
]),
);
});
});
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';
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)]);
}

// 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;

0 comments on commit 0ef3bf1

Please sign in to comment.