Read binary patches (with no binary data) by ethomson · Pull Request #3923 · libgit2/libgit2 · GitHub
Skip to content

Read binary patches (with no binary data)#3923

Merged
ethomson merged 3 commits into
masterfrom
ethomson/diff-read-empty-binary
Sep 6, 2016
Merged

Read binary patches (with no binary data)#3923
ethomson merged 3 commits into
masterfrom
ethomson/diff-read-empty-binary

Conversation

@ethomson

@ethomson ethomson commented Sep 2, 2016

Copy link
Copy Markdown
Member

When parsing a patch file, when we encounter a diff that was produced without --binary, we should include a binary patch that contains no data. This type of patch node is special, and cannot be applied, but it allows us to round-trip a patch file accurately.

git_diff_binary objects now indicate whether they contain data or not. They "contain data" if they were produced by us when SHOW_BINARY is set or when they are parsed from a patch file produced by git with --binary set. They do not "contain data" when they are parsed from a patch file that simply says Binary files ... differ.

Edward Thomson added 2 commits September 2, 2016 11:22
Instead of skipping printing a binary diff when there is no data, skip
printing when we have a status of `UNMODIFIED`.  This is more in-line
with our internal data model and allows us to expand the notion of
binary data.

In the future, there may have no data because the files were unmodified
(there was no data to produce) or it may have no data because there was
no data given to us in a patch.  We want to treat these cases
separately.
Comment thread src/patch_parse.c Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this distinction really required? To the best of my knowledge, most compilers convert strlen("CONST_STRING") to a compile-time constant anyway.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that's reasonable. I think that any compiler that anybody is likely to use will convert strlen("const") to a constant and for the person using MSVC 6.0, this isn't likely to be the slow part of reading patches.

When creating and printing diffs, deal with binary deltas that have
binary data specially, versus diffs that have a binary file but lack the
actual binary data.
@ethomson ethomson force-pushed the ethomson/diff-read-empty-binary branch from 3572768 to adedac5 Compare September 5, 2016 17:27
@ethomson

ethomson commented Sep 5, 2016

Copy link
Copy Markdown
Member Author

Thanks for the careful eyes, @pks-t !

@pks-t

pks-t commented Sep 6, 2016

Copy link
Copy Markdown
Member

@ethomson ethomson merged commit 9ad07fc into master Sep 6, 2016
@ethomson ethomson deleted the ethomson/diff-read-empty-binary branch January 13, 2017 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants