1967: post-review 0.3 unexpected diff behaviour and git repository fatal error

shaheen********@quantera********* (Google Code) (Is this you? Claim this profile.)
chipx86
chipx86
Feb. 1, 2011
What version are you running?
Review Board 1.5.2 on the server
RBTools 0.3 on the Windows Client

What's the URL of the page containing the problem?
http://<private-domain>/api/review-requests/x/diffs (where x is a number)

What steps will reproduce the problem?
Environment
-Python, Setuptools were installed. 
-Used easy_install to install RBTools.
-I'm using msysgit Git with unix tools installed.
-I set the reviewboard.url configuration key in Git.
-I also created a .reviewboardrc file under 'Home' in C:\Users\username\AppData\Roaming with the same value for the REVIEWBOARD_URL parameter.
-I read in the release notes that RBTools 0.3 uses the Git diff, so I did not install GNU's DiffUtils.

1. I'm using Git, and I had not yet pulled down the latest code from master, but had made my own changes since the last pull.
2. I invoked post-review and was greeted with the "There don't seem to be any diffs!" message.
3. I re-ran post-review several more times, including with the debug option, but I didn't see anything that stood out.
4. I executed git diff which correctly output the changes I had made since my last pull.
5. I decided to pull from master and re-ran post-review. Surprisingly, post-review got further along, but not all the way. The debug option revealed that it created a review request but when uploading diff of size 9680, an error occured;
GOT API Error 105 (HTTP code 400): One or more fields had errors
Error data: {u'fields': {u'path': [u"fatal: Not a git repository" 'None'\n"]}, u'stat': u'fail', u'err' : u'One or more fields had errors', u'code': 105}}

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.

Your review request still exists, but the diff is not attached.


6. When I invoke the diff command (git diff --no-color --full-index --no-ext-diff 3b1d3ee1c56b62fc01be53d6f74e11fb1b2fd0e4..refs/heads/master) as reported by post-review with the debug option, I get a different output than I expect. I get the diff between the latest two pushed commits. None of my local changes are shown in the diff.

7. As a last effort, I ran git diff > my_diff_file and used that file as a parameter to --diff-filename=my_diff_file. I can tell a different diff than in 5 was posted to the server because the size was reported to be 845. However, I received the same error.

What is the expected output? What do you see instead?

So, there are a few things to point out here. 
-Not a git repository in the error is alarming. Is it referring to incorrect configuration of the Git repository on the server side? I am unfamiliar with the flow of events. If post-review is generating a diff on the client-side, what is the Review Board server attempting to do with Git repository.
-How come the git diff command run by post-review is not diff'ing MY local changes...

What operating system are you using? What browser?

Windows 7
Firefox

Please provide any additional information below.
#1 shaheen********@quantera********* (Google Code) (Is this you? Claim this profile.)
In the Review Board administration portal, I have a repository with the following fields filled out;
-name
-show this repository
-hosting service: Custom
-repo type: Git
-path: git@<remotehost>:username/project/repo.git

and that's it.
chipx86
#2 chipx86
Review Board has to talk to a repository in order to show the side-by-side diffs. It doesn't display *only* the uploaded diff, but also all the context around it (and interdiffs, optionally), requiring the full files. So it must talk to the repository.

Git doesn't have a concept of remote file access for a repository. You need a full clone. So, we offer two ways to configure a Git repository. Either you have a local clone and Path: points to that on the filesystem, or you have Path point to a remote repository (like you're doing) and you set the Raw URL mask field to point to a special path on a Gitweb, cgit, etc. instance.

See the documentation for how to configure this.

This is why you're hitting the problems. It's a configuration issue.

As for generating diffs... when it complains that it's empty, is this when the code is in the index and not in a commit? We only do a diff against commits since the closest remote by default.
  • +NeedInfo
#3 shaheen********@quantera********* (Google Code) (Is this you? Claim this profile.)
Thank you for your reply. I realize the oversight now with regards to only providing a path and not a mirror (and need a local clone of the repository).

Does Review Board keep the local clone in sync with the master, or do I need to manually/automate pull downs?

I tried the diff when my changes were in the 'working copy' or uncommitted. But I also tried the diff after a commit (but not a push) as I understood Review Board was conceived with intent of using it pre-commit, and I had the same problem.

I will create a blog post with regards to the points you mentioned and some other helpful hints I compiled along the way. I found it difficult to find resources on using post-review in comparison to other SE tools I have had to setup in the past. Your help is definitely appreciated. I read that you managed to work on this FT at some point, is that still the case?

After making the configuration changes on the server-side, I was able to submit a review of uncommited changes by providing a diff file (which I produced using git diff --no-color --full-index --no-ext-diff). I'm not currently utilizing any branching techniques, I simply modify my clone of master, so I'm wondering if that has anything to do with my remaining issue ; not being able to post a review request without manually overriding the diff created by post-review. For example, with SVN, you can simply run post-review and all uncommitted files in the tree are incorporated in the diff. Forgive me I'm missing the obvious, I'm new to Git.
chipx86
#4 chipx86
Review Board doesn't manage your clone at all. What some people do is have a post-commit hook somewhere in the upstream repository that triggers an update on the Review Board server, or have the RB server mount the other clone remotely.

We tend to point people toward the cgit/gitweb/etc method, as that's more reliable, and has the added benefit of having a repository browser set up, which can be useful.

Review Board was designed for pre-commit, but, more specifically, pre-commit on repositories like Subversion and Perforce. It's a bit different with Git. Really, it's more "changes that aren't on the upstream repository" rather than "changes not committed somewhere." When using Git, what we do is commit the change to a branch, and then run post-review. It will post the commits between the remote and HEAD. (If you pass --guess-summary and --guess-description, it will also provide some defaults for your review request).

However, if your change is on master 9or some other branch tracking a remote branch), it will fail, because it's trying to go up the chain until it finds something on a branch tracking a remote. In the case of commits on master, it will find master itself, decide there's nothing to diff, and fail. Generally, it's advisable for Git not to commit to master unless you intend for that to be pushed immediately. Changes not immediately being pushed upstream should nearly always start on a topic branch.

If there are specific things you'd like our docs to cover, let me know. You can file a bug calling out those areas. I'll try to get to it the next time I'm working on the docs.

I'm assuming FT means full time? I've never been full-time on this. All the Review Board work I do is completely in spare time, outside of my day job. Despite misconceptions, Review Board was never a VMware product, and I never worked on it as part of my day job. We do try to get as much done as possible, though. I certainly do spend a good number of hours per day on it.
  • -NeedInfo
    +SetupIssue
  • -Type-Defect
    +Type-Support
  • +chipx86