2310: git diff is invalid when using --parent option
- NotABug
- Review Board
| fanny.*******@gmai***** (Google Code) (Is this you? Claim this profile.) | |
| Sept. 28, 2011 |
What version are you running? 1.6.1 What's the URL of the page containing the problem? http://localhost/r/4732/diff/#index_header What steps will reproduce the problem? 1. Git commit a change: $ git commit foo.txt 2. Make a local change in your working tree: $ vi bar.txt 3. git status should show modified: bar.txt and git show HEAD should show the modifications of the foo.txt file. 4. post-review --parent=HEAD^ What is the expected output? What do you see instead? post-review should upload the diff of the HEAD commit, not the diff of the HEAD + the modifications of bar.txt as it uploads currently. What operating system are you using? What browser? Linux, Opera 10.x Please provide any additional information below. Note that this worked well with 1.5.1 so it seems this is a regression.
Don't know if this is the good fix, bu the one below seems to "make things work"™:
diff --git a/support/scripts/post-review b/support/scripts/post-review
index 8d556e8..9401374 100755
--- a/support/scripts/post-review
+++ b/support/scripts/post-review
@@ -2947,10 +2947,7 @@ class GitClient(SCMClient):
"""
Performs a diff on a particular branch range.
"""
- if commit:
- rev_range = "%s..%s" % (ancestor, commit)
- else:
- rev_range = ancestor
+ rev_range = "%s..%s" % (ancestor, commit)
if self.type == "svn":
diff_lines = execute([self.git, "diff", "--no-color", "--no-prefix",
Maybe it's a feature actually. I didn't really check the git tree before "fixing" it. It was part of a fix to support bare repositories and more revision range in post-review (d7ec4f48 in master). So, should the bug Fanny is encountering be called a feature? Anyway, it seems that what she used to do seems to work fine using "--revision-range=HEAD^:HEAD".
Yeah, a single revision actually wasn't supported before. It was "working" but that wasn't the intended behavior. What we have now is correct. Might take some getting used to though.
-
+ NotABug -
+ Component-RBTools