Binary Files

[brianteeman]

The patch tester (i'm assuming uses diff) which ignores binary files. We just wasted 3 hours debugging an issue with a pull request that had ttf and woff fonts which are binary and were not getting applied.

Either the patchtester needs a way to handle binary files OR at a minimum give a notice that filex was not copied

[mbabker]

In terms of actually applying a patch, it actually just takes a straight copy of files from the branch on the repo where the PR originated; the diff is parsed to figure out what files are changed (with a patch applied, you should have data in administrator/components/com_patchtester/backups).

What I assume is the issue just reading the code is these lines which is retrieving the body of the response from GitHub.

[brianteeman]

You know me - I dont Know the code I can just report the issue

[mbabker]

Gotta leave notes somewhere ;-)

[wilsonge]

@brianteeman I think this part of a bigger issue with binaries though. I know JM had loads of problems with the binaries in TinyMCE when he was trying to test my upgrade.

@mbabker Replacing files is also proving an issue in some cases, where people don't keep their branches 100% upto date - because lines you haven't even changed may be affected by changes in master to other files. For example on an issue yesterday a PHP was being thrown due to a line that wasn't being changed by the PR - and it caused all kinds of confusion!

[brianteeman]

It wouldnt have been an issue if he was using git natively but good luck on convincing him on that ;)

[mbabker]

Replacing files was how it was implemented by developers past (I think this code predates the JFilesystemPatcher class). IMO, it's probably a better move to do it that way than trying to actually patch files (you're definitely more prone to a fail condition if you're patching lines versus backing up and replacing files). Something to be thought about for the future though.

If you are pulling down git branches, transferring binaries works just fine. If you're trying to apply the diff created by a PR, the binaries don't come down. It isn't anything he was doing to be honest, I've run into issues with binaries also and I don't do anything git related outside a command line interface (AKA the way it should be done).

[mbabker]

I've added a warning if binary files are included in a patch. I can't think of a way to correctly handle these so for now it's just completely unsupported.