{"thread":{"id":"13388","subject":"Binary files in format-patch","startedAt":"2008-05-05T22:55:06Z","lastAt":"2008-05-08T20:25:17Z","messageCount":6,"participants":["Caio Marcelo","Shawn O. Pearce","Robin Rosenberg"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"76155","messageId":"d280d7f10805051555x67bd7e87o999a2c9e19663b92@mail.gmail.com","threadId":"13388","inReplyTo":null,"subject":"Binary files in format-patch","fromName":"Caio Marcelo","fromEmail":"cmarcelo@gmail.com","sentAt":"2008-05-05T22:55:06Z","receivedAt":"2008-05-05T22:55:06Z","isPatch":false,"sender":{"key":"cmarcelo@gmail.com","avatar":null},"body":"Hello,\n\nI'm using \"git format-patch\" to generate messages for a code review\nmailing list. It work fine except when we have binary files involved.\nTheir contents are not relevant for us, and doesn't help much in a\nmailing list. Taking a peek at the code I've found out this:\n\nIn commit e47f306d4bf964def1a0b29e8f7cea419471dffd (short name: \"git\nformat-patch: make --binary on by default\"), we add a new restriction\non the possible options to format-patch: if you don't specify --text,\nit enables --binary. But looking at today's code, we have a path that\nis never taken for format-patch, in function builtin_diff (diff.c), at\nlines 1423 to 1433. The fprintf doesn't ever happen, because if it's\nTEXT, it dumps the contents verbatim, if it's BINARY, it encodes (to\nsome baseXX) the files/diff.\n\nWouldn't be nice to allow this code path to happen via some\n--omit-binary / --no-binary option to be checked in cmd_format_patch?\n(I could provide a patch for this, if you think it's a good idea).\n\nOr should I try to solve my problem by other means (post-processing\nthe patches, building my own with git-diff)?\n\n\nCheers,\n  Caio Marcelo\n"},{"id":"76156","messageId":"20080505230153.GL29038@spearce.org","threadId":"13388","inReplyTo":"d280d7f10805051555x67bd7e87o999a2c9e19663b92@mail.gmail.com","subject":"Re: Binary files in format-patch","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-05-05T23:01:53Z","receivedAt":"2008-05-05T23:01:53Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Caio Marcelo <cmarcelo@gmail.com> wrote:\n> I'm using \"git format-patch\" to generate messages for a code review\n> mailing list. It work fine except when we have binary files involved.\n> Their contents are not relevant for us, and doesn't help much in a\n> mailing list. Taking a peek at the code I've found out this:\n\nWhy then are they committed in Git and being modified?  If the\nfiles aren't relevant, why are they tracked?\n \n-- \nShawn.\n"},{"id":"76158","messageId":"d280d7f10805051610u3b020dafxb9e832ecdf14c68c@mail.gmail.com","threadId":"13388","inReplyTo":"20080505230153.GL29038@spearce.org","subject":"Re: Binary files in format-patch","fromName":"Caio Marcelo","fromEmail":"cmarcelo@gmail.com","sentAt":"2008-05-05T23:10:59Z","receivedAt":"2008-05-05T23:10:59Z","isPatch":false,"sender":{"key":"cmarcelo@gmail.com","avatar":null},"body":">  > Their contents are not relevant for us, and doesn't help much in a\n>  > mailing list. Taking a peek at the code I've found out this:\n>\n>  Why then are they committed in Git and being modified?  If the\n>  files aren't relevant, why are they tracked?\n\nSorry, I wasn't very clear there. The binary content is mostly icons,\nthat get added and updated now and then, so outside of the scope of\nour code review mailing list -- but relevant to the project itself.\n(The patches themselves are accessible by other means other than the\nML too, so reviewers can pull and get proper versions).\n\nConsidering that, any of the proposed solutions make more sense now? :-)\n\n\nCheers,\n  Caio Marcelo\n"},{"id":"76190","messageId":"200805061119.14792.robin.rosenberg.lists@dewire.com","threadId":"13388","inReplyTo":"d280d7f10805051555x67bd7e87o999a2c9e19663b92@mail.gmail.com","subject":"Re: Binary files in format-patch","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2008-05-06T09:19:14Z","receivedAt":"2008-05-06T09:19:14Z","isPatch":false,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"tisdagen den 6 maj 2008 00.55.06 skrev Caio Marcelo:\n> Hello,\n>\n> I'm using \"git format-patch\" to generate messages for a code review\n> mailing list. It work fine except when we have binary files involved.\n> Their contents are not relevant for us, and doesn't help much in a\n> mailing list. Taking a peek at the code I've found out this:\n\n> In commit e47f306d4bf964def1a0b29e8f7cea419471dffd (short name: \"git\n> format-patch: make --binary on by default\"), we add a new restriction\n> on the possible options to format-patch: if you don't specify --text,\n> it enables --binary. But looking at today's code, we have a path that\n> is never taken for format-patch, in function builtin_diff (diff.c), at\n> lines 1423 to 1433. The fprintf doesn't ever happen, because if it's\n> TEXT, it dumps the contents verbatim, if it's BINARY, it encodes (to\n> some baseXX) the files/diff.\n>\n> Wouldn't be nice to allow this code path to happen via some\n> --omit-binary / --no-binary option to be checked in cmd_format_patch?\n> (I could provide a patch for this, if you think it's a good idea).\n\nSounds reasonable.  I'd say --no-binary is the right one here.\n\n-- robin\n"},{"id":"76400","messageId":"d280d7f10805081321q7a09b61ke06a1c945650d880@mail.gmail.com","threadId":"13388","inReplyTo":"200805061119.14792.robin.rosenberg.lists@dewire.com","subject":"Re: Binary files in format-patch","fromName":"Caio Marcelo","fromEmail":"cmarcelo@gmail.com","sentAt":"2008-05-08T20:21:53Z","receivedAt":"2008-05-08T20:21:53Z","isPatch":false,"sender":{"key":"cmarcelo@gmail.com","avatar":null},"body":">  > Wouldn't be nice to allow this code path to happen via some\n>  > --omit-binary / --no-binary option to be checked in cmd_format_patch?\n>  > (I could provide a patch for this, if you think it's a good idea).\n>\n>  Sounds reasonable.  I'd say --no-binary is the right one here.\n\nCommited a patch couple a days ago. Could anyone take a look? I've\ntried to reduce the changes to a minimum.\n\n\nCheers,\n  Caio Marcelo\n"},{"id":"76402","messageId":"d280d7f10805081325q505a8397y3fc93ae23d5ed63d@mail.gmail.com","threadId":"13388","inReplyTo":"d280d7f10805081321q7a09b61ke06a1c945650d880@mail.gmail.com","subject":"Re: Binary files in format-patch","fromName":"Caio Marcelo","fromEmail":"cmarcelo@gmail.com","sentAt":"2008-05-08T20:25:17Z","receivedAt":"2008-05-08T20:25:17Z","isPatch":false,"sender":{"key":"cmarcelo@gmail.com","avatar":null},"body":">  Commited a patch couple a days ago. Could anyone take a look? I've\n\nOps.. I didn't only commited but also sent to this mailing list ;-)\n\n\nCheers,\n  Caio Marcelo\n"}]}