Skip to content
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

Correct minSdkVersion to 19 #612

Closed
wants to merge 1 commit into from
Closed

Conversation

zx2c4
Copy link

@zx2c4 zx2c4 commented Feb 19, 2021

zxing:core never increased the minimum version to 24. It increased it to
19: https://github.com/zxing/zxing/blob/44267115/android/AndroidManifest.xml#L34

This most recent bump happened here: zxing/zxing@04595508b

Now, this disagrees with the release notes of 3.4.0:
https://github.com/zxing/zxing/releases/tag/zxing-3.4.0 which say the
minimum verison is API 24 due to Java 8. But these days, AGP desugars
Java 8 automatically, so you can, in fact, use it with older APIs.

See this for more information:
https://developer.android.com/studio/releases#4-0-0-desugar


CC @msfjarvis
Also #606 (comment)

zxing:core never increased the minimum version to 24. It increased it to
19: https://github.com/zxing/zxing/blob/44267115/android/AndroidManifest.xml#L34

This most recent bump happened here: zxing/zxing@04595508b

Now, this disagrees with the release notes of 3.4.0:
https://github.com/zxing/zxing/releases/tag/zxing-3.4.0 which say the
minimum verison is API 24 due to Java 8. But these days, AGP desugars
Java 8 automatically, so you can, in fact, use it with older APIs.

See this for more information:
https://developer.android.com/studio/releases#4-0-0-desugar
@rkistner
Copy link
Member

rkistner commented Feb 19, 2021

Thanks, I wasn't aware of the new desugaring support - this looks very useful.

It seems like this might complicate an application build process a little, so we'd need to expand the documentation for this.

We'd also need to test that all the Java 8 APIs used by zxing:core are actually supported by the desugaring.

Note that the released zxing Barcode Scanner app hasn't really been updated since 2018 and still uses zxing:core 3.3.0, which is why it still has minSdkVersion of 19.

In the meantime, you can override the minSdkVersion in your application by setting this in your manifest:

<uses-sdk tools:overrideLibrary="com.google.zxing.client.android" />

@msfjarvis
Copy link

Thanks, I wasn't aware of the new desugaring support - this looks very useful.

It seems like this might complicate an application build process a little, so we'd need to expand the documentation for this.

I don't expect AGP 4.0.0 to be a problem, considering it came out in May 2020 and active development teams don't typically stick to outdated Android Studio releases.

We'd also need to test that all the Java 8 APIs used by zxing:core are actually supported by the desugaring.

Do they actually use any new APIs? AFAICT, the requirement for Java 8 features was for lambdas and try-with-resources, both of which are handled fine by D8.

Note that the released zxing Barcode Scanner app hasn't really been updated since 2018 and still uses zxing:core 3.3.0, which is why it still has minSdkVersion of 19.

In the meantime, you can override the minSdkVersion in your application by setting this in your manifest:

<uses-sdk tools:overrideLibrary="com.google.zxing.client.android" />

Yep, that's what we did to test against API 21 and 23.

@msfjarvis
Copy link

In the diff between zxing 3.3.3 and 3.4.0 the only minSdk sensitive changes are the use of Objects#equals and Objects#hashCode, both of which were added in API 19.

@msfjarvis
Copy link

Hey @rkistner, are there any remaining blockers for this PR?

@msfjarvis
Copy link

@rkistner are there any plans to merge this?

@rkistner
Copy link
Member

I've reworked this a bit in #665 / v4.3.0:

  1. minSdkVersion is now 19.
  2. The docs still recommend 24 as the default minSdkVersion for the application. But the readme now includes instructions for desugaring.

I found that desugaring is not necessarily trivial, since it also requires multiDex to be enabled. So I don't recommend it as the default for now, but there are notes in the readme for it.

@rkistner rkistner closed this Oct 25, 2021
@msfjarvis
Copy link

msfjarvis commented Oct 25, 2021

I found that desugaring is not necessarily trivial, since it also requires multiDex to be enabled. So I don't recommend it as the default for now, but there are notes in the readme for it.

FWIW, desugaring does not require multidex above API 19 20.

@rkistner
Copy link
Member

Good point, I'll update the readme with that.

@zx2c4 zx2c4 deleted the api19 branch October 26, 2021 08:37
@zx2c4
Copy link
Author

zx2c4 commented Oct 26, 2021

I found that desugaring is not necessarily trivial, since it also requires multiDex to be enabled. So I don't recommend it as the default for now, but there are notes in the readme for it.

Since this supposition turns out to be not correct, it turns out you can recommend it as the default? After all, you're not supporting < 19.

@rkistner
Copy link
Member

@zx2c4 Are you happy with the changes here? #666

@zx2c4
Copy link
Author

zx2c4 commented Oct 26, 2021

Not really. I'll comment over there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants