= bug description =
When getting plain blobs from cgit, the url queries are either returning an empty response or always returning MASTER HEAD even if other sha1s are specified.
An example of this is the blockerbugs/config.py file in the blockerbugs git repo * head on the development branch: - http://git.fedorahosted.org/cgit/blockerbugs.git/tree/blockerbugs/config.py?h=develop * Specifying the id or id2 with the branch name results in an empty return: - http://git.fedorahosted.org/cgit/blockerbugs.git/plain/blockerbugs/config.py?h=develop?id=75cf777d31e7fc3bfe3256704cd349e3ca88c866 - http://git.fedorahosted.org/cgit/blockerbugs.git/plain/blockerbugs/config.py?h=develop?id2=75cf777d31e7fc3bfe3256704cd349e3ca88c866 * Specifying the id w/o branch name returns empty - http://git.fedorahosted.org/cgit/blockerbugs.git/plain/blockerbugs/config.py?id=75cf777d31e7fc3bfe3256704cd349e3ca88c866 * Specifying the id2 w/o branch name returns HEAD from MASTER which has a different sha1 - http://git.fedorahosted.org/cgit/blockerbugs.git/plain/blockerbugs/config.py?id2=75cf777d31e7fc3bfe3256704cd349e3ca88c866 - (ref to actual file returned) http://git.fedorahosted.org/cgit/blockerbugs.git/plain/blockerbugs/config.py?id2=009e7a97f57e31fcc499dba87fe7d33e41252a2c
Oddly enough, using a revision that isn't head on a branch doesn't hit the same problem whether the branch name is specified or not: * (reference) http://git.fedorahosted.org/cgit/blockerbugs.git/tree/blockerbugs/config.py?h=develop&id=b1b2ac7f027914dd8b2fc1ab5b8f42da863038a6 * http://git.fedorahosted.org/cgit/blockerbugs.git/plain/blockerbugs/config.py?h=develop&id=b1b2ac7f027914dd8b2fc1ab5b8f42da863038a6 * http://git.fedorahosted.org/cgit/blockerbugs.git/plain/blockerbugs/config.py?id=b1b2ac7f027914dd8b2fc1ab5b8f42da863038a6
= bug analysis =
The use case that I'm hitting is doing reviews with reviewboard. When generating diffs, reviewboard grabs the source files from cgit's web interface using the filename and sha1 id. Since the incorrect file is being returned from cgit, the diff fails and there are errors in the review.
= fix recommendation =
It looks like this might be an issue with redirects because I can't seem to reproduce it on cgit's cgit instance.
Using jd/zx2c4-deployment/Makefile as a reference: * (reference) http://git.zx2c4.com/cgit/tree/Makefile?h=jd/zx2c4-deployment&id=29d8001a2f7b2075543b11e4cdd737a715e2b4cd * (specifying branch name) http://git.zx2c4.com/cgit/plain/Makefile?h=jd/zx2c4-deployment&id=29d8001a2f7b2075543b11e4cdd737a715e2b4cd * (just speficying sha1) http://git.zx2c4.com/cgit/plain/Makefile?id=29d8001a2f7b2075543b11e4cdd737a715e2b4cd
Had cgit's cgit instance shown the same bug, it would have returned nothing or the contents of: * http://git.zx2c4.com/cgit/plain/Makefile
It's also worth keeping this in mind: https://groups.google.com/forum/?fromgroups=#!topic/reviewboard/PqglgAIX9Ug
There are other issues going on here, which is probably a bug in cgit. First off, I looked up the meaning of id vs id2 and apparently it's expecting sha1 vs sha2 hashes. In the id2 case, if the hash passed to it is the sha1 hash, the returned information is for HEAD (because it will apparently to default to HEAD if it cannot find the requested hash). When I pass the correct hash to id, it returns nothing at all.
FWIW, I believe that git always generates sha1 hashes, so we should probably be using id= in all situations. I'll propose a patch to the ReviewBoard docs too. I'm not sure why cgit even has sha2 support, since it doesn't appear to exist in upstream git.
Yeah, I was thinking this was related to redirects as well, so I disabled them and saw the same issue.
It looks like it's passing a blob sha when cgit expects it to be asking for a commit sha.
So, I think reviewboard is asking for the wrong thing here...
After talking to kevin on IRC, I changed the raw file URL mask in reviewboard to be: http://git.fedorahosted.org/cgit/PROJECT.git/blob/<filename>?id=<revision>
where PROJECT is the fedorahosted project in question.
After I cleared the file cache, the diffs seem to be working fine. I'll try a few more reviews but I suspect this issue is solved and it was a simple fix :)
I've submitted a patch to the Review Board upstream codebase to address this in the future: http://reviews.reviewboard.org/r/3930/
This seems to be solved by the above reviewboard config change.
Feel free to reopen if there's anything further for us to do.
There was more to the Review Board change after all. We were also specifying /plain/ instead of /blob/ in the default configuration provided by the Fedora Hosted hosting service. Switching to blob coupled with id= should be correct now. I'm updating my patch to upstream.
Login to comment on this ticket.