search

rombert / ereviewboard

A mylyn-based Eclipse integration for Review Board
46 stars 31 forks source link

Problem viewing diff of a review on GIT #94

Closed pghilardi closed 12 years ago

pghilardi commented 12 years ago

Hey,

I'm having problems to view a diff of a review on GIT using eReviewBoard. I can view the diff of a new file (a file that didn't exists on previous revision), but cannot view diff of modified files. This is the error message:

"Compare Failed: Invalid Id: eb14816", where the last number changes accordingly to review.

I don't know what is happening, maybe my repository is misconfigured or something like that.

Thanks, Pedro

rombert commented 12 years ago

Can you attach a stack trace from the Eclipse error log?

Hey,

I'm having problems to view a diff of a review on GIT using eReviewBoard. I can view the diff of a new file (a file that didn't exists on previous revision), but cannot view diff of modified files. This is the error message:

"Compare Failed: Invalid Id: eb14816", where the last number changes accordingly to review.

I don't know what is happening, maybe my repository is misconfigured or something like that.

Thanks, Pedro

Reply to this email directly or view it on GitHub: https://github.com/rombert/ereviewboard/issues/94

Sent from my (old) computer

pghilardi commented 12 years ago

Indeed, Eclipse is showing a error message, not a exception, so no message is being show on "Error Log" neither on eclipse logs available at .metadata. I think that the code is handling this "invalid id" condition and showing the message dialog to user.

rombert commented 12 years ago

I'll have a look at the code and add some debugging information. I think this error is reported by the git repo .

pghilardi commented 12 years ago

Thanks ;)

Here a print screen of the error dialog: http://i.imgur.com/XWEQ7.png

rombert commented 12 years ago

I am not able to reproduce this problem. It seems that the jgit layer receives an invalid ObjectId for the change. Can you tell me how you've generated / uploaded the diff and which kind of Git setup do you have?

pghilardi commented 12 years ago

I'm using EGit and the diff was uploaded using ReviewBoard (directly). Using Mylyn with mantis connector and git connector I can view the diff between revisions on Mylyn task viewer. How are you doing the diff on eReviewBoard tool?

rombert commented 12 years ago

I'm not sure why this happens, so I've added slightly better error handling. It seems that the patch contains an ObjectId which is not valid . Either that, or I'm missing something - I've just briefly looked at the JGit APIs. Can you try build #91 or newer from https://ereviewboard.ci.cloudbees.com/?

My only comment is that I created the diff using git format-patch. Did you use EGit to create the patch?

pghilardi commented 12 years ago

I have tried to install build #91 by copying manually .jars from /target folder on jenkins to my eclipse/plugins folder but it doesn't works...

Opening Eclipse it doesn't list ReviewBoard as an available connector to perform a query. Missing something?

rombert commented 12 years ago

You should download the update site zip, right now https://ereviewboard.ci.cloudbees.com/job/eReviewBoard/92/org.review_board.ereviewboard$org.review_board.ereviewboard.update/ -> the -assembly.zip file.

and installing using Help -> Install new software -> Add -> Archive -> ....

Robert

pghilardi commented 12 years ago

With this new version I'm having the same error as before: "Invalid id hash".

And I'm generating diff using 'git diff' (command line)

pghilardi commented 12 years ago

I have tested creating a diff with 'format-patch' and with 'diff' and on both ways I cannot view the diff. :/

rombert commented 12 years ago

I'm really not sure why this happens. Is there some way I can test this myself? It would help if you could generate a patch against a public project and then verify that it does not apply.

pghilardi commented 12 years ago

I think that could be a problem on my server configuration or something like that. I will try to make this on a public project this week...

Thanks for now

pghilardi commented 12 years ago

Hey,

I have downloaded eReviewBoard source code and debugged on my use case. On method getContents() of EGitSCMFileContentsLocator class, I have a repository object (Repository) with a weird path: /home/company/essshub10/services/git/repositories-http/pwda11.git

The path is expected to be on this format?

Thanks

pghilardi commented 12 years ago

I have noticed a weird behavior using ObjectId from jgit. Using this code:

ObjectId.fromString(hash)

On code, with hash equals to "1115d38" jgit returns the exception "Invalid id 1115d38". If I use this id and execute "git show 1115d38" at terminal on my git project it works perfectly! I don't understand what is happening inside Eclipse, because it works at terminal.

rombert commented 12 years ago

I suggest you open a bug on bug.eclipse.org ( JGit project ) and reference this discussion as the originator.

Robert

On Tue, Apr 17, 2012 at 3:09 PM, pghilardi reply@reply.github.com wrote: I have noticed a weird behavior using ObjectId from jgit. Using this code:

ObjectId.fromString(hash)

On code, with hash equals to "1115d38" jgit returns the exception "Invalid id 1115d38". If I use this id and execute "git show 1115d38" at terminal on my git project it works perfectly! I don't understand what is happening inside Eclipse, because it works at terminal.

Reply to this email directly or view it on GitHub: https://github.com/rombert/ereviewboard/issues/94#issuecomment-5173484

Sent from my (old) computer

pghilardi commented 12 years ago

I have forked eReviewBoard project and done two commits that maybe could be integrated on your code.

Fix wrong .project names on egit plugins https://github.com/pghilardi/ereviewboard/commit/348afdf791c39a7d1ecb3e8a263284ecfad4bb3e

Fix bug of invalid object id https://github.com/pghilardi/ereviewboard/commit/60554157b3c007447961a767eb68f9bae3cea802

Now, it's working! :) Probably I will go to contribute with more changes on this plug-in.

Thanks

rombert commented 12 years ago

Thanks!

https://github.com/pghilardi/ereviewboard/commit/60554157b3c007447961a767eb68f9bae3cea802 looks like it has lots of formatting changes and it's hard for me to see the actual changes.

Can you amend this commit to not have any whitespace/formatting changes?

Thanks

On Tue, Apr 17, 2012 at 10:31 PM, pghilardi reply@reply.github.com wrote:

I have forked eReviewBoard project and done two commits that maybe could be integrated on your code.

Fix wrong .project names on egit plugins https://github.com/pghilardi/ereviewboard/commit/348afdf791c39a7d1ecb3e8a263284ecfad4bb3e

Fix bug of invalid object id https://github.com/pghilardi/ereviewboard/commit/60554157b3c007447961a767eb68f9bae3cea802

Now, it's working! :) Probably I will go to contribute with more changes on this plug-in.

Thanks

Reply to this email directly or view it on GitHub: https://github.com/rombert/ereviewboard/issues/94#issuecomment-5183724

Sent from my (old) computer

pghilardi commented 12 years ago

Sorry, I have formatted automatically on Eclipse. I will ammend this commit tomorrow.

Thanks

pghilardi commented 12 years ago

I have fixed the commit removing whitespace/formatting changes.

Link here: https://github.com/pghilardi/ereviewboard/commit/666cf536ead1cf773fe37e573ae68a03634918ea

rombert commented 12 years ago

Fixed with pull request #97 . Thanks!