Fix issue #486. Distinguish xcodebuild `build` command and `archive` command. #487

knight9999 · 2018-12-24T05:39:07Z

distinguish xcodebuild build and xcodebuild archive according to cordova build and cordova run

Platforms affected

cordova-ios

What does this PR do?

Resolve this the #486

removing shelljs in cordova/lib/run.js. This is because shelljs.rm can not remove symbolic link.
using xcodebuild build at cordova run --device. Because ios-deploy can not work normally with .app pacakge obtained byxcodebuild archive and xcodebuild -exportArchive command.
new option --nobuild-device is added. This is force skip build process evenwhen cordova run --device.

In this PR, build.js is called from run.js with the build-action build. This is because we should use internal xcodebuild build command to get the .app package instead of xcodebuild archive command with Xcode 10.

Before this PR,
cordova run --device runs as follows.

cordova-lib calls build.js
cordova-lib calls run.js with --nobuild option. => But ios-deploy does not work normally.

After this PR,
cordova run --device runs as follows.

cordova-lib calls build.js (This build process is unnecessary. Because the ios-deploy does not work normally with the obtained .app package )
cordova-lib calls run.js with --nobuild option. => run.js calls build.js with option buildFlag: CONFIGURATION_BUILD_DIR=[projectPath]/build/device-run and buildAction: build.

Although this PR works with the latest cordova-lib, there is one unnecessary build process.
To remove unnecessary build process, we should update cordova-lib. I will send new PR for cordova-lib.

What testing has been done on this change?

I checked as follows with both Xcode9 and Xcode10 envrionment.

$ npx cordova@nightly create xcode10Test com.foo.bar
$ cd xcode10Test
$ vi build.json
(set the certificate and provisioning file, development for debug build and ad-hoc for release build)
$ npx cordova@nightly platform add github:knight9999/cordova-ios#fix_cordova_run
$ npx cordova@nightly run --emulator --debug 
(In some environment, you should specify the target like
`--target="iPhone-8, 11.3"`.  The available targets are shown by `npx cordova@nightly run --list` This is because the selecting default simulator logic is different between build.js and run.js. This is another issue we should solve. but is out of this PR.) 
(confirm the app on emulator)
$ npx cordova@nightly run --device --debug
(confirm the app on device)
$ npx cordova@nightly run --device --release
(confirm the app on device)
$ npx cordova@nightly run --device --release --nobuild --nobuild-device
(This skip build processes. confirm the app on device)
$ npx cordova@nightly build --device
(install .ipa file and confirm the app on device)
$ npx cordova@nightly build --release
(install .ipa file and confirm the app on device)

Checklist

Reported an issue in the JIRA database
Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
Added automated test coverage as appropriate for this change.

brody4hire

Travis CI failure on Node.js version 6, apparently since it does not support an object spread

I would also favor a title that better explains the change in behavior, for example: use --nobuild-device option to fix device build

I would favor taking the issue number out of the title and putting "Resolves #486" or "Closes #486" in either the description or the commit message.

…ive according to cordova run and cordova build respectively

codecov-io · 2018-12-25T02:20:12Z

Codecov Report

Merging #487 into master will increase coverage by 0.19%.
The diff coverage is 9.09%.

@@            Coverage Diff             @@
##           master     #487      +/-   ##
==========================================
+ Coverage   75.66%   75.86%   +0.19%     
==========================================
  Files          11       11              
  Lines        1796     1790       -6     
==========================================
- Hits         1359     1358       -1     
+ Misses        437      432       -5

Impacted Files	Coverage Δ
bin/templates/scripts/cordova/lib/run.js	`23.76% <0%> (+0.39%)`	⬆️
bin/templates/scripts/cordova/lib/build.js	`57.71% <50%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe01e5d...59c0de0. Read the comment docs.

knight9999 · 2018-12-25T02:32:24Z

Thanks for the valuable comment @brodybits .
I have updated my PR as you mentioned.

knight9999 · 2019-01-07T05:48:35Z

@brodybits I have already updated my PR and now my PR passes all CI Checks.
Would you please re-review my PR?

shazron · 2019-01-09T08:14:44Z

I'm having trouble at the deploy stage since the .app is not found (you removed all the code that unzips the .ipa)

cordova build --developmentTeam=XXXX --device
cordova build --developmentTeam=XXXX --nobuild-device

dpogue · 2019-01-09T09:14:48Z

I am still having trouble with Xcode 10 not making valid builds, which means the generated .ipa file is wrong. I have a suspicion that the invalid .ipa is why unzipping it and running the .app doesn't work.

I'd slightly prefer to try to fix that underlying Xcode 10 build issue before we merge this, because I feel like this PR is fixing a symptom of the problem without fixing the root of the problem.

knight9999 · 2019-01-10T01:55:25Z

@shazron Thanks for the comment.
You mean cordova run or cordova build?
I removed all the code that unzips the .ipa in lib/run.js. This is because unzipped .app file does not work in ios-deploy. (As you pointed out in ios-control/ios-deploy#364, it stalls after showing splashscreen.)
Instead of unzipping .ipa file, I use xcodebuild build command to get .app files in lib/run.js. (where calling lib/build.js internally)
The resulting .app files are in platforms/ios/build/device-run instead of platforms/ios/build/device.

knight9999 · 2019-01-10T02:10:31Z

@dpogue Thanks for the comment.

I am still having trouble with Xcode 10 not making valid builds, which means the generated .ipa file is wrong. I have a suspicion that the invalid .ipa is why unzipping it and running the .app doesn't work.

I feel this is a root of this problem, just as you said.

This may be the Xcode 10's bug. Or Apple chagnes the build specification for some reason. (security reason?)

This PR is dirty in this meaning because of inconsistent way of cordova build and cordova run.
But for now, this is only the solution I found.

knight9999 · 2019-01-10T02:25:19Z

The lldb patch in ios-control/ios-deploy#364 (comment) may be the solution.
If the patch resolves this issue, I will remove this PR. Thanks.

shazron · 2019-01-10T03:17:36Z

The ios-deploy issue is a red-herring. Turns out the .app wasn't built with the --buildFlag='-UseModernBuildSystem=0' flag. Once I did that, it could deploy fine (the Modern Build System generates a .app that doesn't have an embedded.mobileprovision)

knight9999 · 2019-01-11T01:11:47Z

@shazron Thanks you for the great work #494 .
This resolves the issue about ios-deploy (in 'cordova run' command) when using explicit provisioning.
Although there is an issue with automatic provisioning case, I close this PR.

brody4hire mentioned this pull request Dec 24, 2018

Cordova run does not work #486

Closed

brody4hire suggested changes Dec 24, 2018

View reviewed changes

Resolves apache#486. distinguish xcodebuild build and xcodebuild arch…

59c0de0

…ive according to cordova run and cordova build respectively

knight9999 force-pushed the fix_cordova_run branch from 300dff3 to 59c0de0 Compare December 25, 2018 02:10

knight9999 closed this Jan 11, 2019

ArcTanSusan mentioned this pull request Sep 19, 2019

(#659) Remove CONFIGURATION_BUILD_DIR default string #671

Closed

5 tasks

snyk-bot mentioned this pull request Mar 15, 2023

[Snyk] Upgrade semver from 7.3.5 to 7.3.8 davidmather/cordova-ios#2

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue #486. Distinguish xcodebuild `build` command and `archive` command. #487

Fix issue #486. Distinguish xcodebuild `build` command and `archive` command. #487

knight9999 commented Dec 24, 2018 •

edited

Loading

brody4hire left a comment

codecov-io commented Dec 25, 2018

knight9999 commented Dec 25, 2018

knight9999 commented Jan 7, 2019

shazron commented Jan 9, 2019

dpogue commented Jan 9, 2019

knight9999 commented Jan 10, 2019 •

edited

Loading

knight9999 commented Jan 10, 2019 •

edited

Loading

knight9999 commented Jan 10, 2019 •

edited

Loading

shazron commented Jan 10, 2019 •

edited

Loading

knight9999 commented Jan 11, 2019

Fix issue #486. Distinguish xcodebuild build command and archive command. #487

Fix issue #486. Distinguish xcodebuild build command and archive command. #487

Conversation

knight9999 commented Dec 24, 2018 • edited Loading

Platforms affected

What does this PR do?

What testing has been done on this change?

Checklist

brody4hire left a comment

Choose a reason for hiding this comment

codecov-io commented Dec 25, 2018

Codecov Report

knight9999 commented Dec 25, 2018

knight9999 commented Jan 7, 2019

shazron commented Jan 9, 2019

dpogue commented Jan 9, 2019

knight9999 commented Jan 10, 2019 • edited Loading

knight9999 commented Jan 10, 2019 • edited Loading

knight9999 commented Jan 10, 2019 • edited Loading

shazron commented Jan 10, 2019 • edited Loading

knight9999 commented Jan 11, 2019

Fix issue #486. Distinguish xcodebuild `build` command and `archive` command. #487

Fix issue #486. Distinguish xcodebuild `build` command and `archive` command. #487

knight9999 commented Dec 24, 2018 •

edited

Loading

knight9999 commented Jan 10, 2019 •

edited

Loading

knight9999 commented Jan 10, 2019 •

edited

Loading

knight9999 commented Jan 10, 2019 •

edited

Loading

shazron commented Jan 10, 2019 •

edited

Loading