Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Commit

Permalink
Which returns first result
Browse files Browse the repository at this point in the history
Summary:
This fixes a problem on windows where `where.exe` can return multiple lines if the command is found multiple times on the `PATH`. This is especially painful since the recommended `npm install -g flow-bin` creates two `flow` binaries on by default so the entire flow support is broken, as mentioned in #321.

Also added tests.
Closes #712

Reviewed By: nmote

Differential Revision: D3892797

Pulled By: zsol

fbshipit-source-id: 9de5a425f2f113525677e3dcae7ec3e1ec294bd3
  • Loading branch information
zsol authored and Facebook Github Bot 5 committed Sep 20, 2016
1 parent e6f4a8a commit 5c96ef8
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 1 deletion.
86 changes: 86 additions & 0 deletions pkg/commons-node/spec/which-spec.js
@@ -0,0 +1,86 @@
'use babel';
/* @flow */

/*
* Copyright (c) 2015-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the license found in the LICENSE file in
* the root directory of this source tree.
*/

import which from '../which';

describe('which', () => {
let checkOutput: JasmineSpy;
let checkOutputReturn: {stdout: string} = (null: any);

beforeEach(() => {
checkOutputReturn = {stdout: ''};
checkOutput = spyOn(require('../process'), 'checkOutput').andCallFake(() =>
checkOutputReturn,
);
});

afterEach(() => {
jasmine.unspy(require('../process'), 'checkOutput');
});

describe('on windows', () => {
const real_platform: string = process.platform;
const eol = '\r\n';
const os = require('os');
const real_eol = os.EOL;
beforeEach(() => {
Object.defineProperty(process, 'platform', {value: 'win32'});
os.EOL = eol;
});
afterEach(() => {
Object.defineProperty(process, 'platform', {value: real_platform});
os.EOL = real_eol;
});

it('calls where on Windows', () => {
const param: string = '';
which(param);
expect(checkOutput).toHaveBeenCalledWith('where', [param]);
});

it('returns the first match', () => {
waitsForPromise(async () => {
checkOutputReturn.stdout = 'hello' + os.EOL + 'hello.exe' + os.EOL;
const ret = await which('bla');
expect(ret).toEqual('hello');
});
});
});

describe('on linux', () => {
const real_platform: string = process.platform;
const eol = '\n';
const os = require('os');
const real_eol = os.EOL;
beforeEach(() => {
Object.defineProperty(process, 'platform', {value: 'linux'});
os.EOL = eol;
});
afterEach(() => {
Object.defineProperty(process, 'platform', {value: real_platform});
os.EOL = real_eol;
});

it('calls which', () => {
const param: string = '';
which(param);
expect(checkOutput).toHaveBeenCalledWith('which', [param]);
});

it('returns the first match', () => {
waitsForPromise(async () => {
checkOutputReturn.stdout = 'hello' + os.EOL + '/bin/hello' + os.EOL;
const ret = await which('bla');
expect(ret).toEqual('hello');
});
});
});
});
3 changes: 2 additions & 1 deletion pkg/commons-node/which.js
Expand Up @@ -10,6 +10,7 @@
*/

import {checkOutput} from './process';
import os from 'os';

/**
* Provides a cross-platform way to check whether a binary is available.
Expand All @@ -21,7 +22,7 @@ export default async function which(command: string): Promise<?string> {
const whichCommand = process.platform === 'win32' ? 'where' : 'which';
try {
const result = await checkOutput(whichCommand, [command]);
return result.stdout.trim();
return result.stdout.split(os.EOL)[0];
} catch (e) {
return null;
}
Expand Down

0 comments on commit 5c96ef8

Please sign in to comment.