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

Move mavenCentral repo below local paths #32326

Closed

Conversation

friederbluemle
Copy link
Contributor

Summary

In #31609, the deprecated jcenter() was replaced with mavenCentral(). In the template build.gradle, it also changed the order of repos. I am not sure if this was done intentionally or not (@dulmandakh please confirm). Instead of appearing right after google(), mavenCentral() was put first in the list, even before the local repos (that, for example, contain the react-native artifacts fetched by npm). Now, under normal circumstance, this might not cause issues because of latency, but there is chance that Gradle could resolve incorrect versions (or at least look in the wrong repo first). The last version of react-native published to the public repo was 0.20.1, uploaded in February 2016!

This PR changes the order of mavenCentral() so that is consistent with both the repo's current root level build.gradle, as well as other default Android templates. Putting the local repos first will ensure they have the highest priority when looking for artifacts. react-native should always come from the locally downloaded node_modules/ folder, not from a remote repo.

Changelog

[Android] [Changed] - Move mavenCentral repo below local paths

Test Plan

Create new app from template, ensure local repos appear before remote repos; react-native resolves to correct version.

@react-native-bot react-native-bot added the Platform: Android Android applications. label Oct 3, 2021
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,716,028 +0
android hermes armeabi-v7a 7,244,255 +0
android hermes x86 8,135,345 +0
android hermes x86_64 8,100,900 +0
android jsc arm64-v8a 9,635,310 +0
android jsc armeabi-v7a 8,551,062 +0
android jsc x86 9,648,919 +0
android jsc x86_64 10,258,149 +0

Base commit: 36f3bf2

@analysis-bot
Copy link

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

Base commit: 36f3bf2

@react-native-bot react-native-bot added the No CLA Authors need to sign the CLA before a PR can be reviewed. label Oct 4, 2021
@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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Oct 4, 2021
@react-native-bot react-native-bot removed the No CLA Authors need to sign the CLA before a PR can be reviewed. label Oct 4, 2021
@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@cortinico
Copy link
Contributor

Thanks for addressing this.

Now, under normal circumstance, this might not cause issues because of latency, but there is chance that Gradle could resolve incorrect versions (or at least look in the wrong repo first). The last version of react-native published to the public repo was 0.20.1, uploaded in February 2016!

You're right. I got actually affected by this multiple times. I believe we could:

    mavenCentral {
        content {
            excludeGroup "com.facebook.react"
        }
    }

So Gradle will never fetch RN from mavenCentral (at least till we resort publishing artifacts there).
As a side note: this is actually an issue if you end up using the nightly builds as the version number is 0.0.0-... which is lower than the version on Maven Central (0.20.1). So the Maven Central build will be used instead of a nightly.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @friederbluemle in 046b026.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 5, 2021
@friederbluemle friederbluemle deleted the templ-gradle-repo-order branch October 5, 2021 20:55
facebook-github-bot pushed a commit that referenced this pull request Oct 13, 2021
Summary:
This Diff is restricting the scope of `mavenCentral` to do not
include react-native packages. This will make us sure we don't pickup older
versions of react-native.
This specifically is a problem if you're building on a nightly as the version
of RN nightly is `0.0.0.xxx` which is lower than then version on maven central.
More on this here #32326 (comment)

Changelog:
[Internal] [Changed] - Restrict mavenCentral to exclude react-native older packages

Reviewed By: ShikaSD

Differential Revision: D31571803

fbshipit-source-id: d7ce7e82825cbebda2e4e534565d7ab15dba2624
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. Merged This PR has been merged. Platform: Android Android applications. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants