Fix build on Apple Silicon macs #7782

Atemu · 2022-02-03T13:26:43Z

What is it?

Bugfix (user facing)
Feature (user facing)
Codebase improvement (dev facing)
Meta improvement to the project (dev facing)

Description of the changes in your PR

Fixes the build on M1 macs.
For more information see this comment

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

I read the contribution guidelines.

Required for newer versions of some dependencies

triallax · 2022-02-03T17:57:46Z

app/build.gradle

@@ -101,7 +101,7 @@ ext {
    checkstyleVersion = '9.2.1'

    androidxLifecycleVersion = '2.3.1'
-    androidxRoomVersion = '2.3.0'
+    androidxRoomVersion = '2.4.1'


Changelog: https://developer.android.com/jetpack/androidx/releases/room

Before merging, we should review the changelog carefully, and test these changes in the app.

I'll be running this patch backported to master in the coming days.

litetex

@Atemu
Thank you for the PR.

Fixes the build on M1 macs.

That's not enough.
Please describe how your changes fix the problem.

This version of Room includes a fix for building dependant apps such as NewPipe on Apple Silicon devices (aarch64-darwin)

sonarqubecloud · 2022-02-04T09:07:49Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

litetex

Not sure why my review is requested but the situation since
#7782 (review)
hasn't changed...

triallax · 2022-02-07T19:09:23Z

@litetex it somewhat did, read the commit messages.

litetex · 2022-02-07T19:24:20Z

@litetex it somewhat did, read the commit messages.

Thanks for the hint, however 2 problems:

A commit message != a comment here
"compileSdk 31" is still open and waiting for explanation

Atemu · 2022-02-07T20:15:05Z

@litetex ah, you meant the PR comment! I thought you meant the commit messages.

You did not mention anything regarding compileSdk 31 in your review though, I'm a bit confused.

litetex · 2022-02-08T20:14:08Z

You did not mention anything regarding compileSdk 31 in your review though, I'm a bit confused.

Well you updated the compileSdk to 31.
Why did you do that?

triallax · 2022-02-08T20:21:50Z

From the commit message:

Required for newer versions of some dependencies

So I guess it's needed for the dependency update in the other commit.

Atemu · 2022-02-08T20:23:04Z

Should I be more specific about it or even put it in the same commit?

litetex · 2022-02-09T19:07:48Z

Simply answer my questions in a comment next time. The commit messages are great too but they are easily overlooked in a conversation here.

I also still don't know

what problem with the current build is present on Mac's. Our CI (which uses MacOS) works fine. Is this a problem with Android studio, the emulator, the build or something else?
how the above changes (updating a dependency) fixed the problem on Mac. I see no fixed bug regarding MacOS in the changelogs.

It would be great too have things like stacktraces, bug reports, links, etc which we can refer to?

ktprograms · 2022-02-10T00:57:58Z

@litetex It's a problem that earlier versions of Android Room can't be used on Arm64 (M1) Macs.

AFAIK, It can be fixed either by updating the Room dependency, or by adding

kapt "org.xerial:sqlite-jdbc:3.34.0"

to the dependencies.

If you don't want to update to room 2.4.0, then I recommend using the kapt line. I have been putting it in my app/build.gradle (around line 218, below the kapt for room-compiler) for several months now, and the built APKs don't seem to be broken in any way.

ktprograms · 2022-02-10T01:02:11Z

@litetex Here's where it's mentioned in the Android Room Changelogs: https://developer.android.com/jetpack/androidx/releases/room#2.4.0-alpha03 (first bullet point under the Bug Fixes heading).

Fixed an issue with Room’s SQLite native library to support Apple’s M1 chips. (b/174695268

Atemu · 2022-02-10T14:54:37Z

how the above changes (updating a dependency) fixed the problem on Mac. I see no fixed bug regarding MacOS in the changelogs.

Should I really include a link to commit/patchnotes? That seems a bit overkill for a simple dependency bump that includes a fix.

AFAIK, It can be fixed either by updating the Room dependency, or by adding
kapt "org.xerial:sqlite-jdbc:3.34.0"
to the dependencies.

Neat, didn't know that.

I'd still prefer upgrading room over injecting just the updated jdbc. We should update room at some point anyways and it's been working perfectly for me the past week (with the caveat that I backported it to master).

litetex · 2022-02-10T18:23:24Z

Thx @ktprograms for clarifying that.
Now I understand what is going on here 👍

Should I really include a link to commit/patchnotes?

👉

Simply answer my questions in a comment next time

litetex

LGTM

Did a quick test and couldn't find any problems

Stypox · 2022-02-16T22:27:04Z

Warning: the bump to compileSdk should be tested carefully on Android 12 and non-12 (devices)/emulators. This this is so simple to revert, I'd merge this after 0.22.0 is released and then test on the dev branch if anything wrong happens when updating the compile sdk.

This will allow newer libraries to be used, see TeamNewPipe#7782 and TeamNewPipe#2335. This should have no changes on the app since the targetSdk stayed the same.

Atemu · 2022-02-26T19:25:02Z

Thanks!

app/build.grade: compileSdk 30 -> 31

3a9cdb2

Required for newer versions of some dependencies

triallax reviewed Feb 3, 2022

View reviewed changes

litetex requested changes Feb 3, 2022

View reviewed changes

litetex added bug Issue is related to a bug device/software specific Issues that only happen on some devices or with some specific hardware/software labels Feb 3, 2022

app/build.grade: androidxRoomVersion 2.3.0 -> 2.4.1

67b2503

This version of Room includes a fix for building dependant apps such as NewPipe on Apple Silicon devices (aarch64-darwin)

Atemu force-pushed the apple-silicon branch from 86a4b90 to 67b2503 Compare February 4, 2022 08:57

Atemu requested a review from litetex February 4, 2022 08:58

litetex requested changes Feb 7, 2022

View reviewed changes

litetex approved these changes Feb 10, 2022

View reviewed changes

Stypox mentioned this pull request Feb 18, 2022

New streams notifications #2335

Merged

1 task

Stypox added a commit to Stypox/NewPipe that referenced this pull request Feb 19, 2022

Change compileSdk from 30 to 31

579b861

This will allow newer libraries to be used, see TeamNewPipe#7782 and TeamNewPipe#2335. This should have no changes on the app since the targetSdk stayed the same.

Stypox mentioned this pull request Feb 19, 2022

Change compileSdk from 30 to 31 #7900

Merged

5 tasks

litetex merged commit a4dee77 into TeamNewPipe:dev Feb 26, 2022

Atemu deleted the apple-silicon branch February 26, 2022 19:24

TacoTheDank mentioned this pull request Feb 26, 2022

Update Kotlin to 1.6.10 #7952

Merged

5 tasks

litetex mentioned this pull request Mar 2, 2022

Sonarcloud: Remove Security hotspots from quality gate #7980

Closed

Stypox mentioned this pull request Apr 16, 2022

Release 0.23.0 (please TEST!) #8230

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix build on Apple Silicon macs #7782

Fix build on Apple Silicon macs #7782

Atemu commented Feb 3, 2022 •

edited by litetex

Loading

triallax Feb 3, 2022 •

edited

Loading

Atemu Feb 4, 2022

litetex left a comment

sonarqubecloud bot commented Feb 4, 2022

litetex left a comment

triallax commented Feb 7, 2022

litetex commented Feb 7, 2022

Atemu commented Feb 7, 2022

litetex commented Feb 8, 2022

triallax commented Feb 8, 2022

Atemu commented Feb 8, 2022

litetex commented Feb 9, 2022

ktprograms commented Feb 10, 2022 •

edited

Loading

ktprograms commented Feb 10, 2022

Atemu commented Feb 10, 2022

litetex commented Feb 10, 2022

litetex left a comment

Stypox commented Feb 16, 2022

Atemu commented Feb 26, 2022

Fix build on Apple Silicon macs #7782

Fix build on Apple Silicon macs #7782

Conversation

Atemu commented Feb 3, 2022 • edited by litetex Loading

What is it?

Description of the changes in your PR

APK testing

Due diligence

triallax Feb 3, 2022 • edited Loading

Choose a reason for hiding this comment

Atemu Feb 4, 2022

Choose a reason for hiding this comment

litetex left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Feb 4, 2022

litetex left a comment

Choose a reason for hiding this comment

triallax commented Feb 7, 2022

litetex commented Feb 7, 2022

Atemu commented Feb 7, 2022

litetex commented Feb 8, 2022

triallax commented Feb 8, 2022

Atemu commented Feb 8, 2022

litetex commented Feb 9, 2022

ktprograms commented Feb 10, 2022 • edited Loading

ktprograms commented Feb 10, 2022

Atemu commented Feb 10, 2022

litetex commented Feb 10, 2022

litetex left a comment

Choose a reason for hiding this comment

Stypox commented Feb 16, 2022

Atemu commented Feb 26, 2022

Atemu commented Feb 3, 2022 •

edited by litetex

Loading

triallax Feb 3, 2022 •

edited

Loading

ktprograms commented Feb 10, 2022 •

edited

Loading