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

Suggest git apply --reject for failed upgrades #18636

Closed
wants to merge 1 commit into from
Closed

Suggest git apply --reject for failed upgrades #18636

wants to merge 1 commit into from

Conversation

getaaron
Copy link
Contributor

@getaaron getaaron commented Mar 31, 2018

The upgrade script uses git apply --3way:

log.info('Apply the patch');
await exec(`git apply --3way ${patchPath}`, true, (data, stream) => {

If you've already upgraded something that React Native upgraded, this will silently fail. For example if you have already upgraded Gradle to the version available in the new version of React Native, the 3-way patch will fail. In this case, the upgraded version is installed into node_modules but package.json is not updated. More context, discussion, and information is in #12112 (comment).

Until a more robust solution is implemented, this PR proposes to mitigate the effects of the problem by informing the user of the possible issue and instructing them of a workaround.

Test Plan

In an affected project, I ran react-native-git-upgrade after applying my patch.

As expected, I now receive the following output:

image

After running the suggested command, git status now shows me the two rejections:

image

And I can now manually apply these changes.

Related PRs

#17266 proposed to solve the problem by switching from --3way to --reject entirely, which I think was a subpar solution since --3way is the happy path. This PR was closed due to the author not signing the CLA.

Release Notes

[CLI] [ENHANCEMENT] [react-native-git-upgrade/cliEntry.js] - Provide instructions for upgrading React Native when 3-way merges don't apply

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 31, 2018
@pull-bot
Copy link

Warnings
⚠️

📋 Release Notes - This PR may have incorrectly formatted Release Notes.

Generated by 🚫 dangerJS

@react-native-bot react-native-bot added the Missing Changelog This PR appears to be missing a changelog, or they are incorrectly formatted. label Mar 31, 2018
@getaaron
Copy link
Contributor Author

@react-native-bot I updated the release notes

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Apr 12, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

bunnyc1986 pushed a commit to bunnyc1986/react-native that referenced this pull request May 11, 2018
Summary:
The upgrade script uses `git apply --3way`:

https://github.com/facebook/react-native/blob/4906f8d28cdbaf1b432494b473a4c61c1e193881/react-native-git-upgrade/cliEntry.js#L368-L369

If you've already upgraded something that React Native upgraded, this will silently fail. For example if you have already upgraded Gradle to the version available in the new version of React Native, the 3-way patch will fail. In this case, the upgraded version is installed into `node_modules` but `package.json` is not updated. More context, discussion, and information is in facebook#12112 (comment).

Until a more robust solution is implemented, this PR proposes to mitigate the effects of the problem by informing the user of the possible issue and instructing them of a workaround.

In an affected project, I ran `react-native-git-upgrade` after applying my patch.

As expected, I now receive the following output:

![image](https://user-images.githubusercontent.com/789577/38165770-6209ac68-34de-11e8-9d96-7c782e9ef7ce.png)

After running the suggested command, `git status` now shows me the two rejections:

![image](https://user-images.githubusercontent.com/789577/38165796-d10ca7c8-34de-11e8-9037-8427513e3a84.png)

And I can now [manually apply](https://stackoverflow.com/a/28569128/1445366) these changes.

[CLI] [ENHANCEMENT] [react-native-git-upgrade/cliEntry.js] - Provide instructions for upgrading React Native when 3-way merges don't apply
Closes facebook#18636

Differential Revision: D7603073

Pulled By: hramos

fbshipit-source-id: 32d387e1ee67d8b182d248c585cd33ba94808357
@getaaron getaaron deleted the patch-1 branch May 15, 2018 15:50
macdoum1 pushed a commit to macdoum1/react-native that referenced this pull request Jun 28, 2018
Summary:
The upgrade script uses `git apply --3way`:

https://github.com/facebook/react-native/blob/4906f8d28cdbaf1b432494b473a4c61c1e193881/react-native-git-upgrade/cliEntry.js#L368-L369

If you've already upgraded something that React Native upgraded, this will silently fail. For example if you have already upgraded Gradle to the version available in the new version of React Native, the 3-way patch will fail. In this case, the upgraded version is installed into `node_modules` but `package.json` is not updated. More context, discussion, and information is in facebook#12112 (comment).

Until a more robust solution is implemented, this PR proposes to mitigate the effects of the problem by informing the user of the possible issue and instructing them of a workaround.

In an affected project, I ran `react-native-git-upgrade` after applying my patch.

As expected, I now receive the following output:

![image](https://user-images.githubusercontent.com/789577/38165770-6209ac68-34de-11e8-9d96-7c782e9ef7ce.png)

After running the suggested command, `git status` now shows me the two rejections:

![image](https://user-images.githubusercontent.com/789577/38165796-d10ca7c8-34de-11e8-9037-8427513e3a84.png)

And I can now [manually apply](https://stackoverflow.com/a/28569128/1445366) these changes.

[CLI] [ENHANCEMENT] [react-native-git-upgrade/cliEntry.js] - Provide instructions for upgrading React Native when 3-way merges don't apply
Closes facebook#18636

Differential Revision: D7603073

Pulled By: hramos

fbshipit-source-id: 32d387e1ee67d8b182d248c585cd33ba94808357
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. Import Started This pull request has been imported. This does not imply the PR has been approved. Missing Changelog This PR appears to be missing a changelog, or they are incorrectly formatted.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants