apache / cordova-ios

Apache Cordova iOS
https://cordova.apache.org/
Apache License 2.0
2.16k stars 989 forks source link

(ios) issue-912: fix deployment to device #936

Closed imgos closed 4 years ago

imgos commented 4 years ago

cordova-ios used to use "mv -f" and it was replaced with fs-extra.moveSync. moveSync behaves differently than mv, which broke deployment to devices. This fixes the folder move and subsequently deployment to device (when using "cordova run ios --device ..."

Platforms affected

Motivation and Context

This was motivated by wanting to run an ios app on the device. This worked in versions of cordova-ios <= 5.1.1 and was broken in 6.0.0 and 6.1.0.

fixes: https://github.com/apache/cordova-ios/issues/912

Description

Fix a file copy. The behavior of moveSync is not exactly the same as mv -f which was previously used.

Testing

I have tested by deploying ios apps with cordova run ios --device from the command line. This restores the functionality from the old versions of cordova-ios.

Checklist

erisu commented 4 years ago

The changes are correct and the files are copied over to the correct location.

The app deploys to the phone but I noticed that only a black screen displays, no default app shown. If I open the project with Xcode and run on device, the application will launch normally and display default app content.

I used an iPhone Xs w/ iOS 13.5.1 for this test.

@imgos Is this the same behaviour you see?

imgos commented 4 years ago

@erisu - I've been able to deploy two cordova apps to my ios devices (iphone 7 plus and iphone 8) successfully with this. I have not had an app build and then open with a black screen.

I see there is an xcode project in the tests folder, but I'm not sure how to deploy it without opening in xcode. If you can point me to the instructions I'd be happy to try it and report the results.

timbru31 commented 4 years ago

I'll give this PR a test today - codewise it looks good. Thanks for the investigation :)

StratusBase commented 4 years ago

When is a release going to happen with this change? This is pretty crucial...

timbru31 commented 4 years ago

We do not give any ETAs of new releases.

motla commented 4 years ago

It has been a month, is it complicated for you to release a patch?

mignam commented 4 years ago

Hello !

Same here, hope it will be patch soon :)

Thanks for the investigation !

codecat commented 4 years ago

Thanks for the pull request, I was facing this issue as well today.

As a temporary workaround until this gets released to stable, you can either downgrade to 5.1.1 (latest version prior to 6.0.0) or use the latest nightly version. (Which you can find here under "versions".)

Here's how to change the version of your iOS platform:

$ cordova platform remove ios
$ cordova platform add ios@5.1.1

To install the nightly version, simply replace 5.1.1 with nightly.

breautek commented 4 years ago

To install the nightly version, simply replace 5.1.1 with nightly.

Nightly builds should be reserved for testing only. But for this case, a safer workaround is to use a specific version, such as 6.2.0-nightly.2020.8.11.f1a67376

This will at least provide consistency over nightly which will change for every commit merged into master and may suddenly turn unstable (by accident of course).

StratusBase commented 4 years ago

Downgrading to 5.1.1 is not an option as I will not start a brand new project using the old UIWebview... Apple won’t allow apps with any reference to it. The 6x version removes that reference and uses the WKWebView but has a bug that prevents the package from even getting deployed to your iOS device. A recent commit has resolved this but hasn’t made it into a release so I’m hard-coding the change for the time being... Anyone who wants to use this package is basically stuck doing this since no one is going to want to downgrade if they’re taking iOS deployment seriously... Apple won’t allow it.

codecat commented 4 years ago

Do you have a link to the Apple docs where it talks about this? I'll have to deploy soon and I have some projects still on 5.1.1.

imgos commented 4 years ago

Can we open a separate issue to address problems in the release process?

What are the issues in the release process that make it difficult to release a patch? Once these are identified, maybe there are ways some of us can help automate the process.

StratusBase commented 4 years ago

Do you have a link to the Apple docs where it talks about this? I'll have to deploy soon and I have some projects still on 5.1.1.

https://developer.apple.com/news/?id=12232019b

https://developer.apple.com/documentation/uikit/uiwebview https://developer.apple.com/documentation/webkit/wkwebview

StratusBase commented 4 years ago

Can we open a separate issue to address problems in the release process?

What are the issues in the release process that make it difficult to release a patch? Once these are identified, maybe there are ways some of us can help automate the process.

Exactly, at a minimum this should just be a patch, not a minor version release or requiring me to tag nightly or any of that...

timbru31 commented 4 years ago

Sorry to interrupt here, but some comments are simply wrong. It is safe to use cordova-ios@5 with the WKWebView plugin. v6 simply integrated those changes into the core platform.

@imgos please refer to https://www.apache.org/legal/release-policy.html#release-approval - we can’t alter the process much. And yes a patch is more work than a minor because we need to cherry pick to the correct branches etc. besides that, all development (no support!) discussions should take place via the dev mailing list.

breautek commented 4 years ago

Do you have a link to the Apple docs where it talks about this? I'll have to deploy soon and I have some projects still on 5.1.1.

5.1.1 uses UIWebView by default. Additional steps are necessary to make it compliant with Apple, which are documented here.