{"thread":{"id":"14558","subject":"problem using jgit","startedAt":"2008-07-21T06:24:45Z","lastAt":"2008-07-27T03:21:07Z","messageCount":8,"participants":["Stephen Bannasch","Marek Zawirski","Shawn O. Pearce"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"84144","messageId":"p06240809c4a9d887fda4@[192.168.1.106]","threadId":"14558","inReplyTo":null,"subject":"problem using jgit","fromName":"Stephen Bannasch","fromEmail":"stephen.bannasch@deanbrook.org","sentAt":"2008-07-21T06:24:45Z","receivedAt":"2008-07-21T06:24:45Z","isPatch":false,"sender":{"key":"stephen.bannasch@deanbrook.org","avatar":null},"body":"I've setup a simple test class that integrates jgit to clone a git \nrepository. However I'm getting a NullPointerError when \nRevWalk.parseAny ends up producing a null object id.\n\nThe code and the stack trace for the error are here:\n\n http://pastie.org/237711\n\nThis problem occurs using the jgit from the master branch from this repo:\n\n git://repo.or.cz/egit.git\n"},{"id":"84173","messageId":"488467E3.7000107@gmail.com","threadId":"14558","inReplyTo":"p06240809c4a9d887fda4@[192.168.1.106]","subject":"Re: problem using jgit","fromName":"Marek Zawirski","fromEmail":"marek.zawirski@gmail.com","sentAt":"2008-07-21T10:41:39Z","receivedAt":"2008-07-21T10:41:39Z","isPatch":false,"sender":{"key":"marek.zawirski@gmail.com","avatar":null},"body":"Stephen Bannasch wrote:\n> I've setup a simple test class that integrates jgit to clone a git \n> repository. However I'm getting a NullPointerError when \n> RevWalk.parseAny ends up producing a null object id.\n>\n> The code and the stack trace for the error are here:\n>\n> http://pastie.org/237711\n>\n> This problem occurs using the jgit from the master branch from this repo:\n>\n> git://repo.or.cz/egit.git\nHello Stephen,\n\nI think you've experienced error caused by the same bug as me, during my \nlatest fetch/push GUI works few days ago.\nYour code looks fine, probably it's actually bug in jgit. I think it's \nsome regression. Thanks for reporting.\nI'll try to look at this problem or jgit fetch experts (Robin and Shawn) \nwill have a look when they come back. \n\nMarek\n"},{"id":"84189","messageId":"488482A2.4000601@gmail.com","threadId":"14558","inReplyTo":"488467E3.7000107@gmail.com","subject":"Re: problem using jgit","fromName":"Marek Zawirski","fromEmail":"marek.zawirski@gmail.com","sentAt":"2008-07-21T12:35:46Z","receivedAt":"2008-07-21T12:35:46Z","isPatch":false,"sender":{"key":"marek.zawirski@gmail.com","avatar":null},"body":"Marek Zawirski wrote:\n> Stephen Bannasch wrote:\n>> I've setup a simple test class that integrates jgit to clone a git \n>> repository. However I'm getting a NullPointerError when \n>> RevWalk.parseAny ends up producing a null object id.\n>>\n>> The code and the stack trace for the error are here:\n>>\n>> http://pastie.org/237711\n>>\n>> This problem occurs using the jgit from the master branch from this \n>> repo:\n>>\n>> git://repo.or.cz/egit.git\n> Hello Stephen,\n>\n> I think you've experienced error caused by the same bug as me, during \n> my latest fetch/push GUI works few days ago.\n> Your code looks fine, probably it's actually bug in jgit. I think \n> it's some regression. Thanks for reporting.\nIt's caused by 14a630c3: Cached modification times for symbolic refs too\nChanges introduced by this patch made Repository#getAllRefs() including \nRef objects with null ObjectId in case of unresolvable (invalid?) HEAD \nsymbolic ref, and null Ref for HEAD when it doesn't exist. Previous \nbehavior was just not including such refs in result.\n\nFix for null Ref is just a matter of simple filtering out null Ref \nobject for HEAD, if it doesn't exist (just is it considered to be legal \nstate of repository when HEAD doesn't exist?).\n\nTo fix null ObjectId issue, we have to either change all clients of this \nmethod or revert method to previous behavior. Now it's just unspecified \nin javadoc.\nRobin, Shawn, what do you think? If we want to have unresolvable refs \nincluded, IMO it may be sensible to provide argument includeUnresolbable \nfor Repository#getAllRefs() to let clients avoid burden of filtering \nthem out when they don't need them (most cases, perhaps).\nI can prepare fix for it (rather easy one) as you are unavailable now, \nlet me now what's your opinion.\n"},{"id":"84207","messageId":"p0624080dc4aa78c93ffe@[192.168.1.106]","threadId":"14558","inReplyTo":"488482A2.4000601@gmail.com","subject":"Re: problem using jgit","fromName":"Stephen Bannasch","fromEmail":"stephen.bannasch@deanbrook.org","sentAt":"2008-07-21T17:36:18Z","receivedAt":"2008-07-21T17:36:18Z","isPatch":false,"sender":{"key":"stephen.bannasch@deanbrook.org","avatar":null},"body":"At 2:35 PM +0200 7/21/08, Marek Zawirski wrote:\n>Marek Zawirski wrote:\n>>Stephen Bannasch wrote:\n>>>I've setup a simple test class that integrates jgit to clone a git repository. However I'm getting a NullPointerError when RevWalk.parseAny ends up producing a null object id.\n>>>\n>>>The code and the stack trace for the error are here:\n>>>\n>>> http://pastie.org/237711\n>>>\n>>>This problem occurs using the jgit from the master branch from this repo:\n>>>\n>>> git://repo.or.cz/egit.git\n>>Hello Stephen,\n>>\n>>I think you've experienced error caused by the same bug as me, during my latest fetch/push GUI works few days ago.\n>>Your code looks fine, probably it's actually bug in jgit. I think it's some regression. Thanks for reporting.\n>It's caused by 14a630c3: Cached modification times for symbolic refs too\n>Changes introduced by this patch made Repository#getAllRefs() including Ref objects with null ObjectId in case of unresolvable (invalid?) HEAD symbolic ref, and null Ref for HEAD when it doesn't exist. Previous behavior was just not including such refs in result.\n>\n>Fix for null Ref is just a matter of simple filtering out null Ref object for HEAD, if it doesn't exist (just is it considered to be legal state of repository when HEAD doesn't exist?).\n>\n>To fix null ObjectId issue, we have to either change all clients of this method or revert method to previous behavior. Now it's just unspecified in javadoc.\n>Robin, Shawn, what do you think? If we want to have unresolvable refs included, IMO it may be sensible to provide argument includeUnresolbable for Repository#getAllRefs() to let clients avoid burden of filtering them out when they don't need them (most cases, perhaps).\n>I can prepare fix for it (rather easy one) as you are unavailable now, let me now what's your opinion.\n\nThanks for looking at this problem Marek. \n\nIf you get a change working for jgit that might not be available in a branch on git://repo.or.cz/egit.gi will you send a patch.\n\nI am looking forward to continuing my tests using jgit from Java and JRuby.\n"},{"id":"84324","messageId":"4885C9C8.9090003@gmail.com","threadId":"14558","inReplyTo":"p0624080dc4aa78c93ffe@[192.168.1.106]","subject":"Re: problem using jgit","fromName":"Marek Zawirski","fromEmail":"marek.zawirski@gmail.com","sentAt":"2008-07-22T11:51:36Z","receivedAt":"2008-07-22T11:51:36Z","isPatch":false,"sender":{"key":"marek.zawirski@gmail.com","avatar":null},"body":"Stephen Bannasch wrote:\n> At 2:35 PM +0200 7/21/08, Marek Zawirski wrote:\n> \n>> Marek Zawirski wrote:\n>> \n>>> Stephen Bannasch wrote:\n>>> \n>>>> I've setup a simple test class that integrates jgit to clone a git repository. However I'm getting a NullPointerError when RevWalk.parseAny ends up producing a null object id.\n>>>>\n>>>> The code and the stack trace for the error are here:\n>>>>\n>>>> http://pastie.org/237711\n>>>>\n>>>> This problem occurs using the jgit from the master branch from this repo:\n>>>>\n>>>> git://repo.or.cz/egit.git\n>>>> \n>>> Hello Stephen,\n>>>\n>>> I think you've experienced error caused by the same bug as me, during my latest fetch/push GUI works few days ago.\n>>> Your code looks fine, probably it's actually bug in jgit. I think it's some regression. Thanks for reporting.\n>>> \n>> It's caused by 14a630c3: Cached modification times for symbolic refs too\n>> Changes introduced by this patch made Repository#getAllRefs() including Ref objects with null ObjectId in case of unresolvable (invalid?) HEAD symbolic ref, and null Ref for HEAD when it doesn't exist. Previous behavior was just not including such refs in result.\n>>\n>> Fix for null Ref is just a matter of simple filtering out null Ref object for HEAD, if it doesn't exist (just is it considered to be legal state of repository when HEAD doesn't exist?).\n>>\n>> To fix null ObjectId issue, we have to either change all clients of this method or revert method to previous behavior. Now it's just unspecified in javadoc.\n>> Robin, Shawn, what do you think? If we want to have unresolvable refs included, IMO it may be sensible to provide argument includeUnresolbable for Repository#getAllRefs() to let clients avoid burden of filtering them out when they don't need them (most cases, perhaps).\n>> I can prepare fix for it (rather easy one) as you are unavailable now, let me now what's your opinion.\n>> \n>\n> Thanks for looking at this problem Marek. \n>\n> If you get a change working for jgit that might not be available in a branch on git://repo.or.cz/egit.gi will you send a patch.\n>\n> I am looking forward to continuing my tests using jgit from Java and JRuby.\n> \n\nYou can find temporary workaround for that in \"workaround\" branch at \ngit://repo.or.cz/egit/zawir.git\nIt's rather not a final solution (disables unresolvable HEADs), but I \nhope it let you continue using jgit for a while, as it does for me ;)\n"},{"id":"84336","messageId":"20080722165831.GA11173@spearce.org","threadId":"14558","inReplyTo":"488482A2.4000601@gmail.com","subject":"Re: problem using jgit","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-22T16:58:31Z","receivedAt":"2008-07-22T16:58:31Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Marek Zawirski wrote:\n> Marek Zawirski wrote:\n>> Stephen Bannasch wrote:\n>>> I've setup a simple test class that integrates jgit to clone a git \n>>> repository. However I'm getting a NullPointerError when \n>>> RevWalk.parseAny ends up producing a null object id.\n...\n> It's caused by 14a630c3: Cached modification times for symbolic refs too\n> Changes introduced by this patch made Repository#getAllRefs() including \n> Ref objects with null ObjectId in case of unresolvable (invalid?) HEAD \n> symbolic ref, and null Ref for HEAD when it doesn't exist. Previous \n> behavior was just not including such refs in result.\n\nMy intention here was that if a ref cannot be resolved, it should\nnot be reported. So Ref.getObjectId should never return null, and\nit should also never return an ObjectId for which the object does\nnot exist in the Repository's object database(s). (Though that can\nhappen in the face of repository corruption, but lets not go there\njust yet).\n\nSo IMHO the RefDatabase code is _wrong_ for returning HEAD with a\nnull objectId.\n\nNow this case can happen if HEAD points at a stillborn branch. This\nis easily reproduced in any repository, e.g. just do:\n\n\tgit symbolic-ref HEAD refs/heads/`date`\n\nYou'll wind up on a branch which doesn't exist. In this case HEAD\nshouldn't be reported back from RefDatabase, it doesn't exist, as\nbranch `date` does not exist either.\n\n-- \nShawn.\n"},{"id":"84888","messageId":"4889E88E.8000701@gmail.com","threadId":"14558","inReplyTo":"20080722165831.GA11173@spearce.org","subject":"Re: problem using jgit","fromName":"Marek Zawirski","fromEmail":"marek.zawirski@gmail.com","sentAt":"2008-07-25T14:51:58Z","receivedAt":"2008-07-25T14:51:58Z","isPatch":false,"sender":{"key":"marek.zawirski@gmail.com","avatar":null},"body":"Shawn O. Pearce wrote:\n> Marek Zawirski wrote:\n> \n>> Marek Zawirski wrote:\n>> \n>>> Stephen Bannasch wrote:\n>>> \n>>>> I've setup a simple test class that integrates jgit to clone a git \n>>>> repository. However I'm getting a NullPointerError when \n>>>> RevWalk.parseAny ends up producing a null object id.\n>>>> \n> ...\n> \n>> It's caused by 14a630c3: Cached modification times for symbolic refs too\n>> Changes introduced by this patch made Repository#getAllRefs() including \n>> Ref objects with null ObjectId in case of unresolvable (invalid?) HEAD \n>> symbolic ref, and null Ref for HEAD when it doesn't exist. Previous \n>> behavior was just not including such refs in result.\n>> \n>\n> My intention here was that if a ref cannot be resolved, it should\n> not be reported. So Ref.getObjectId should never return null, and\n> it should also never return an ObjectId for which the object does\n> not exist in the Repository's object database(s). (Though that can\n> happen in the face of repository corruption, but lets not go there\n> just yet).\n>\n> So IMHO the RefDatabase code is _wrong_ for returning HEAD with a\n> null objectId.\n>\n> Now this case can happen if HEAD points at a stillborn branch. This\n> is easily reproduced in any repository, e.g. just do:\n>\n> \tgit symbolic-ref HEAD refs/heads/`date`\n>\n> You'll wind up on a branch which doesn't exist. In this case HEAD\n> shouldn't be reported back from RefDatabase, it doesn't exist, as\n> branch `date` does not exist either.\n>\n> \nBeside of my temporary fix for that that filters null Ref and Ref with \nnull objectId, I think that 2 more issues may need to be resolved:\n\n1) readRefBasic() method is used for reading arbitrary refs, potentially \nnot only those from well-known prefixes as readRefs() does. Is calling \nsetModified() appropriate for those other refs?\n\n2) Am I wrong that setModified() is not called in all cases? Consider \nempty ref file and just...\nif (line == null || line.length() == 0)\n return new Ref(Ref.Storage.LOOSE, name, null);\n"},{"id":"85135","messageId":"20080727032107.GC17425@spearce.org","threadId":"14558","inReplyTo":"4889E88E.8000701@gmail.com","subject":"Re: problem using jgit","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-27T03:21:07Z","receivedAt":"2008-07-27T03:21:07Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Marek Zawirski wrote:\n> Shawn O. Pearce wrote:\n>> Marek Zawirski wrote:\n>>> \n>>> It's caused by 14a630c3: Cached modification times for symbolic refs too\n>>> Changes introduced by this patch made Repository#getAllRefs() \n>>> including Ref objects with null ObjectId in case of unresolvable \n>>> (invalid?) HEAD symbolic ref, and null Ref for HEAD when it doesn't \n>>> exist. Previous behavior was just not including such refs in result.\n>>\n>> My intention here was that if a ref cannot be resolved, it should\n>> not be reported. [...]\n> \n> Beside of my temporary fix for that that filters null Ref and Ref with \n> null objectId, I think that 2 more issues may need to be resolved:\n\nWell, I think your temporary fix is correct. I don't see another way\nto implement around it.\n\n> 1) readRefBasic() method is used for reading arbitrary refs, potentially \n> not only those from well-known prefixes as readRefs() does. Is calling \n> setModified() appropriate for those other refs?\n\nYes. If we read something and it is different from what we have cached\nwe need to signal that the cached data is invalid (which is setModified).\nDoing so allows listeners to (eventually) find out that there are changes\nmade on disk which their subscribers don't know about, but may need to be\ninformed of. This way we can identify changes made by command line tools\nto a repository that egit has open in the workbench.\n\n> 2) Am I wrong that setModified() is not called in all cases? Consider \n> empty ref file and just...\n> if (line == null || line.length() == 0)\n> return new Ref(Ref.Storage.LOOSE, name, null);\n>\n\nIn this case (and the other like it) we don't call setModified\nbecause there hasn't been a change on disk. The file wasn't\npreviously in our cache and the file is empty now. Either way\nthe ref has no data so there is no need to notify listeners.\n\nNow there may be other cases that are missing, but this one\nis fine as is.\n\nSo I'm thinking of applying this:\n\n--8<--\nFrom: Marek Zawirski \nSubject: [PATCH] Fix Repository.getAllRefs to skip HEAD if it points to an unborn branch\n\nIf HEAD is a symbolic reference pointing to an unborn branch (branch\nnot yet created) we get a Ref object back for it supplying the name\nof the branch, but its ObjectId is null as the branch file itself is\nnot present in the repository. In such cases we should not include\nthe HEAD reference in the returned output.\n\nSigned-off-by: Marek Zawirski \nSigned-off-by: Shawn O. Pearce \n---\n .../src/org/spearce/jgit/lib/RefDatabase.java | 4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\nindex 82b995e..b9c8c8c 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n@@ -209,7 +209,9 @@ class RefDatabase {\n \t\treadPackedRefs(avail);\n \t\treadLooseRefs(avail, REFS_SLASH, refsDir);\n \t\ttry {\n-\t\t\tavail.put(Constants.HEAD, readRefBasic(Constants.HEAD, 0));\n+\t\t\tfinal Ref r = readRefBasic(Constants.HEAD, 0);\n+\t\t\tif (r != null && r.getObjectId() != null)\n+\t\t\t\tavail.put(Constants.HEAD, r);\n \t\t} catch (IOException e) {\n \t\t\t// ignore here\n \t\t}\n-- \n1.6.0.rc0.182.gb96c7\n\n\n-- \nShawn.\n"}]}