Fix bug in - [PBGitRepository addRef:] with symbolic refs showing the wrong name

Author: rowanj

Classes/git/PBGitCommit.m

@@ -115,9 +115,9 @@ - (NSString *)SVNRevision
 - (PBGitSHA *)sha
 {
 	if (!self->_sha) {
-		const git_oid *oid = git_commit_id(self.gtCommit.git_commit);
+		const git_oid *oid = self.gtCommit.OID.git_oid;
 		if (oid) {
-			PBGitSHA *newSha = [PBGitSHA shaWithOID:*oid];
+			PBGitSHA *newSha = [PBGitSHA shaWithOID:oid];
 			self.sha = newSha;
 		}
 	}
@@ -210,12 +210,12 @@ - (BOOL) hasRef:(PBGitRef *)ref
 
 - (NSMutableArray *)refs
 {
-	return [[self.repository refs] objectForKey:self.sha];
+	return self.repository.refs[self.sha];
 }
 
 - (void) setRefs:(NSMutableArray *)refs
 {
-	[[self.repository refs] setObject:refs forKey:self.sha];
+	self.repository.refs[self.sha] = [NSMutableArray arrayWithArray:refs];
 }
 
 

Classes/git/PBGitRepository.m

@@ -189,43 +189,30 @@ - (PBGitWindowController *)windowController
 
 - (void) addRef:(GTReference*)gtRef
 {
-	while (gtRef && gtRef.referenceType == GTReferenceTypeSymbolic) {
-//		GTReference *oldRef = gtRef;
-		gtRef = gtRef.resolvedReference;
-//		NSLog(@"Resolved reference %@ to %@", oldRef, gtRef);
-	}
-	if (gtRef == nil)
-	{
-		NSLog(@"Sneaky attempt to add nil GTReference");
+	GTObject *refTarget = gtRef.resolvedTarget;
+	if (![refTarget isKindOfClass:[GTObject class]]) {
+		NSLog(@"Tried to add invalid ref %@ -> %@", gtRef, refTarget);
 		return;
 	}
-	if (gtRef.referenceType != GTReferenceTypeOid) {
-		NSLog(@"Can't addRef for %@ ref of unsupported type \"%u\"", gtRef.name, gtRef.referenceType);
+
+	PBGitSHA *sha = [PBGitSHA shaWithOID:refTarget.OID.git_oid];
+	if (!sha) {
+		NSLog(@"Couldn't determine sha for ref %@ -> %@", gtRef, refTarget);
 		return;
 	}
 
-	git_oid refOid = *gtRef.OID.git_oid;
-	git_object* gitTarget = NULL;
-	git_tag* gitTag = NULL;
-
-	PBGitSHA *sha = [PBGitSHA shaWithOID:refOid];
-	if (git_tag_lookup(&gitTag, self.gtRepo.git_repository, gtRef.git_oid) == GIT_OK)
-	{
-		if (git_tag_peel(&gitTarget, gitTag) == GIT_OK)
-		{
-			GTObject* peeledObject = [GTObject objectWithObj:gitTarget inRepository:self.gtRepo];
-//			NSLog(@"peeled sha:%@", peeledObject.sha);
-			sha = [PBGitSHA shaWithString:peeledObject.SHA];
-		}
-	}
-	
 	PBGitRef* ref = [[PBGitRef alloc] initWithString:gtRef.name];
 //	NSLog(@"addRef %@ %@ at %@", ref.type, gtRef.name, [sha string]);
-	NSMutableArray* curRefs;
-	if ( (curRefs = [refs objectForKey:sha]) != nil )
+	NSMutableArray* curRefs = refs[sha];
+	if ( curRefs != nil ) {
+		if ([curRefs containsObject:ref]) {
+			NSLog(@"Duplicate ref shouldn't be added: %@", ref);
+			return;
+		}
 		[curRefs addObject:ref];
-	else
-		[refs setObject:[NSMutableArray arrayWithObject:ref] forKey:sha];
+	} else {
+		refs[sha] = [NSMutableArray arrayWithObject:ref];
+	}
 }
 
 int addSubmoduleName(git_submodule *module, const char* name, void * context)
@@ -343,9 +330,10 @@ - (PBGitSHA *)shaForRef:(PBGitRef *)ref
 	if (!ref)
 		return nil;
 
-	for (PBGitSHA *sha in refs)
+	for (PBGitSHA *sha in refs.allKeys)
 	{
-		for (PBGitRef *existingRef in [refs objectForKey:sha])
+		NSMutableSet *refsForSha = [refs objectForKey:sha];
+		for (PBGitRef *existingRef in refsForSha)
 		{
 			if ([existingRef isEqualToRef:ref])
 			{
@@ -459,11 +447,12 @@ - (BOOL) checkRefFormat:(NSString *)refName
 
 - (BOOL) refExists:(PBGitRef *)ref
 {
-	int retValue = 1;
-    NSString *output = [self outputInWorkdirForArguments:[NSArray arrayWithObjects:@"for-each-ref", [ref ref], nil] retValue:&retValue];
-    if (retValue || [output isEqualToString:@""])
-        return NO;
-    return YES;
+	NSError *gtError = nil;
+	GTReference *gtRef = [GTReference referenceByLookingUpReferencedNamed:ref.ref inRepository:self.gtRepo error:&gtError];
+	if (gtRef) {
+		return YES;
+	}
+	return NO;
 }
 
 // useful for getting the full ref for a user entered name
@@ -579,8 +568,9 @@ - (PBGitRef *) remoteRefForBranch:(PBGitRef *)branch error:(NSError **)error
 		if (remoteName && ([remoteName isKindOfClass:[NSString class]] && ![remoteName isEqualToString:@""])) {
 			PBGitRef *remoteRef = [PBGitRef refFromString:[kGitXRemoteRefPrefix stringByAppendingString:remoteName]];
 			// check that the remote is a valid ref and exists
-			if ([self checkRefFormat:[remoteRef ref]] && [self refExists:remoteRef])
+			if ([self checkRefFormat:[remoteRef ref]] && [self refExists:remoteRef]) {
 				return remoteRef;
+			}
 		}
 	}
 

Classes/git/PBGitSHA.h

@@ -10,19 +10,15 @@
 #include <git2/oid.h>
 
 
-@interface PBGitSHA : NSObject <NSCopying> {
-	git_oid oid;
-	NSString *string;
-}
+@interface PBGitSHA : NSObject <NSCopying>
 
-
-+ (PBGitSHA *)shaWithOID:(git_oid)oid;
++ (PBGitSHA *)shaWithOID:(git_oid const *)oid;
 + (PBGitSHA *)shaWithString:(NSString *)shaString;
 + (PBGitSHA *)shaWithCString:(const char *)shaCString;
 
-- (BOOL)isEqualToOID:(git_oid)other_oid;
+- (BOOL)isEqualToOID:(git_oid const *)other_oid;
 
-@property (readonly) git_oid oid;
-@property (readonly) NSString *string;
+@property (nonatomic, assign, readonly) git_oid oid;
+@property (nonatomic, strong, readonly) NSString *string;
 
 @end

Classes/git/PBGitSHA.m

@@ -12,19 +12,16 @@
 
 @interface PBGitSHA ()
 
-- (id)initWithOID:(git_oid)g_oid;
+@property (nonatomic, assign) git_oid oid;
+@property (nonatomic, strong) NSString *string;
 
 @end
 
 
 @implementation PBGitSHA
 
 
-@synthesize oid;
-@synthesize string;
-
-
-+ (PBGitSHA *)shaWithOID:(git_oid)oid
++ (PBGitSHA *)shaWithOID:(git_oid const *)oid
 {
 	return [[PBGitSHA alloc] initWithOID:oid];
 }
@@ -34,10 +31,11 @@ + (PBGitSHA *)shaWithString:(NSString *)shaString
 {
 	git_oid oid;
 	int err = git_oid_fromstr(&oid, [shaString UTF8String]);
-	if (err != GIT_OK)
+	if (err != GIT_OK) {
 		return nil;
+	}
 
-	return [self shaWithOID:oid];
+	return [self shaWithOID:&oid];
 }
 
 
@@ -48,7 +46,7 @@ + (PBGitSHA *)shaWithCString:(const char *)shaCString
 	if (err != GIT_OK)
 		return nil;
 
-	return [self shaWithOID:oid];
+	return [self shaWithOID:&oid];
 }
 
 
@@ -68,29 +66,35 @@ + (BOOL)isKeyExcludedFromWebScript:(const char *)name
 #pragma mark -
 #pragma mark PBGitSHA
 
-- (id)initWithOID:(git_oid)g_oid
+- (id)initWithOID:(git_oid const *)g_oid
 {
+	if (!g_oid) {
+		return nil;
+	}
+
 	self = [super init];
-	if (!self)
+	if (!self) {
 		return nil;
+	}
 
-	oid = g_oid;
+	_oid = *g_oid;
 
 	return self;
 }
 
 
 - (NSString *)string
 {
-	if (!string) {
-		const size_t buffer_size = GIT_OID_HEXSZ + 1;
-		char hex[buffer_size] = {0};
-
-		const char* result = git_oid_tostr(hex, buffer_size, &oid);
-		string = [NSString stringWithUTF8String:result];
+	if (_string) {
+		return _string;
 	}
 
-	return string;
+	const size_t buffer_size = GIT_OID_HEXSZ + 1;
+	char hex[buffer_size] = {0};
+
+	const char* result = git_oid_tostr(hex, buffer_size, &_oid);
+	_string = [NSString stringWithUTF8String:result];
+	return _string;
 }
 
 
@@ -102,21 +106,23 @@ - (BOOL)isEqual:(id)otherSHA
 	if  (!otherSHA)
 		return NO;
 
-	git_oid other_oid = ((PBGitSHA *)otherSHA)->oid;
-	return git_oid_cmp(&oid, &other_oid) == 0;
+	return git_oid_cmp(&_oid, &((PBGitSHA *)otherSHA)->_oid) == 0;
 }
 
 
-- (BOOL)isEqualToOID:(git_oid)other_oid
+- (BOOL)isEqualToOID:(git_oid const *)other_oid
 {
-	return git_oid_cmp(&oid, &other_oid) == 0;
+	if (!other_oid) {
+		return nil;
+	}
+	return git_oid_cmp(&_oid, other_oid) == 0;
 }
 
 
 - (NSUInteger)hash
 {
 	NSUInteger hash;
-	memcpy(&hash, &(oid.id), sizeof(NSUInteger));
+	memcpy(&hash, &(_oid.id), sizeof(NSUInteger));
 
 	return hash;
 }
@@ -133,9 +139,7 @@ - (NSString *)description
 
 - (id)copyWithZone:(NSZone *)zone
 {
-	git_oid oidCopy;
-	git_oid_cpy(&oidCopy, &oid);
-    PBGitSHA *copy = [[[self class] allocWithZone:zone] initWithOID:oidCopy];
+    PBGitSHA *copy = [[[self class] allocWithZone:zone] initWithOID:&(self->_oid)];
 
 	return copy;
 }

GitX.xcodeproj/project.pbxproj

@@ -969,6 +969,8 @@
 				4A5D768914A9A9CC00DF6C68 /* GitXRelativeDateFormatter.m */,
 				4A5D768A14A9A9CC00DF6C68 /* GitXScriptingConstants.h */,
 				4A5D768B14A9A9CC00DF6C68 /* main.m */,
+				4A2125A217C0C78A00B5B582 /* NSColor+RGB.h */,
+				4A2125A317C0C78A00B5B582 /* NSColor+RGB.m */,
 				4A5D768C14A9A9CC00DF6C68 /* PBChangedFile.h */,
 				4A5D768D14A9A9CC00DF6C68 /* PBChangedFile.m */,
 				4A5D768E14A9A9CC00DF6C68 /* PBCLIProxy.h */,
@@ -988,8 +990,6 @@
 				4A5D76A314A9A9CC00DF6C68 /* Terminal.h */,
 				4A5D76A414A9A9CC00DF6C68 /* Util */,
 				4A5D76B314A9A9CC00DF6C68 /* Views */,
-				4A2125A217C0C78A00B5B582 /* NSColor+RGB.h */,
-				4A2125A317C0C78A00B5B582 /* NSColor+RGB.m */,
 			);
 			path = Classes;
 			sourceTree = "<group>";