You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2021/01/20 09:19:34 UTC
[GitHub] [cordova-android] wedgberto opened a new pull request #1154: Move MainActivity.java to folder that tracks the app package name (widget id)
wedgberto opened a new pull request #1154:
URL: https://github.com/apache/cordova-android/pull/1154
<!--
Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
http://cordova.apache.org/contribute/contribute_guidelines.html
Thanks!
-->
### Platforms affected
Android
### Motivation and Context
Cordova has built in hook functionality that enables customisation of the build. My project has used this hook feature to change the package name via the build command line arguments so that the app can be built with the same code but different package names. e.g. io.cordova.helloworld can be installed and ran alongside io.cordova.helloworld2.
This is very useful for installing test, UAT or beta releases side by side. This was possible in cordova-android@8.1.0 but was broken by changes made in 9.0.0.
When the package name of the project is changed in config.xml after a cordova build has completed, and before another build, the MainActivity.Java file is no longer moved to a folder that tracks the package name and ends up being removed.
This leads to an app crash when launched because the source code for a class that extends CordovaActivity no longer exists.
Subsequent build attempts of the project fail because the source code for a class that extends CordovaActivity is missing from the project.
<!-- If it fixes an open issue, please link to the issue here. -->
Fixes #1139
### Description
<!-- Describe your changes in detail -->
This PR reinstates code that took care of moving the Java source code file containing the class that extends CordovaActivity to a folder path that matches the app's package name (e.g. app/src/main/java/**io/cordova/helloworld**).
### Testing
<!-- Please describe in detail how you tested your changes. -->
1. Create a new project and set its package name to io.cordova.helloworld
2. Build the project for Android
3. Edit config.xml and set the package name to io.cordova.helloword2
4. Build the project for Android again (same command as step 2)
5. The app will build but will crash when launched on an Android device because MainActivity is missing
6. Build the project for Android again (do not alter anything, just run the exact same build command as step 5)
7. This time, the build fails with a compiler error (no class found that extends CordovaAcivity)
### Checklist
- [x] I've run the tests to see all new and existing tests pass
- [ ] I added automated test coverage as appropriate for this change
- [ ] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
- [x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
- [ ] I've updated the documentation if necessary
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] wedgberto commented on a change in pull request #1154: (android) Move MainActivity.java to folder that tracks the app package name (widget id)
Posted by GitBox <gi...@apache.org>.
wedgberto commented on a change in pull request #1154:
URL: https://github.com/apache/cordova-android/pull/1154#discussion_r650820208
##########
File path: .gitignore
##########
@@ -54,3 +54,4 @@ coverage/
.project
.settings
.classpath
+.vscode/
Review comment:
I added this to prevent the addition of visual studio code config to the repo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] Davilink commented on pull request #1154: (android) Move MainActivity.java to folder that tracks the app package name (widget id)
Posted by GitBox <gi...@apache.org>.
Davilink commented on pull request #1154:
URL: https://github.com/apache/cordova-android/pull/1154#issuecomment-856309739
I was attempting to do the same thing that you are doing, but discover the same problem that this pr want to fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] wedgberto commented on a change in pull request #1154: (android) Move MainActivity.java to folder that tracks the app package name (widget id)
Posted by GitBox <gi...@apache.org>.
wedgberto commented on a change in pull request #1154:
URL: https://github.com/apache/cordova-android/pull/1154#discussion_r650822249
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -251,24 +250,20 @@ function updateProjectAccordingTo (platformConfig, locations) {
const destFile = java_files[0];
- // var destFile = path.join(locations.root, 'app', 'src', 'main', 'java', androidPkgName.replace(/\./g, '/'), path.basename(java_files[0]));
- // fs.ensureDirSync(path.dirname(destFile));
- // events.emit('verbose', java_files[0]);
- // events.emit('verbose', destFile);
- // console.log(locations);
- // fs.copySync(java_files[0], destFile);
- utils.replaceFileContents(destFile, /package [\w.]*;/, 'package ' + androidPkgName + ';');
- events.emit('verbose', 'Wrote out Android package name "' + androidPkgName + '" to ' + destFile);
-
- var removeOrigPkg = checkReqs.isWindows() || checkReqs.isDarwin()
- ? manifestId.toUpperCase() !== androidPkgName.toUpperCase()
- : manifestId !== androidPkgName;
-
- if (removeOrigPkg) {
Review comment:
rather than using a completely different set of rules to determine whether to delete the **essential** Android Activity class source code file, this change relocates the file when the package name is changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] wedgberto commented on pull request #1154: (android) Move MainActivity.java to folder that tracks the app package name (widget id)
Posted by GitBox <gi...@apache.org>.
wedgberto commented on pull request #1154:
URL: https://github.com/apache/cordova-android/pull/1154#issuecomment-860574348
@erisu @breautek please review this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] wedgberto commented on a change in pull request #1154: (android) Move MainActivity.java to folder that tracks the app package name (widget id)
Posted by GitBox <gi...@apache.org>.
wedgberto commented on a change in pull request #1154:
URL: https://github.com/apache/cordova-android/pull/1154#discussion_r650820208
##########
File path: .gitignore
##########
@@ -54,3 +54,4 @@ coverage/
.project
.settings
.classpath
+.vscode/
Review comment:
ignore visual studio code config
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-io edited a comment on pull request #1154: Move MainActivity.java to folder that tracks the app package name (widget id)
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1154:
URL: https://github.com/apache/cordova-android/pull/1154#issuecomment-763544148
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=h1) Report
> Merging [#1154](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=desc) (4625c33) into [master](https://codecov.io/gh/apache/cordova-android/commit/7428bd3a7fff2b95e5885429acc2a50a95e1dfbc?el=desc) (7428bd3) will **increase** coverage by `1.99%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1154 +/- ##
==========================================
+ Coverage 71.80% 73.80% +1.99%
==========================================
Files 21 21
Lines 1745 1752 +7
==========================================
+ Hits 1253 1293 +40
+ Misses 492 459 -33
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1154/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `59.07% <100.00%> (+11.27%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=footer). Last update [7428bd3...4625c33](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-io commented on pull request #1154: Move MainActivity.java to folder that tracks the app package name (widget id)
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #1154:
URL: https://github.com/apache/cordova-android/pull/1154#issuecomment-763544148
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=h1) Report
> Merging [#1154](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=desc) (4625c33) into [master](https://codecov.io/gh/apache/cordova-android/commit/7428bd3a7fff2b95e5885429acc2a50a95e1dfbc?el=desc) (7428bd3) will **increase** coverage by `1.93%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1154 +/- ##
==========================================
+ Coverage 71.80% 73.74% +1.93%
==========================================
Files 21 21
Lines 1745 1752 +7
==========================================
+ Hits 1253 1292 +39
+ Misses 492 460 -32
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1154/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `59.07% <100.00%> (+11.27%)` | :arrow_up: |
| [bin/templates/cordova/lib/check\_reqs.js](https://codecov.io/gh/apache/cordova-android/pull/1154/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jaGVja19yZXFzLmpz) | `69.75% <0.00%> (-0.49%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=footer). Last update [7428bd3...4625c33](https://codecov.io/gh/apache/cordova-android/pull/1154?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] wedgberto commented on a change in pull request #1154: (android) Move MainActivity.java to folder that tracks the app package name (widget id)
Posted by GitBox <gi...@apache.org>.
wedgberto commented on a change in pull request #1154:
URL: https://github.com/apache/cordova-android/pull/1154#discussion_r650822249
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -251,24 +250,20 @@ function updateProjectAccordingTo (platformConfig, locations) {
const destFile = java_files[0];
- // var destFile = path.join(locations.root, 'app', 'src', 'main', 'java', androidPkgName.replace(/\./g, '/'), path.basename(java_files[0]));
- // fs.ensureDirSync(path.dirname(destFile));
- // events.emit('verbose', java_files[0]);
- // events.emit('verbose', destFile);
- // console.log(locations);
- // fs.copySync(java_files[0], destFile);
- utils.replaceFileContents(destFile, /package [\w.]*;/, 'package ' + androidPkgName + ';');
- events.emit('verbose', 'Wrote out Android package name "' + androidPkgName + '" to ' + destFile);
-
- var removeOrigPkg = checkReqs.isWindows() || checkReqs.isDarwin()
- ? manifestId.toUpperCase() !== androidPkgName.toUpperCase()
- : manifestId !== androidPkgName;
-
- if (removeOrigPkg) {
Review comment:
instead of using a completely different set of rules to determine whether to delete the **essential** Android Activity class source code file, relocate the file when the package name is changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org