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

Use Bazel's builtin patch support. #41393

Merged
merged 1 commit into from
Jul 17, 2020

Conversation

benjaminp
Copy link
Contributor

@benjaminp benjaminp commented Jul 14, 2020

This removes a dependency on the system having a patch executable.

@google-ml-butler google-ml-butler bot added the size:S CL Change Size: Small label Jul 14, 2020
@benjaminp benjaminp force-pushed the builtin-patch branch 2 times, most recently from 7faf48b to 766f4e1 Compare July 15, 2020 00:00
@gbaned gbaned self-assigned this Jul 15, 2020
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Jul 15, 2020
@mihaimaruseac
Copy link
Collaborator

We had issues in the past where patching wouldn't work on Windows/Linux. We have tried git patch, git apply and patch.

Adding gunan@ to verify if this is likely to solve all issues form above and if the diffs are also ok, as I was not expecting them to be generated given PR title/description

@mihaimaruseac mihaimaruseac requested a review from gunan July 15, 2020 23:38
@benjaminp
Copy link
Contributor Author

I had to regenerate some of the patch files because the builtin patch is stricter than patch(1). (Basically, I just did patch -p1 old.patch and diff -ru.)

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Jul 16, 2020
mihaimaruseac
mihaimaruseac previously approved these changes Jul 16, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jul 16, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 16, 2020
This removes a dependency on the system having a patch executable.
PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Jul 16, 2020
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Jul 16, 2020
@gunan gunan added the kokoro:force-run Tests on submitted change label Jul 16, 2020
@gunan
Copy link
Contributor

gunan commented Jul 16, 2020

As long as presubmits pass, I think we can accept this change.

@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 16, 2020
@benjaminp
Copy link
Contributor Author

Only "Windows Bazel GPU " appears to have failed, but it also is broken on master.

@gunan gunan added the kokoro:force-run Tests on submitted change label Jul 16, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 16, 2020
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Jul 16, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jul 16, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 16, 2020
@mihaimaruseac
Copy link
Collaborator

Will import this manually, something breaks

@tensorflow-copybara tensorflow-copybara merged commit 5555c36 into tensorflow:master Jul 17, 2020
PR Queue automation moved this from Approved by Reviewer to Merged Jul 17, 2020
@benjaminp benjaminp deleted the builtin-patch branch July 17, 2020 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes ready to pull PR ready for merge process size:S CL Change Size: Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

7 participants