Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trying to use .diffSummary with binary file differences #243

Closed
mwpowellhtx opened this issue Feb 5, 2018 · 12 comments
Closed

Trying to use .diffSummary with binary file differences #243

mwpowellhtx opened this issue Feb 5, 2018 · 12 comments

Comments

@mwpowellhtx
Copy link

I have a binary file that I am trying to track using the .diffSummary. I'm not sure what is being returned to be honest, because when I try to JSON.stringify(d), I get back TypeError: Converting circular structure to JSON.

Regardless whether binary of textual, I expect something like { files: [], insertions: 0, deletions: 0 } back when there are no differences, correct?

Thanks!

@mwpowellhtx
Copy link
Author

I could be wrong but it seems like the DiffSummary.prototype.insertions and .deletions are initialized to 0, but never rolled up from .files. Am I incorrect in that assessment?

And actually, upon closer inspection of the result object, it does not contain any of the three expected properties:

differences: object
diff property: _baseDir
diff property: _runCache
diff property: ChildProcess
diff property: Buffer
diff property: _childProcess
diff property: _command
diff property: _env
diff property: _outputHandler
diff property: _silentLogging
diff property: customBinary
diff property: env
diff property: cwd
diff property: outputHandler
diff property: init
diff property: status
diff property: stashList
diff property: stash
diff property: clone
diff property: mirror
diff property: mv
diff property: checkoutLatestTag
diff property: add
diff property: commit
diff property: _getLog
diff property: pull
diff property: fetch
diff property: silent
diff property: tags
diff property: rebase
diff property: reset
diff property: revert
diff property: addTag
diff property: addAnnotatedTag
diff property: checkout
diff property: checkoutBranch
diff property: checkoutLocalBranch
diff property: deleteLocalBranch
diff property: branch
diff property: branchLocal
diff property: addConfig
diff property: raw
diff property: submoduleAdd
diff property: submoduleUpdate
diff property: submoduleInit
diff property: subModule
diff property: listRemote
diff property: addRemote
diff property: removeRemote
diff property: getRemotes
diff property: remote
diff property: mergeFromTo
diff property: merge
diff property: tag
diff property: updateServerInfo
diff property: push
diff property: pushTags
diff property: rm
diff property: rmKeepLocal
diff property: catFile
diff property: binaryCatFile
diff property: _catFile
diff property: diff
diff property: diffSummary
diff property: revparse
diff property: show
diff property: clean
diff property: exec
diff property: then
diff property: log
diff property: clearQueue
diff property: checkIgnore
diff property: checkIsRepo
diff property: _rm
diff property: _parseCheckout
diff property: _parseCheckIgnore
diff property: _run
diff property: _schedule

In fact, it does not look like a DiffSummary whatsoever.

@steveukx
Copy link
Owner

steveukx commented Feb 6, 2018

The top level stats aren't rolled up from the individual file details in the response, they're parsed from the summary detail.

That output is an instance of the simple-git itself, so it looks as though you are logging the result of calling diff (eg console.log( git.diff() )). To get to the DiffSummary you should either use a callback or promise chain:

const repo = `path/to/repo`;
const git = require('simple-git');

git(repo). diffSummary((err, summary) => { console.log(summary) });

or with promises:

const repo = `path/to/repo`;
const gitP = require('simple-git/promise');

 gitP(repo).diffSummary().then((summary) => { console.log(summary) });

@mwpowellhtx
Copy link
Author

@steveukx Ah, I see. I will give that a try. Thanks!

@mwpowellhtx
Copy link
Author

@steveukx That did the trick. I may have a follow up to that concerning the binary nature of the file(s) under difference comparison. It "should" work as long as it is just a pass through from the git diff features, but that's the next challenge for me. Thanks so much!

@steveukx
Copy link
Owner

steveukx commented Feb 6, 2018

Let me know if you have problems running against binary files - the parser used for diffSummary converts the Buffer of raw data into a UTF8 string. There is a .raw(...) method for running arbitrary commands against the git binary, but it currently doesn't allow for returning the buffer of data directly, only the UTF8 string.

@mwpowellhtx
Copy link
Author

mwpowellhtx commented Feb 6, 2018

Sure thing. So far so good; no insertions/deletions for the "same" package. But this is a false positive. When there are changes, or I expect there to be changes, Beyond Compare tells me there should be changes, but these are not reported via .insertions, .deletions, somewhat as expected. These are never rolled up, at least based on static code analysis. Even the .files is does not contain any entries.

@mwpowellhtx mwpowellhtx reopened this Feb 6, 2018
@steveukx
Copy link
Owner

steveukx commented Feb 6, 2018

Static code analysis wouldn't show the calculation of insertions or deletions - they're taken from the summary line at the end of the response to git diff, examples of which are in the tests.

If you're seeing inconsistencies with the diff summary, please run git diff --stat=4096 in a terminal window and paste its output here, along with a JSON stringified version the DiffSummary so I can create a test case.

@mwpowellhtx
Copy link
Author

@steveukx Reviewing the code, I see .insertions and .deletions being added to the .files objects, but not at a summary level.

Running diff in this manner is insufficient for binary files.

$ git diff --stat=4096
 my-package.tar.gz | Bin 3163 -> 3244 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)

It would be better IMO to run with --raw:

:100644 100644 23e48aa... 0000000... M  my-package.tar.gz

@steveukx
Copy link
Owner

steveukx commented Feb 6, 2018

The trailing summary line is parsed in the block https://github.com/steveukx/git-js/blob/master/src/responses/DiffSummary.js#L33-L41 with the status object itself being updated on https://github.com/steveukx/git-js/blob/master/src/responses/DiffSummary.js#L38.

The DiffSummary parser doesn't roll up the individual file summary values to ensure there is no error in the overall detail being reported.

@mwpowellhtx
Copy link
Author

mwpowellhtx commented Feb 6, 2018

@steveukx Let's assume for a moment that a summary line is being parsed. I'm not sure that it is, but let's assume that it is. Where does it land in the resulting summary?

Ah, I think I see it. It may try to find "insertions" or "deletions", but I'm not sure whether it does.

Considering the summary line is:

 1 file changed, 0 insertions(+), 0 deletions(-)

I assure you the files are "hexadecimal" different, according to Beyond Compare. Whereas "line based" insertions or deletions are not necessarily detected in this way.

@mwpowellhtx
Copy link
Author

@steveukx Here as well as here.

@mwpowellhtx
Copy link
Author

@steveukx Thanks for pursuing this issue. Any ideas as to a publish time frame?

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

No branches or pull requests

2 participants