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.

Sign up for GitHub

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

adds --port option to react-native run-ios as well as patches port … #16172

Closed
wants to merge 2 commits into from
Closed

adds --port option to react-native run-ios as well as patches port … #16172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1dbe4fa
# This is a combination of 6 commits.
Oct 2, 2017
8251757
adds --port option to `react-native run-ios` as well as patches port …
Jan 4, 2018
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
6 changes: 6 additions & 0 deletions Libraries/WebSocket/RCTWebSocket.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,11 @@
isa = XCBuildConfiguration;
buildSettings = {
EXECUTABLE_PREFIX = lib;
GCC_PREPROCESSOR_DEFINITIONS = (
"DEBUG=1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we add DEBUG here?

Copy link
Author

Choose a reason for hiding this comment

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

@shergin When I added the GCC preprocessor section in XCode, it was added automatically, likely because this is the debug preset (see line 440). My guess is that if you omit the preprocessor section XCode auto-assigns the DEBUG define, but I am no XCode/Objective-C wizard and can't confirm this.

"RCT_METRO_PORT=${RCT_METRO_PORT}",
"$(inherited)",
);
GCC_TREAT_WARNINGS_AS_ERRORS = NO;
OTHER_LDFLAGS = "-ObjC";
PRODUCT_NAME = "$(TARGET_NAME)";
Expand All @@ -443,6 +448,7 @@
isa = XCBuildConfiguration;
buildSettings = {
EXECUTABLE_PREFIX = lib;
GCC_PREPROCESSOR_DEFINITIONS = "RCT_METRO_PORT=${RCT_METRO_PORT}";
GCC_TREAT_WARNINGS_AS_ERRORS = NO;
OTHER_LDFLAGS = "-ObjC";
PRODUCT_NAME = "$(TARGET_NAME)";
Expand Down
2 changes: 1 addition & 1 deletion Libraries/WebSocket/RCTWebSocketExecutor.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ - (instancetype)initWithURL:(NSURL *)URL
- (void)setUp
{
if (!_url) {
NSInteger port = [[[_bridge bundleURL] port] integerValue] ?: 8081;
NSInteger port = [[[_bridge bundleURL] port] integerValue] ?: RCT_METRO_PORT;
NSString *host = [[_bridge bundleURL] host] ?: @"localhost";
NSString *URLString = [NSString stringWithFormat:@"http://%@:%lld/debugger-proxy?role=client", host, (long long)port];
_url = [RCTConvert NSURL:URLString];
Expand Down
2 changes: 1 addition & 1 deletion React/Base/RCTBundleURLProvider.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

NSString *const RCTBundleURLProviderUpdatedNotification = @"RCTBundleURLProviderUpdatedNotification";

const NSUInteger kRCTBundleURLProviderDefaultPort = 8081;
const NSUInteger kRCTBundleURLProviderDefaultPort = RCT_METRO_PORT;

static NSString *const kRCTJsLocationKey = @"RCT_jsLocation";
static NSString *const kRCTEnableLiveReloadKey = @"RCT_enableLiveReload";
Expand Down
17 changes: 17 additions & 0 deletions React/Base/RCTDefines.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,23 @@
#define RCT_PROFILE RCT_DEV
#endif

/**
* Add the default Metro packager port number
*/
#ifndef RCT_METRO_PORT
#define RCT_METRO_PORT 8081
#else
// test if RCT_METRO_PORT is empty
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh my, it this a standard way to handle this kind of problem?

Copy link
Author

Choose a reason for hiding this comment

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

@shergin good question :) . This is the only way I could find to cleanly handle the empty string dilemma. I thought at first that we could use a bash expansion trick where you say ${RCT_METRO_PORT:=8081} but xcode did not support the default value there. I tried about 10 other ways that I found on varies stack overflow/exchange posts but this was by far the cleanest. I am open to suggestions, but this works.

#define RCT_METRO_PORT_DO_EXPAND(VAL) VAL ## 1
#define RCT_METRO_PORT_EXPAND(VAL) RCT_METRO_PORT_DO_EXPAND(VAL)
#if !defined(RCT_METRO_PORT) || (RCT_METRO_PORT_EXPAND(RCT_METRO_PORT) == 1)
// Only here if RCT_METRO_PORT is not defined
// OR RCT_METRO_PORT is the empty string
#undef RCT_METRO_PORT
#define RCT_METRO_PORT 8081
#endif
#endif

/**
* By default, only raise an NSAssertion in debug mode
* (custom assert functions will still be called).
Expand Down
14 changes: 12 additions & 2 deletions React/React.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3957,7 +3957,7 @@
);
runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
shellScript = "if [ -z \"${RCT_NO_LAUNCH_PACKAGER+xxx}\" ] ; then\n if nc -w 5 -z localhost 8081 ; then\n if ! curl -s \"http://localhost:8081/status\" | grep -q \"packager-status:running\" ; then\n echo \"Port 8081 already in use, packager is either not running or not running correctly\"\n exit 2\n fi\n else\n open \"$SRCROOT/../scripts/launchPackager.command\" || echo \"Can't start packager automatically\"\n fi\nfi";
shellScript = "export RCT_METRO_PORT=\"${RCT_METRO_PORT:=8081}\"\necho \"export RCT_METRO_PORT=${RCT_METRO_PORT}\" > \"${SRCROOT}/../scripts/.packager.env\"\nif [ -z \"${RCT_NO_LAUNCH_PACKAGER+xxx}\" ] ; then\n if nc -w 5 -z localhost ${RCT_METRO_PORT} ; then\n if ! curl -s \"http://localhost:${RCT_METRO_PORT}/status\" | grep -q \"packager-status:running\" ; then\n echo \"Port ${RCT_METRO_PORT} already in use, packager is either not running or not running correctly\"\n exit 2\n fi\n else\n open \"$SRCROOT/../scripts/launchPackager.command\" || echo \"Can't start packager automatically\"\n fi\nfi";
Copy link
Contributor

Choose a reason for hiding this comment

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

Maaaaaagic...

Copy link
Author

Choose a reason for hiding this comment

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

Not sure what this comment means. Would you like me to a comment to the code to explain what is going on here?

showEnvVarsInLog = 0;
};
142C4F7F1B582EA6001F0B58 /* Include RCTJSCProfiler */ = {
Expand Down Expand Up @@ -5100,6 +5100,13 @@
buildSettings = {
CLANG_CXX_LANGUAGE_STANDARD = "c++14";
CLANG_STATIC_ANALYZER_MODE = deep;
GCC_PREPROCESSOR_DEFINITIONS = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Author

Choose a reason for hiding this comment

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

The RCT_METRO_PORT is passed in as an environment variable to keep the code as clean as possible. I only added the RCT_METRO_PORT, when adding the GCC preprocessor definitions xcode pre-populated the list with the other defines.

"DEBUG=1",
"RCT_DEBUG=1",
"RCT_DEV=1",
"RCT_NSASSERT=1",
"RCT_METRO_PORT=${RCT_METRO_PORT}",
);
GCC_WARN_ABOUT_MISSING_NEWLINE = YES;
OTHER_LDFLAGS = "-ObjC";
PRODUCT_NAME = "$(TARGET_NAME)";
Expand All @@ -5114,7 +5121,10 @@
buildSettings = {
CLANG_CXX_LANGUAGE_STANDARD = "c++14";
CLANG_STATIC_ANALYZER_MODE = deep;
GCC_PREPROCESSOR_DEFINITIONS = "$(inherited)";
GCC_PREPROCESSOR_DEFINITIONS = (
"$(inherited)",
"RCT_METRO_PORT=${RCT_METRO_PORT}",
);
GCC_WARN_ABOUT_MISSING_NEWLINE = YES;
OTHER_LDFLAGS = "-ObjC";
PRODUCT_NAME = "$(TARGET_NAME)";
Expand Down
14 changes: 10 additions & 4 deletions local-cli/runAndroid/runAndroid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ function runAndroid(argv, config, args) {
return buildAndRun(args);
}

return isPackagerRunning().then(result => {
return isPackagerRunning(args.port).then(result => {
if (result === 'running') {
console.log(chalk.bold('JS server already running.'));
} else if (result === 'unrecognized') {
console.warn(chalk.yellow('JS server not recognized, continuing with build...'));
} else {
// result == 'not_running'
console.log(chalk.bold('Starting JS server...'));
startServerInNewWindow();
startServerInNewWindow(args.port);
}
return buildAndRun(args);
});
Expand Down Expand Up @@ -262,7 +262,7 @@ function runOnAllDevices(args, cmd, packageNameWithSuffix, packageName, adbPath)
}
}

function startServerInNewWindow() {
function startServerInNewWindow(port) {
const scriptFile = /^win/.test(process.platform) ?
'launchPackager.bat' :
'launchPackager.command';
Expand All @@ -271,6 +271,12 @@ function startServerInNewWindow() {
const procConfig = {cwd: scriptsDir};
const terminal = process.env.REACT_TERMINAL;

// setup the .packager.env file to ensure the packager starts on the right port
const packagerEnvFile = path.join(__dirname, '..', '..', 'scripts', '.packager.env');
const content = `export RCT_METRO_PORT=${port}`;
// ensure we overwrite file by passing the 'w' flag
fs.writeFileSync(packagerEnvFile, content, {encoding: 'utf8', flag: 'w'});

if (process.platform === 'darwin') {
if (terminal) {
return child_process.spawnSync('open', ['-a', terminal, launchPackagerScript], procConfig);
Expand Down Expand Up @@ -333,7 +339,7 @@ module.exports = {
description: 'Do not launch packager while building',
}, {
command: '--port [number]',
default: 8081,
default: process.env.RCT_METRO_PORT || 8081,
parse: (val: string) => Number(val),
}],
};
24 changes: 15 additions & 9 deletions local-cli/runIOS/runIOS.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ function runIOS(argv, config, args) {
function runOnDeviceByUdid(args, scheme, xcodeProject, devices) {
const selectedDevice = matchingDeviceByUdid(devices, args.udid);
if (selectedDevice) {
return runOnDevice(selectedDevice, scheme, xcodeProject, args.configuration, args.packager, args.verbose);
return runOnDevice(selectedDevice, scheme, xcodeProject, args.configuration, args.packager, args.verbose, args.port);
} else {
if (devices && devices.length > 0) {
console.log('Could not find device with the udid: "' + args.udid + '".');
Expand Down Expand Up @@ -115,7 +115,7 @@ function runOnSimulator(xcodeProject, args, scheme) {
}
resolve(selectedSimulator.udid);
})
.then((udid) => buildProject(xcodeProject, udid, scheme, args.configuration, args.packager, args.verbose))
.then((udid) => buildProject(xcodeProject, udid, scheme, args.configuration, args.packager, args.verbose, args.port))
.then((appName) => {
if (!appName) {
appName = scheme;
Expand All @@ -135,8 +135,8 @@ function runOnSimulator(xcodeProject, args, scheme) {
});
}

function runOnDevice(selectedDevice, scheme, xcodeProject, configuration, launchPackager, verbose) {
return buildProject(xcodeProject, selectedDevice.udid, scheme, configuration, launchPackager, verbose)
function runOnDevice(selectedDevice, scheme, xcodeProject, configuration, launchPackager, verbose, port) {
return buildProject(xcodeProject, selectedDevice.udid, scheme, configuration, launchPackager, verbose, port)
.then((appName) => {
if (!appName) {
appName = scheme;
Expand All @@ -159,7 +159,7 @@ function runOnDevice(selectedDevice, scheme, xcodeProject, configuration, launch
});
}

function buildProject(xcodeProject, udid, scheme, configuration = 'Debug', launchPackager = false, verbose) {
function buildProject(xcodeProject, udid, scheme, configuration = 'Debug', launchPackager = false, verbose, port) {
return new Promise((resolve,reject) =>
{
var xcodebuildArgs = [
Expand All @@ -174,7 +174,7 @@ function buildProject(xcodeProject, udid, scheme, configuration = 'Debug', launc
if (!verbose) {
xcpretty = xcprettyAvailable() && child_process.spawn('xcpretty', [], { stdio: ['pipe', process.stdout, process.stderr] });
}
const buildProcess = child_process.spawn('xcodebuild', xcodebuildArgs, getProcessOptions(launchPackager));
const buildProcess = child_process.spawn('xcodebuild', xcodebuildArgs, getProcessOptions(launchPackager, port));
let buildOutput = '';
buildProcess.stdout.on('data', function(data) {
buildOutput += data.toString();
Expand Down Expand Up @@ -232,13 +232,15 @@ function printFoundDevices(devices) {
}
}

function getProcessOptions(launchPackager) {
function getProcessOptions(launchPackager, port) {
if (launchPackager) {
return {};
return {
env: { ...process.env, RCT_METRO_PORT: port }
};
}

return {
env: Object.assign({}, process.env, { RCT_NO_LAUNCH_PACKAGER: true }),
env: { ...process.env, RCT_NO_LAUNCH_PACKAGER: true },
};
}

Expand Down Expand Up @@ -287,5 +289,9 @@ module.exports = {
}, {
command: '--verbose',
description: 'Do not use xcpretty even if installed',
},{
command: '--port [number]',
default: process.env.RCT_METRO_PORT || 8081,
parse: (val: string) => Number(val),
}],
};
2 changes: 1 addition & 1 deletion local-cli/server/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ module.exports = {
description: 'starts the webserver',
options: [{
command: '--port [number]',
default: 8081,
default: process.env.RCT_METRO_PORT || 8081,
parse: (val: string) => Number(val),
}, {
command: '--host [string]',
Expand Down
4 changes: 2 additions & 2 deletions local-cli/util/isPackagerRunning.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ const fetch = require('node-fetch');
* - `unrecognized`: one other process is running on the port we expect the
* packager to be running.
*/
function isPackagerRunning() {
return fetch('http://localhost:8081/status').then(
function isPackagerRunning(packagerPort = (process.env.RCT_METRO_PORT || 8081)) {
return fetch(`http://localhost:${packagerPort}/status`).then(
res => res.text().then(body =>
body === 'packager-status:running' ? 'running' : 'unrecognized'
),
Expand Down
1 change: 1 addition & 0 deletions scripts/packager.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
# of patent rights can be found in the PATENTS file in the same directory.

THIS_DIR=$(dirname "$0")
source "${THIS_DIR}/.packager.env"
cd "$THIS_DIR/.."
node "./local-cli/cli.js" start "$@"