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
PHP autosynth failing with error: cannot apply binary patch to 'Dlp/metadata/V2/Dlp.php' without full index line
#918
Comments
Git thinks that some generated PHP files are binary. @vam-google, the timing of this issue exactly matches the timing of googleapis/gapic-generator#3334. Do you think they're related? |
Speculative fix for googleapis/synthtool#918
Pass the --binary flag to git diff so it can diff files no matter if they're binary or text. Fixes googleapis#918
#928 will fix autosynth so it can handle the new binary output, but it leaves a question for @dwsupplee and @bshaffer: Do we actually want to generate binary php files? The results are pull requests like this that are impossible to visually diff. @vam-google, I see that the generator used to hex-encode the binary payload like this but now injects raw binary into the php files like this. Was that a recent protobuf compiler change? Was it intentional? |
@SurferJeffAtGoogle, it does look intentional based on: protocolbuffers/protobuf#8006 I just tested the changes out and they do work, so I think we're okay to move forward with this for now. To your point, I don't love not being able to read the diff anymore and am interested in alternatives to this approach at some point in the future. Thanks for resolving this for us! |
Thanks David. I'll move forward with my fix in synthtool.
…On Thu, Jan 28, 2021 at 9:09 AM David Supplee ***@***.***> wrote:
@SurferJeffAtGoogle <https://github.com/SurferJeffAtGoogle>, it does look
intentional based on: protocolbuffers/protobuf#8006
<protocolbuffers/protobuf#8006>
I just tested the changes out and they do work, so I think we're okay to
move forward with this for now. To your point, I don't love not being able
to read the diff anymore and am interested in alternatives to this approach
at some point in the future.
Thanks for resolving this for us!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#918 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACZ3TYX6BISRDIEPPSUBDWDS4GK4HANCNFSM4WSBBI3Q>
.
|
* fix: git thought some php files were binary and refused to diff Pass the --binary flag to git diff so it can diff files no matter if they're binary or text. Fixes #918 * fix: add the --binary flag to one more 'git diff' statement About 1/3 of the APIs are still failing due to binary files. * fix: don't attempt to parse the result of git diff as text * fix: update regex code to work with binary * fix: yet another binary-utf8 issue
https://fusion.corp.google.com/runanalysis/test/prod:cloud-devrel%2Fclient-libraries%2Fautosynth%2Fphp/prod:cloud-devrel%2Fclient-libraries%2Fautosynth%2Fphp/KOKORO/df06a580-bede-479d-aeeb-550c077e48a0/1611572173159/build%20%23936/Targets?target=logs%2FDlp&drilldownTab=logs
The text was updated successfully, but these errors were encountered: