New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error/warning while building with android studio #24145

Merged
merged 1 commit into from Dec 18, 2018

Conversation

Projects
None yet
2 participants
@volzhs
Copy link
Member

volzhs commented Dec 3, 2018

min/target sdk should be defined in build.gradle, not in manifest.xml

updated : it's down to 16 now.
updated 2 : it's down to 1 now.

edit : working on other errors and warnings

Functionality test

  • Touches
  • Sound
  • Text input
  • In-app purchase
  • Download .obb expansion

@volzhs volzhs added this to the 3.1 milestone Dec 3, 2018

@volzhs volzhs changed the title Fix error/warning while building with android studio [WIP] Fix error/warning while building with android studio Dec 3, 2018

@volzhs volzhs force-pushed the volzhs:android-version-gradle branch from 2757d5f to 87274e1 Dec 4, 2018

@volzhs volzhs modified the milestones: 3.1, 3.2 Dec 4, 2018

@volzhs

This comment has been minimized.

Copy link
Member Author

volzhs commented Dec 4, 2018

I think this needs to be tested, so let's merge it after releasing 3.1-stable

@@ -1,5 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
package="com.godot.game"

This comment has been minimized.

@akien-mga

akien-mga Dec 4, 2018

Member

There seems to be a mixed use of tabs and spaces in this file, could you also fix that?

@akien-mga

This comment has been minimized.

Copy link
Member

akien-mga commented Dec 4, 2018

If some of those fixes are trivial enough, it might be worth splitting them into another PR that could be merged for 3.1, while we keep the more complex/risky ones for 3.2.

@akien-mga

This comment has been minimized.

Copy link
Member

akien-mga commented Dec 4, 2018

Would likely fix #6626 BTW.

@volzhs volzhs force-pushed the volzhs:android-version-gradle branch from 87274e1 to 2e2063c Dec 5, 2018

@volzhs

This comment has been minimized.

Copy link
Member Author

volzhs commented Dec 5, 2018

it's now down to 13 issues.
those seem tricky to fix

@volzhs volzhs closed this Dec 11, 2018

@volzhs volzhs deleted the volzhs:android-version-gradle branch Dec 11, 2018

@akien-mga

This comment has been minimized.

Copy link
Member

akien-mga commented Dec 11, 2018

Why close?

@akien-mga akien-mga added the archived label Dec 11, 2018

@volzhs

This comment has been minimized.

Copy link
Member Author

volzhs commented Dec 11, 2018

oh... it's by mistake...

@volzhs volzhs restored the volzhs:android-version-gradle branch Dec 11, 2018

@volzhs volzhs reopened this Dec 11, 2018

@akien-mga akien-mga removed the archived label Dec 11, 2018

@volzhs volzhs force-pushed the volzhs:android-version-gradle branch from 2e2063c to a8596f4 Dec 18, 2018

@volzhs

This comment has been minimized.

Copy link
Member Author

volzhs commented Dec 18, 2018

#Weak RNG

../../src/com/google/android/vending/licensing/LicenseChecker.java:68: Potentially insecure random numbers on Android 4.3 and older. Read https://android-developers.blogspot.com/2013/08/some-securerandom-thoughts.html for more info.

  66     private static final int TIMEOUT_MS = 10 * 1000;
  67 
  68     private static final SecureRandom RANDOM = new SecureRandom();                                  
  69     private static final boolean DEBUG_LICENSE_ERROR = false;
  70 
  71     private ILicensingService mService;

Now it's only 1 issue left.
it says this issue could be a problem on Android 4.3(api 18) or older.

based on https://developer.android.com/about/dashboards/
screenshot_20181218_100733
about 0.4% of devices could have a issue with it. (because we support api 18 or above)

if we must fix this issue, I would like to let someone fix it, I currenlty don't have a idea to fix it.
or we can ignore it by adding TrulyRandom to lint options.

@akien-mga what do you think?

@volzhs volzhs force-pushed the volzhs:android-version-gradle branch 3 times, most recently from 4cbdb7b to 98a2a1b Dec 18, 2018

@volzhs

This comment has been minimized.

Copy link
Member Author

volzhs commented Dec 18, 2018

@akien-mga I tested most functions.
it seems pretty stable state.
would it be better to merge it before 3.1-beta out?

@akien-mga

This comment has been minimized.

Copy link
Member

akien-mga commented Dec 18, 2018

The changes to platform/android/java/src/com/android/vending come from upstream? Could you document the provenance and process to update them in a platform/android/java/README.md file?

@volzhs

This comment has been minimized.

Copy link
Member Author

volzhs commented Dec 18, 2018

@volzhs volzhs force-pushed the volzhs:android-version-gradle branch from 98a2a1b to 067be04 Dec 18, 2018

@volzhs volzhs changed the title [WIP] Fix error/warning while building with android studio Fix error/warning while building with android studio Dec 18, 2018

@volzhs

This comment has been minimized.

Copy link
Member Author

volzhs commented Dec 18, 2018

@akien-mga updated.

### Modify some files to avoid compile error and lint warning

#### com/google/android/vending/licensing/util/Base64.java
<pre><code>

This comment has been minimized.

@akien-mga

akien-mga Dec 18, 2018

Member

You can use

```

like on GitHub I think. Aren't pre + code redundant too?

This comment has been minimized.

@akien-mga

akien-mga Dec 18, 2018

Member

Maybe even

```diff
...
```

to get proper syntax highlighting, at least on GitHub.

This comment has been minimized.

@volzhs

volzhs Dec 18, 2018

Author Member

I will update it soon.

@volzhs volzhs force-pushed the volzhs:android-version-gradle branch from 067be04 to b385a4b Dec 18, 2018

@volzhs

This comment has been minimized.

Copy link
Member Author

volzhs commented Dec 18, 2018

updated

@akien-mga akien-mga modified the milestones: 3.2, 3.1 Dec 18, 2018

@akien-mga akien-mga merged commit df78e80 into godotengine:master Dec 18, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@akien-mga

This comment has been minimized.

Copy link
Member

akien-mga commented Dec 18, 2018

Thanks!

@volzhs volzhs deleted the volzhs:android-version-gradle branch Dec 18, 2018

akien-mga added a commit to akien-mga/godot that referenced this pull request Dec 20, 2018

akien-mga added a commit to akien-mga/godot that referenced this pull request Dec 20, 2018

akien-mga added a commit to akien-mga/godot that referenced this pull request Dec 20, 2018

akien-mga added a commit to akien-mga/godot that referenced this pull request Dec 20, 2018

Android: Drop unused thirdparty Google code
The `cpu-features.{c,h}` code was used by chance by the webm/libvpx
code, so I moved it there. It could potentially be compiled on the fly
from the Android NDK, but since we plan to replace the webm module by
a GDNative plugin in the near future, I went the bundling route.

`ifaddrs_android.h` is also provided in the Android NDK as `ifaddrs.h`,
same as on other Unixes, so we use that.

Also removes dropped thirdparty files from COPYRIGHT.txt
Reflects the changes from godotengine#24105 and godotengine#24145.

akien-mga added a commit to akien-mga/godot that referenced this pull request Dec 20, 2018

Android: Better identify thirdparty code
- The `cpu-features.{c,h}` code was only used by chance by the webm
  (libvpx) code, so I moved it there. It was actually introduced before
  that and wasn't in use, and libvpx just happened to be able to
  compile thanks to it being bundled.
  It could potentially be compiled on the fly from the Android NDK, but
  since we plan to replace the webm module by a GDNative plugin in the
  near future, I went the bundling route.

- `ifaddrs_android.h` is already provided in the Android NDK as
  `ifaddrs.h`, same as on other Unixes. Yet we cannot use it until we
  up the min API level to 24, where `getifaddrs` is first defined.
  I moved the files to `thirdparty/misc` and synced them with upstream
  WebRTC (only indentation changes and removal of `static` qualifiers).

Also removes dropped thirdparty files from COPYRIGHT.txt after changes
in godotengine#24105 and godotengine#24145.

akien-mga added a commit to akien-mga/godot that referenced this pull request Dec 20, 2018

Android: Better identify thirdparty code
- The `cpu-features.{c,h}` code was only used by chance by the webm
  (libvpx) code, so I moved it there. It was actually introduced before
  that and wasn't in use, and libvpx just happened to be able to
  compile thanks to it being bundled.
  It could potentially be compiled on the fly from the Android NDK, but
  since we plan to replace the webm module by a GDNative plugin in the
  near future, I went the bundling route.

- `ifaddrs_android.h` is already provided in the Android NDK as
  `ifaddrs.h`, same as on other Unixes. Yet we cannot use it until we
  up the min API level to 24, where `getifaddrs` is first defined.
  I moved the files to `thirdparty/misc` and synced them with upstream
  WebRTC (only indentation changes and removal of `static` qualifiers).

Also removes dropped thirdparty files from COPYRIGHT.txt after changes
in godotengine#24105 and godotengine#24145.

akien-mga added a commit to akien-mga/godot that referenced this pull request Dec 20, 2018

Android: Better identify thirdparty C/C++ code
- The `cpu-features.{c,h}` code was only used by chance by the webm
  (libvpx) code, so I moved it there. It was actually introduced before
  that and wasn't in use, and libvpx just happened to be able to
  compile thanks to it being bundled.
  It could potentially be compiled on the fly from the Android NDK, but
  since we plan to replace the webm module by a GDNative plugin in the
  near future, I went the bundling route.

- `ifaddrs_android.h` is already provided in the Android NDK as
  `ifaddrs.h`, same as on other Unixes. Yet we cannot use it until we
  up the min API level to 24, where `getifaddrs` is first defined.
  I moved the files to `thirdparty/misc` and synced them with upstream
  WebRTC (only indentation changes and removal of `static` qualifiers).

Also removes dropped thirdparty files from COPYRIGHT.txt after changes
in godotengine#24105 and godotengine#24145.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment