[Webkit-unassigned] [Bug 9299] Teach svn-create-patch and friends to work with binary files

[bugzilla-daemon at opendarwin.org]

http://bugzilla.opendarwin.org/show_bug.cgi?id=9299





------- Comment #5 from darin at apple.com  2006-06-04 10:51 PDT -------
I think this is a fine start and it's good to just land what you have here,
even though it could use some refinement.

Compressing the binary data is a great idea. We should not be shy about adding
some additional lines of text to make the format easier to understand. I don't
think we should use a "MIME multipart" approach. Instead we should use
something that's human readable and consistent in style with standard "svn di"
output.

I think that we should try to be completely unambiguous in the format. The
base64 data for the file (before and after in the case of a modification,
before for deletion, after for addition) should be preceded by something that
says in plain English whether it's a file being added, deleted, or modified.

In fact, I think we should omit "Cannot display: file marked as a binary
type.", because it's misleading. I like the economy of staying as close as
possible to the original patch, but I don't want anything actively misleading
in the patch. So the text should say something like "Binary file:
Base64-encoded data follows." instead of "Cannot display." But some variation
on that which is clear for the 3 cases of addition, deletion, and modification.

Putting all the binary changes at the bottom is a great idea -- makes it easier
to review the patch. Since "svn diff" gives changes in random order, it would
be good to sort them in the generated patch. I suggest sorting filenames the
same way that prepare-ChangeLog does, but using the "binary vs. text" as the
primary key.

I'd like to see the apply and unapply scripts check the binary against the data
in the patch and do something safe when the file doesn't match. I'd suggest a
rule that says "complain and do nothing for the entire patch if binary files
are present but do not match". Probably a pass before applying the patch that
checks that everything applies successfully. For diff patches we could use
--dry-run to check that they apply, and for binary patches just compare what's
on disk with what's in the patch.

As a special case, we could allow entirely-missing files for a patch that
modifies a file. That way, you can get a patch to apply just by removing some
files explicitly.


-- 
Configure bugmail: http://bugzilla.opendarwin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.