PHP autosynth failing with `error: cannot apply binary patch to 'Dlp/metadata/V2/Dlp.php' without full index line`

[SurferJeffAtGoogle]

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

[SurferJeffAtGoogle]

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?

[SurferJeffAtGoogle]

#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?

[dwsupplee]

@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!

[SurferJeffAtGoogle]

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> .