Fix error in react-native run-ios when Product Name and Scheme are inequal #10178

StevePotter · 2016-09-29T18:13:40Z

In xcode, you can modify the Product Name per build configuration. During the build, the app file name is made using that value. For example, I name my app "MyApp Dev", it will build into "MyApp Dev.app".

react-native run-ios doesn't extract the proper app name. Instead it uses the scheme name + ".app". So if in the example above I use the default Scheme "MyApp", which references the build configuration whose name is "MyApp Dev". The build will succeed, but when runIOS.js goes to run the app, it fails because the file name doesn't exist.

My fix parses the build output and extracts the app name from the line "export FULL_PRODUCT_NAME=$(appfilename)" and uses that instead of the scheme. If there is any issue parsing, the scheme name is used like it currently is.

Test plan (required)

Change the Product Name in xcode project manager to be something different than the scheme name.
Run react-native run-ios and ensure it works.

…l if Product Name and Scheme aren't equal

facebook-github-bot · 2016-09-29T18:13:52Z

By analyzing the blame information on this pull request, we identified @LearningDave and @grabbou to be potential reviewers.

facebook-github-bot · 2016-09-29T18:13:53Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

facebook-github-bot · 2016-09-29T18:37:31Z

@StevePotter updated the pull request - view changes

facebook-github-bot · 2016-09-29T18:45:57Z

@StevePotter updated the pull request - view changes

…wnSync stdio param had to change from inherit to pipe, so user was no longer seeing build output

facebook-github-bot · 2016-09-29T18:47:12Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

facebook-github-bot · 2016-09-29T18:47:17Z

@StevePotter updated the pull request - view changes

StevePotter · 2016-09-30T02:17:39Z

Not sure why the Travis CI failed. Can an expert look at this? I don't see how my fix would affect that script.

facebook-github-bot · 2016-10-03T18:06:27Z

@StevePotter updated the pull request - view changes

javache · 2016-10-05T10:49:00Z

local-cli/runIOS/runIOS.js

  }

-  buildProject(xcodeProject, selectedSimulator.udid, scheme);
+  var appName = buildProject(xcodeProject, selectedSimulator.udid, scheme);


Use let for this

javache · 2016-10-05T10:49:07Z

local-cli/runIOS/runIOS.js


 function runOnDevice(selectedDevice, scheme, xcodeProject){
-  buildProject(xcodeProject, selectedDevice.udid, scheme);
+  var appName = buildProject(xcodeProject, selectedDevice.udid, scheme);


Use let here too

javache · 2016-10-05T10:50:17Z

local-cli/runIOS/runIOS.js

-  child_process.spawnSync('xcodebuild', xcodebuildArgs, {stdio: 'inherit'});
+  var buildProcess = child_process.spawnSync('xcodebuild', xcodebuildArgs, {stdio: 'pipe'});
+  var buildOutput = buildProcess.stdout.toString();
+  console.log(buildOutput);


Doing console.log like this means we will buffer all output and it will seem like the process is stuck while xcode is building. Can you stream it to the console while also scanning it?

I had the same concern. I tried different combinations for stdout and got nothing that would pipe output and allow me to buffer it. I totally get the problem it creates but node basically gives you the option to buffer or pipe: https://nodejs.org/api/child_process.html#child_process_child_process_spawnsync_command_args_options

Another option could be to do try using child_process.spawn, as done here.

I'll see what I can do. But IMO it's worth it now, considering it is fixing a bug that I consider to be major. Even adding a console.log("Building, please wait this can take a while." before could be good enough for now.

I think the spawn approach is more correct here. We're seeing issues with console output being hidden already on travis (e.g. your build hits a timeout and gets killed, this console.log will never get executed).

Given that in the default configuration for RN, the app name is identical to the scheme, I think we should try to get it right instead of a hacking a fix in.

Done via promises.

javache · 2016-10-05T10:50:55Z

local-cli/runIOS/runIOS.js

+  var buildOutput = buildProcess.stdout.toString();
+  console.log(buildOutput);
+  //FULL_PRODUCT_NAME is the actual file name of the app, which actually comes from the Product Name in the build config, which does not necessary match a scheme name,  example output line: export FULL_PRODUCT_NAME="Super App Dev.app"
+  var exportFullProductLines = buildOutput.match(new RegExp("export FULL_PRODUCT_NAME=.+\.app"));


Use a regex capture group to match once here and extract the value you're looking for later: http://www.regular-expressions.info/refcapture.html

…atching for the app name

facebook-github-bot · 2016-10-10T22:04:25Z

@StevePotter updated the pull request - view changes

StevePotter · 2016-10-10T22:06:15Z

local-cli/runIOS/runIOS.js

+  let productNameMatch = /(export FULL_PRODUCT_NAME=)("*)(.+)(.app)/.exec(buildOutput);
+  if (productNameMatch && productNameMatch.length && productNameMatch.length > 3)
+  {
+    return productNameMatch[3];//0 is the full match, 1 is 'export FULL_PRODUCT_NAME=', 2 is possible double quote, 3 is app name and 4 is '.app'


@javache, this better?

You don't need to use () for everything, just put it around the part of the input string you care about.

const productNameMatch = /export FULL_PRODUCT_NAME="?(.+)\.app/.exec(buildOutput);

…se child_process.spawn instead of spawnsync, which fixes the problem of build output not being shown in the console.

facebook-github-bot · 2016-10-11T14:25:24Z

@StevePotter updated the pull request - view changes

facebook-github-bot · 2016-10-11T14:30:53Z

@StevePotter updated the pull request - view changes

StevePotter · 2016-10-11T14:35:53Z

@javache, I employed promises to allow for the use of child_process.spawn function, which then you can listen to events on asynchronously. This allowed me to buffer the build output to isolate the app name while still streaming build output to the console. All your other requests have been fulfilled.

I also added the fix from PR 10155, which allows you just use --device without a name if you have a single device connected. This makes testing easier, as entering a full device name is tedious and typically in my experience there is just one device attached at a time.

Please review. Thanks!

…nto fixiosappname

facebook-github-bot · 2016-10-11T14:39:47Z

@StevePotter updated the pull request - view changes

StevePotter · 2016-10-11T14:40:55Z

local-cli/runIOS/runIOS.js


+
 function matchingDevice(devices, deviceName) {
+  if (deviceName === true && devices.length === 1)


This is the part that will honor an empty device name ("deviceName === true") with only one device connected, and use that name. Much easier for testing on a single device with zero side effects.

javache

Looks good! Thanks for making these changes. Just a couple of style nits.

javache · 2016-10-11T14:58:19Z

local-cli/runIOS/runIOS.js

-  console.log(`Installing ${appPath}`);
-  child_process.spawnSync('xcrun', ['simctl', 'install', 'booted', appPath], {stdio: 'inherit'});
+  return new Promise((resolve) =>
+  {


nit: have opening brace on previous line

javache · 2016-10-11T14:58:32Z

local-cli/runIOS/runIOS.js

+  return new Promise((resolve) =>
+  {
+    try {
+      var simulators = JSON.parse(


const simulators

left as var because simulators is used outside the context of the try/catch it's in. Turn it into a const and you get "simulators is not defined" error. You could pull it out of the try/catch, but then you have to use let. So I thought var was actually the best choice here. If you want, I'll add "let simulators;" above the try. What do you prefer?

Ah yes, let simulators outside of the try-catch block would be the preferred style.

javache · 2016-10-11T14:59:21Z

local-cli/runIOS/runIOS.js

+  })
+  .then((udid) => buildProject(xcodeProject, udid, scheme))
+  .then((appName) =>
+  {


nit: open brace on previous line

javache · 2016-10-11T14:59:41Z

local-cli/runIOS/runIOS.js

+  .then((udid) => buildProject(xcodeProject, udid, scheme))
+  .then((appName) =>
+  {
+    if (!appName)


nit: Add braces after the if

javache · 2016-10-11T15:00:13Z

local-cli/runIOS/runIOS.js

+  .then((appName) =>
+  {
+    if (!appName)
+      appName = scheme;


Same nits as above

javache · 2016-10-11T15:00:41Z

local-cli/runIOS/runIOS.js

+      '--justlaunch'
+    ];
+    console.log(`installing and launching your app on ${selectedDevice.name}...`);
+    let iosDeployOutput = child_process.spawnSync('ios-deploy', iosDeployInstallArgs, {encoding: 'utf8'});


This should be const

javache · 2016-10-11T15:04:15Z

local-cli/runIOS/runIOS.js

+      buildOutput += data.toString();
+    });
+    buildProcess.stderr.on('data', function(data) {
+        console.log(data.toString());


This should be console.error? Also, indent with 2 spaces.

javache · 2016-10-11T15:04:50Z

local-cli/runIOS/runIOS.js

+    });
+    buildProcess.on('close', function(code) {
+      //FULL_PRODUCT_NAME is the actual file name of the app, which actually comes from the Product Name in the build config, which does not necessary match a scheme name,  example output line: export FULL_PRODUCT_NAME="Super App Dev.app"
+      let productNameMatch = /export FULL_PRODUCT_NAME="*(.+).app/.exec(buildOutput);


Since there's just going to be 1 or 0 " in your regex, use "? instead of "*"

javache · 2016-10-11T15:05:44Z

local-cli/runIOS/runIOS.js

+      //FULL_PRODUCT_NAME is the actual file name of the app, which actually comes from the Product Name in the build config, which does not necessary match a scheme name,  example output line: export FULL_PRODUCT_NAME="Super App Dev.app"
+      let productNameMatch = /export FULL_PRODUCT_NAME="*(.+).app/.exec(buildOutput);
+      if (productNameMatch && productNameMatch.length && productNameMatch.length > 1)
+      {


nit: on previous line

javache · 2016-10-11T15:06:49Z

Could you also check out the approach taken in https://github.com/facebook/react-native/pull/10156/files. It seems to be a lot simpler.

StevePotter · 2016-10-12T17:59:40Z

@javache I fixed all issues you brought up, except one I commented on. I also commented on #10156, which is a great approach but unfortunately doesn't work yet.

facebook-github-bot · 2016-10-12T17:59:44Z

@StevePotter updated the pull request - view changes

javache · 2016-10-14T17:12:11Z

@facebook-github-bot shipit

facebook-github-bot · 2016-10-14T17:13:27Z

Thanks for importing. If you are a Facebook employee, you can view this diff on Phabricator.

…equal Summary: In xcode, you can modify the Product Name per build configuration. During the build, the app file name is made using that value. For example, I name my app "MyApp Dev", it will build into "MyApp Dev.app". react-native run-ios doesn't extract the proper app name. Instead it uses the scheme name + ".app". So if in the example above I use the default Scheme "MyApp", which references the build configuration whose name is "MyApp Dev". The build will succeed, but when runIOS.js goes to run the app, it fails because the file name doesn't exist. My fix parses the build output and extracts the app name from the line "export FULL_PRODUCT_NAME=$(appfilename)" and uses that instead of the scheme. If there is any issue parsing, the scheme name is used like it currently is. **Test plan (required)** 1) Change the Product Name in xcode project manager to be something different than the scheme name. 2) Run react-native run-ios and ensure it works. Closes facebook/react-native#10178 Differential Revision: D4022116 Pulled By: javache fbshipit-source-id: c1bd8e7a1f6364d681c505557a96955a293bc05c

Uses correct ios app file name so that react-native run-ios won't fai…

d4e9495

…l if Product Name and Scheme aren't equal

facebook-github-bot added the GH Review: review-needed label Sep 29, 2016

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 29, 2016

Improved regex and parsing slightly

028535a

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 29, 2016

Fixed double spawnsync

d9e2d2d

Wrote build output to the console. Needed this becaeuse called to spa…

93d3aab

…wnSync stdio param had to change from inherit to pipe, so user was no longer seeing build output

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 29, 2016

StevePotter mentioned this pull request Sep 29, 2016

Latest react-native app doesn't work ":CFBundleIdentifier", Does Not Exist #7308

Closed

Merge branch 'master' into fixiosappname

4db0df2

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 3, 2016

javache requested changes Oct 5, 2016

View reviewed changes

mikelambert mentioned this pull request Oct 5, 2016

Fixes run-ios when the PRODUCT_NAME differs from the scheme. #10156

Closed

Switched a vew var keywords to let. Tightened up build output regex m…

c48a6df

…atching for the app name

StevePotter commented Oct 10, 2016

View reviewed changes

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 10, 2016

Promisified runOnX and buildProject functions so buildProject could u…

ee2b353

…se child_process.spawn instead of spawnsync, which fixes the problem of build output not being shown in the console.

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 11, 2016

Fixed build output regex to use a single capture group.

e37439d

Merge branch 'master' into fixiosappname

9501c1d

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 11, 2016

Merge branch 'fixiosappname' of github.com:StevePotter/react-native i…

e357742

…nto fixiosappname

StevePotter commented Oct 11, 2016

View reviewed changes

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 11, 2016

javache approved these changes Oct 11, 2016

View reviewed changes

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 11, 2016

Few style tweaks

c01f837

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 12, 2016

facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Oct 14, 2016

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 14, 2016

StevePotter mentioned this pull request Oct 14, 2016

Update iOS run script to fix the build launch in the simulator #10393

Closed

facebook-github-bot closed this in 8b55e50 Oct 14, 2016

StevePotter mentioned this pull request Oct 17, 2016

Fixed the run-android cli script to start the correct activity when using different applicationId for different variants #8950

Closed



		function matchingDevice(devices, deviceName) {
		if (deviceName === true && devices.length === 1)

Fix error in react-native run-ios when Product Name and Scheme are inequal #10178

Fix error in react-native run-ios when Product Name and Scheme are inequal #10178

Uh oh!

Conversation

StevePotter commented Sep 29, 2016

Uh oh!

facebook-github-bot commented Sep 29, 2016

Uh oh!

facebook-github-bot commented Sep 29, 2016

Uh oh!

facebook-github-bot commented Sep 29, 2016

Uh oh!

facebook-github-bot commented Sep 29, 2016

Uh oh!

facebook-github-bot commented Sep 29, 2016

Uh oh!

facebook-github-bot commented Sep 29, 2016

Uh oh!

StevePotter commented Sep 30, 2016

Uh oh!

facebook-github-bot commented Oct 3, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Oct 10, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Oct 11, 2016

Uh oh!

facebook-github-bot commented Oct 11, 2016

Uh oh!

StevePotter commented Oct 11, 2016

Uh oh!

facebook-github-bot commented Oct 11, 2016

Uh oh!

StevePotter Oct 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javache left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javache Oct 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

StevePotter Oct 11, 2016 •

edited

Loading

javache Oct 11, 2016 •

edited

Loading

StevePotter commented Oct 12, 2016 •

edited

Loading