2494: Merge conflict resolutions cannot be reviewed (`git diff-tree --cc` diffs don't parse)

Ellio******@gmai***** (Google Code) (Is this you? Claim this profile.)
Jan. 31, 2014
2896
What version are you running?
bleeding edge, c848031b769cf84b8f9fe42430210459735fed11

What's the URL of the page containing the problem?
rbtools post-review

What steps will reproduce the problem?
1. generate a diff of a merge commit with --cc. {you can just do git show COMMIT --format=format:'' > out.diff}
2. post-review it

What is the expected output?
the diff to be posted to reviewboard

What do you see instead?
git@source:~/repositories/repo.git$ post-review \
  --publish \
  --debug \
  --username="git" \
  --password="git" \
  --diff-filename="/tmp/diff_eaca7ff" \
  --server="https://site.com/reviewboard" \
  --branch="master" \
  --description="Merge branch 'feature' into master\nConflicts:\nsrc/file" \
  --summary="eaca7ff (JAVA) Merge branch 'point/5.0.26' into develop" \
  | tee -a post-review.log

Error uploading diff

The generated diff file was empty. This usually means no files were
modified in this change.

Try running with --output-diff and --debug for more information.

>>> RBTools 0.4 alpha 0 (dev)
>>> Home = /opt/git
>>> HTTP GETting api/
>>> HTTP GETting https://site.com/reviewboard/api/info/
>>> Using the new web API
>>> Attempting to create review request on /opt/git/repositories/repo.git for None
>>> HTTP POSTing to https://site.com/reviewboard/api/review-requests/: {'repository': '/opt/git/repositories/repo.git'}
>>> Review request created
>>> Attempting to set field 'summary' to 'eaca7ff (JAVA) Merge branch 'point/5.0.26' into develop'
>>> HTTP PUTting to https://site.com/reviewboard/api/review-requests/6953/draft/: {'summary': "eaca7ff (JAVA) Merge branch 'point/5.0.26' into develop"}
>>> Attempting to set field 'branch' to 'master' for review request '6953'
>>> HTTP PUTting to https://site.com/reviewboard/api/review-requests/6953/draft/: {'branch': 'master'}
>>> Attempting to set field 'description' to 'Merge branch 'feature' into master\nConflicts:\nsrc/file'
>>> HTTP PUTting to https://site.com/reviewboard/api/review-requests/6953/draft/: {'description': "Merge branch 'feature' into master\nConflicts:\nsrc/file"}
>>> Uploading diff, size: 5873
>>> HTTP POSTing to https://site.com/reviewboard/api/review-requests/6953/diffs/: {}
>>> Got API Error 105 (HTTP code 400): One or more fields had errors
>>> Error data: {u'fields': {u'path': [u'The diff file is empty']}, u'stat': u'fail', u'err': {u'msg': u'One or more fields had errors', u'code': 105}}
Your review request still exists, but the diff is not attached.

What operating system are you using? What browser?
centos/firefox

more info:
scrubbed a few things. note that the diff size is 5873 bytes, and that this works when the diff is any git diff aside from diff --cc.
david
#1 david
I can't seem to produce a diff with any content given the instructions you provided.

What kind of revision information is in the diff?
  • +NeedInfo
#2 Ellio******@gmai***** (Google Code) (Is this you? Claim this profile.)
merge conflict resolution.
#3 Ellio******@gmai***** (Google Code) (Is this you? Claim this profile.)
e.g.:
wolke:~$ mkdir /tmp/a; cd /tmp/a
wolke:/tmp/a$ git init
Initialized empty Git repository in /tmp/a/.git/
wolke:/tmp/a$ touch a
wolke:/tmp/a$ git add a
wolke:/tmp/a$ git commit -m 'initial commit'
[master (root-commit) 4b17940] initial commit
 0 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 a
wolke:/tmp/a$ git branch feature
wolke:/tmp/a$ echo stable-change >> a
wolke:/tmp/a$ git add a
wolke:/tmp/a$ git ci -m 'added a stable commit'
[master fc720f4] added a stable commit
 1 files changed, 1 insertions(+), 0 deletions(-)
wolke:/tmp/a$ git checkout feature
Switched to branch 'feature'
wolke:/tmp/a$ echo unstable-change >> a
wolke:/tmp/a$ git add a
wolke:/tmp/a$ git ci -m 'added an unstable commit'
[feature 3332df1] added an unstable commit
 1 files changed, 1 insertions(+), 0 deletions(-)
wolke:/tmp/a$ git checkout master
Switched to branch 'master'
wolke:/tmp/a$ git merge feature
Auto-merging a
CONFLICT (content): Merge conflict in a
Automatic merge failed; fix conflicts and then commit the result.
wolke:/tmp/a$ echo resolution-of-stable-and-unstable-change > a
wolke:/tmp/a$ git add a
wolke:/tmp/a$ git commit -m 'Merged branch 'feature', fixed conflict on a'
[master 799525f] Merge branch 'feature'
wolke:/tmp/a$ git show HEAD --format=format:''

diff --cc a
index eb0b6a2,caf0188..13303c2
--- a/a
+++ b/a
@@@ -1,1 -1,1 +1,1 @@@
- stable change
 -unstable-change
++union-of-stable-and-unstable-change
david
#4 david
  • -NeedInfo
    +New
david
#5 david
  • +Component-SCMTools
#6 Ellio******@gmai***** (Google Code) (Is this you? Claim this profile.)
guys this is a big deal. merge conflicts exist, and their resolutions involve actual code that needs peer review just as much as any other code.

merge conflict resolutions CANNOT BE REVIEWED WITH REVIEW BOARD.

their are many workable solutions. you could show the changes against the new-parent, or even the old-parent. better still would be to put the (diff of the old and new) on the left and the actual-code-changes on the right. of course, a visual 3-way diff would be best, but probably isnt feasible.

even just popping the diff file in as a 'new file' for review would be enough to get eyeballs on it.
david
#7 david
  • +Merge conflict resolutions cannot be reviewed (`git diff-tree --cc` diffs don't parse)
david
#9 david
It's not hard to make review board parse these diffs, but unfortunately, 'patch' doesn't know what to do with them and the git patch tools (apply, am) don't allow us to isolate individual orig files the way patch does.

I suggest you generate your merge conflict diffs by comparing your new commit against one of its parents directly.
  • -New
    +WontFix