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

Upgrade python-patch to the latest upstream release #2284

Merged
merged 1 commit into from May 14, 2019

Conversation

@fmarier
Copy link
Member

fmarier commented Apr 19, 2019

This fixes brave/brave-browser#3932.

The version we were using was an svn import of code.google.com to GitHub but both code.google.com and the importer are now defunct:

http://piotr.gabryjeluk.pl/blog:closing-svn2github

The upstream developer, according to the PyPI project page (https://pypi.org/project/patch/), has moved to a dedicated repo on GitHub:

https://github.com/techtonik/python-patch

I picked the latest upstream release (1.16):

https://github.com/techtonik/python-patch/releases/tag/1.16
techtonik/python-patch@2148d54

as the commit to pin our dependency to since the only commits on master after that are related to a part of the package that we don't use and that has been moved to another package:

techtonik/python-patch@76f0205

Submitter Checklist:

Test Plan:

To test that the new version gives me the same output with our current patches, I did the following under the old version:

$ cd src
$ git reset --hard
HEAD is now at 97cde3a81abd Publish DEPS for 74.0.3729.91
$ cd ..
$ npm run apply_patches
...
$ npm run update_patches

which gave me the following diff in src/brave: https://gist.github.com/fmarier/7b127e6d2d6b2b6076f98d81202defe2

Then I applied and updated the patches again:

$ npm run apply_patches
...
$ npm run update_patches

and got the following diff: https://gist.github.com/fmarier/2899dfd116a02a17fa31a1131825f89c

which is in line with this known bug (also reported upstream).

I ran gclient sync and then tested applying the patches again.

I got the same results as above in the first and second runs.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.
@fmarier fmarier requested a review from bbondy Apr 19, 2019
@fmarier fmarier self-assigned this Apr 19, 2019
@fmarier fmarier force-pushed the francois-3932-upgrade-python-patch branch from aa21265 to 1054a86 Apr 26, 2019
This fixes brave/brave-browser#3932.

The version we were using was an svn import of `code.google.com` to
GitHub but both `code.google.com` and the importer are now defunct:

http://piotr.gabryjeluk.pl/blog:closing-svn2github

The upstream developer, according to the PyPI project page
(https://pypi.org/project/patch/), has moved to a dedicated repo
on GitHub:

  https://github.com/techtonik/python-patch

I picked the latest upstream release (1.16):

  https://github.com/techtonik/python-patch/releases/tag/1.16
  techtonik/python-patch@2148d54

as the commit to pin our dependency to since the only commits on master
after that are related to a part of the package that we don't use and that
has been moved to another package:

  techtonik/python-patch@76f0205
@fmarier fmarier force-pushed the francois-3932-upgrade-python-patch branch from 1054a86 to 7ea4713 May 14, 2019
@bbondy
bbondy approved these changes May 14, 2019
@fmarier fmarier merged commit f9c2dec into master May 14, 2019
2 checks passed
2 checks passed
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@fmarier fmarier deleted the francois-3932-upgrade-python-patch branch May 14, 2019
@fmarier fmarier added this to the 0.67.x - Nightly milestone May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.