You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by matrosov-nikita <gi...@git.apache.org> on 2017/07/24 09:19:50 UTC
[GitHub] cordova-android pull request #397: CB-13031: Fix bug with case-sensitivity o...
GitHub user matrosov-nikita opened a pull request:
https://github.com/apache/cordova-android/pull/397
CB-13031: Fix bug with case-sensitivity of android-packageName
<!--
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
self
### What does this PR do?
Steps to reproduce:
1) cordova create testapp com.example.testapp testapp
2) cd testapp
3) edit config.xml, add android-packageName="com.example.Testapp" to <widget>
4) cordova platform add android
Since `windows` and `osx` file systems are case-insensitive, [this line](https://github.com/apache/cordova-android/blob/893356abcd1b005a13bb09e7467f227b52de3ae5/bin/templates/cordova/lib/prepare.js#L213) doesn't create a directory with a name from `android-packageName` attribute if it differs in casing from original package name.
This PR makes case-sensitive check before removal of an old package.
### What testing has been done on this change?
### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
- [x] 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.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/matrosov-nikita/cordova-android CB-13031
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-android/pull/397.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #397
----
commit 61add83c4145a234df0c9cfd5bdbfdd4dd945609
Author: Nikita Matrosov <v-...@microsoft.com>
Date: 2017-07-19T13:36:20Z
CB-13031: Fix bug with case-sensitivity of android-packageName
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android issue #397: CB-13031: Fix bug with case-sensitivity of andro...
Posted by codecov-io <gi...@git.apache.org>.
Github user codecov-io commented on the issue:
https://github.com/apache/cordova-android/pull/397
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/397?src=pr&el=h1) Report
> Merging [#397](https://codecov.io/gh/apache/cordova-android/pull/397?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/458e47968158642e803a36b2aaed15b5a789a994?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/cordova-android/pull/397?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #397 +/- ##
=======================================
Coverage 42.55% 42.55%
=======================================
Files 17 17
Lines 1678 1678
Branches 304 304
=======================================
Hits 714 714
Misses 964 964
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/397?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/397?src=pr&el=tree#diff-YmluL2xpYi9jcmVhdGUuanM=) | `50.82% <100%> (ø)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/397?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/397?src=pr&el=footer). Last update [458e479...a3f393c](https://codecov.io/gh/apache/cordova-android/pull/397?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request #397: CB-13031: Fix bug with case-sensitivity o...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-android/pull/397
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request #397: CB-13031: Fix bug with case-sensitivity o...
Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/397#discussion_r129094172
--- Diff: bin/templates/cordova/lib/prepare.js ---
@@ -214,7 +215,11 @@ function updateProjectAccordingTo (platformConfig, locations) {
shell.sed(/package [\w\.]*;/, 'package ' + pkg + ';', java_files[0]).to(destFile);
events.emit('verbose', 'Wrote out Android package name "' + pkg + '" to ' + destFile);
- if (orig_pkg !== pkg) {
+ var removeOrigPkg = checkReqs.isWindows() || checkReqs.isDarwin() ?
+ orig_pkg.toUpperCase() !== pkg.toUpperCase() :
+ orig_pkg !== pkg;
--- End diff --
I know the naming of these variables didn't originate from you, but while we're here, can we also update the variable names used here? I found `orig_pkg` and `pkg` to be not very descriptive. Maybe `manifest_id` instead of `orig_pkg`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android issue #397: CB-13031: Fix bug with case-sensitivity of andro...
Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/397
Hey @matrosov-nikita, rebase w/ latest master, I should have a fix (workaround) in for appveyor troubles.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android issue #397: CB-13031: Fix bug with case-sensitivity of andro...
Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/397
Just for the record, here's the full stack trace I get when I try these steps:
```
~/src
➔ cordova create testapp com.example.testapp testapp
Warning: using prerelease version 7.0.2-dev (cordova-lib@7.0.1)
Creating a new cordova project.
~/src
➔ cd testapp
~/src/testapp
➔ vim config.xml
~/src/testapp
➔ cordova platform add android
Warning: using prerelease version 7.0.2-dev (cordova-lib@7.0.1)
Using cordova-fetch for cordova-android@~6.2.2
Adding android project...
Creating Cordova project for the Android platform:
Path: platforms/android
Package: com.example.testapp
Name: testapp
Activity: MainActivity
Android target: android-25
Subproject Path: CordovaLib
Android project created with cordova-android@6.2.3
Discovered plugin "cordova-plugin-whitelist" in config.xml. Adding it to the project
Installing "cordova-plugin-whitelist" for android
This plugin is only applicable for versions of cordova-android greater than 4.0. If you have a previous platform version, you do *not* need this plugin since the whitelist will be built in.
Adding cordova-plugin-whitelist to package.json
Saved plugin info for "cordova-plugin-whitelist" to config.xml
ls: no such file or directory: /Users/maj/src/testapp/platforms/android/src/com/example/Testapp/*.java
Error: No Java files found that extend CordovaActivity.
```
This patch seems to fix that. However, I noticed that the output even using this patch still says:
```
Package: com.example.testapp
```
Can we update the PR to also print out the correct, overridden package name, if provided?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android issue #397: CB-13031: Fix bug with case-sensitivity of andro...
Posted by matrosov-nikita <gi...@git.apache.org>.
Github user matrosov-nikita commented on the issue:
https://github.com/apache/cordova-android/pull/397
@filmaj, thanks for the information, I'm planning to merge in once Appveyor is fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android issue #397: CB-13031: Fix bug with case-sensitivity of andro...
Posted by matrosov-nikita <gi...@git.apache.org>.
Github user matrosov-nikita commented on the issue:
https://github.com/apache/cordova-android/pull/397
@filmaj, updated. But I little doubt about folders names after prepare, this PR just skips the removal of an original package if names (manifestId and androidPkgName) differ in casing only. So, `platforms\android\src\com\example\testapp` is still in the lower case despite appropriate changes in the manifest.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org