On Sun, Mar 22, 2009 at 12:54 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > A tag can point at anything, so this is not an issue about "crash on a > _corrupt_ repository". Ah, my bad. I wrongly assumed corrupted repos were the only ways of triggering this issue. I quite easily managed to reproduce the crash by setting up some tags to trees and tag objects to trees. > I am not very familiar with this program, but the codepath involved should > be prepared to _see_ any type of object instead of dying. > > What to do after _seeing_ a type of object is a different matter. It > appears that there is no way to feed a tree object to fast-import, but I > think the fast-import language can represent a tag that points at another > tag just fine. So the best you can do is perhaps to issue a warning > "skipping a tag that points at a tree object" and impoement a proper > handling of a tag that points at a tag. > My patch simply applied the same error that was already present for tags to tag objects, but yeah. Handling tagged tags and warning instead of erroring-out makes more sense to me as well. I'll see if I can write it up, and resubmit a patch. After looking some more at the code, it seems that there's an attempt to handle tags to tags there already, but it doesn't seem to work properly; the program error out with a "fatal: Tag [tagname] points nowhere?". This seems to be because the tagged-pointer of the second tag-object is NULL. Now, I'm no expert, but from browsing some code, it seems that "parse_object(tag->object.sha1)" should have been performed on the tag before looking up the tagged object. Does this make sense? Also, I guess this calls for a couple of test-cases or something. I haven't written any tests yet, so I might need some time to figuring it out. Anyway, I guess this makes the most sense as a four-patch series: 1) Add test-cases for tags of tag objects, tag objects of tag objects, tags of trees and tag objects of trees. 2) Turn the existing error into a warning 3) Add the missing warning and remove the crash 4) Fix fast-export to export tags pointing to tags. Makes sense? Additionally, to reduce the chance of similar bugs in the future, the code could be refactored a bit to have a handle_commit()-function, so what goes on becomes a bit more obvious. Since this doesn't really change any functionality, I guess it could be handed in as a separate patch. -- Erik "kusma" Faye-Lund kusmabite@xxxxxxxxx (+47) 986 59 656 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
