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

Bump fresco to 2.3.0 #29791

Closed
wants to merge 2 commits into from
Closed

Bump fresco to 2.3.0 #29791

wants to merge 2 commits into from

Conversation

mikehardy
Copy link
Contributor

@mikehardy mikehardy commented Aug 27, 2020

Summary

Fresco animated-gif and drawee pipeline has an incompatible change from 2.2 to 2.3 and it needs a bump here to use newer fresco:
facebook/fresco@036605d

OkHTTP 3.12.x all work with Android 4 (minSdkVersion 16), no need to sit on 3.12.1

Robolectric is test only but better to stay current if possible

Changelog

[Android] [Changed] - Bump fresco (2.3.0), okhttp (3.12.12), robolectric (4.4)

Test Plan

I use OkHTTP 3.12.12 in other projects, it works, down to API16 (https://github.com/ankidroid/Anki-Android/blob/44650ca45ab5aac03211cd483dc8d740dd4e4773/AnkiDroid/build.gradle#L249)

Fresco animated-gif 2.2 works fine but attempting to use current stable (2.3) pointed out this incompatibility. CI should show that it works fine?

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Sony Partner: Sony labels Aug 27, 2020
@react-native-bot react-native-bot added the Platform: Android Android applications. label Aug 27, 2020
@analysis-bot
Copy link

analysis-bot commented Aug 27, 2020

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 5acf7c9

@dulmandakh
Copy link
Contributor

@mikehardy you need to update buck dependencies too

@mikehardy
Copy link
Contributor Author

@dulmandakh sorry - I know nothing of buck, I'm a non-facebook type person and have never used buck in real life - any pointers?

@dulmandakh
Copy link
Contributor

Please look into #29741 which bumps OkHTTP

@mikehardy
Copy link
Contributor Author

Ah perfect thanks @dulmandakh - need to remove okhttp from my PR then, verify there are no others active for robolectric and/or fresco, then re-push with buck changes. Thanks for the pointer

@mikehardy mikehardy marked this pull request as draft August 28, 2020 17:16
Fresco animated-gif and drawee pipeline has an incompatible change from 2.2 to 2.3 and it needs a bump here to use newer fresco:
facebook/fresco@036605d

Note that to find the SHA1 entries for BUCK, you need to surf around
in this maven tree at the corresponding URLs, for example:
https://repo1.maven.org/maven2/com/facebook/fresco/imagepipeline-okhttp3/2.3.0/imagepipeline-okhttp3-2.3.0.aar.sha1
@mikehardy mikehardy marked this pull request as ready for review September 4, 2020 19:34
@mikehardy
Copy link
Contributor Author

@dulmandakh I believe I have BUCK updated correctly, and this applies cleanly to master. Should be ready to go?

@mikehardy mikehardy changed the title Bump fresco (2.3.0), okhttp (3.12.12), robolectric (4.4) Bump fresco to 2.3.0 Sep 4, 2020
@mikehardy
Copy link
Contributor Author

CircleCI refuses to let me in and see the details, so I can't investigate to see if that's from this change or not 🤷

@safaiyeh
Copy link
Contributor

safaiyeh commented Sep 6, 2020

Here ya go @mikehardy

test_android

stderr: /root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.java:551: error: cannot access com.facebook.fresco.ui.common.OnDrawControllerListener
      combinedListener.addListener(mDownloadListener);
                      ^
  class file for com.facebook.fresco.ui.common.OnDrawControllerListener not found
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageDownloadListener.java:-1: note: Some input files use or override a deprecated API.

/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageDownloadListener.java:-1: note: Recompile with -Xlint:deprecation for details.

/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageManager.java:-1: note: Some input files use unchecked or unsafe operations.

/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageManager.java:-1: note: Recompile with -Xlint:unchecked for details.

Errors: 1. Warnings: 0.

    When running <javac>.
    When building rule //ReactAndroid/src/main/java/com/facebook/react/views/image:image.

Exited with code exit status 1

test_docker

stderr: /app/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.java:551: error: cannot access com.facebook.fresco.ui.common.OnDrawControllerListener
      combinedListener.addListener(mDownloadListener);
                      ^
  class file for com.facebook.fresco.ui.common.OnDrawControllerListener not found
/app/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageDownloadListener.java:-1: note: Some input files use or override a deprecated API.

/app/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageDownloadListener.java:-1: note: Recompile with -Xlint:deprecation for details.

/app/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageManager.java:-1: note: Some input files use unchecked or unsafe operations.

/app/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageManager.java:-1: note: Recompile with -Xlint:unchecked for details.

@mikehardy
Copy link
Contributor Author

Ah okay, looks like it will need a little forward porting. Interesting that a semver minor caused a break but hey, versioning. I'll look into it thanks @safaiyeh

@dulmandakh
Copy link
Contributor

hey @mikehardy, I created a PR to bump Fresco to 2.3.0 #30061

@mikehardy
Copy link
Contributor Author

Thanks @dulmandakh i was just out of time so far, I appreciate it!

@mikehardy mikehardy closed this Sep 29, 2020
@mikehardy mikehardy deleted the patch-1 branch November 17, 2020 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Sony Partner: Sony Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants