Handle submodule conflicts (#996)

GitUpKit/Components/GIDiffContentsViewController.m

@@ -69,6 +69,14 @@ @interface GIConflictDiffCellView : NSTableCellView
 @property(nonatomic, weak) IBOutlet NSButton* resolveButton;
 @end
 
+@interface GISubmoduleConflictDiffCellView : NSTableCellView
+@property(nonatomic, weak) IBOutlet NSTextField* statusTextField;
+@property(nonatomic, weak) IBOutlet NSTextField* oursTextField;
+@property(nonatomic, weak) IBOutlet NSTextField* theirsTextField;
+@property(nonatomic, weak) IBOutlet NSButton* chooseOursButton;
+@property(nonatomic, weak) IBOutlet NSButton* chooseTheirsButton;
+@end
+
 @interface GISubmoduleDiffCellView : NSTableCellView
 @property(nonatomic, weak) IBOutlet NSView* contentView;
 @property(nonatomic, weak) IBOutlet NSTextField* oldSHA1TextField;
@@ -158,6 +166,9 @@ @implementation GIBinaryDiffCellView
 @implementation GIConflictDiffCellView
 @end
 
+@implementation GISubmoduleConflictDiffCellView
+@end
+
 @implementation GISubmoduleDiffCellView
 @end
 
@@ -183,6 +194,7 @@ @implementation GIDiffContentsViewController {
   CGFloat _headerViewHeight;
   CGFloat _emptyViewHeight;
   CGFloat _conflictViewHeight;
+  CGFloat _submoduleConflictViewHeight;
   CGFloat _submoduleViewHeight;
   CGFloat _binaryViewHeight;
 }
@@ -227,6 +239,7 @@ - (void)loadView {
   _headerViewHeight = [[_tableView makeViewWithIdentifier:@"header" owner:self] frame].size.height;
   _emptyViewHeight = [[_tableView makeViewWithIdentifier:@"empty" owner:self] frame].size.height;
   _conflictViewHeight = [[_tableView makeViewWithIdentifier:@"conflict" owner:self] frame].size.height;
+  _submoduleConflictViewHeight = [[_tableView makeViewWithIdentifier:@"submodule_conflict" owner:self] frame].size.height;
   _submoduleViewHeight = [[_tableView makeViewWithIdentifier:@"submodule" owner:self] frame].size.height;
   _binaryViewHeight = [[_tableView makeViewWithIdentifier:@"binary" owner:self] frame].size.height;
 
@@ -537,12 +550,23 @@ - (NSView*)tableView:(NSTableView*)tableView viewForTableColumn:(NSTableColumn*)
           status = NSLocalizedString(@"deleted by them", nil);
           break;
       }
-      GIConflictDiffCellView* view = [_tableView makeViewWithIdentifier:@"conflict" owner:self];
-      view.statusTextField.stringValue = [NSString stringWithFormat:NSLocalizedString(@"This file has conflicts (%@)", nil), status];
-      view.openButton.tag = (uintptr_t)data;
-      view.mergeButton.tag = (uintptr_t)data;
-      view.resolveButton.tag = (uintptr_t)data;
-      return view;
+      if (data.conflict.ancestorFileMode == kGCFileMode_Commit) {
+        // Submodule conflict
+        GISubmoduleConflictDiffCellView* view = [_tableView makeViewWithIdentifier:@"submodule_conflict" owner:self];
+        view.statusTextField.stringValue = [NSString stringWithFormat:NSLocalizedString(@"This submodule has conflicts (%@)", nil), status];
+        view.oursTextField.stringValue = data.conflict.ourBlobSHA1;
+        view.theirsTextField.stringValue = data.conflict.theirBlobSHA1;
+        view.chooseOursButton.tag = (uintptr_t)data;
+        view.chooseTheirsButton.tag = (uintptr_t)data;
+        return view;
+      } else {
+        GIConflictDiffCellView* view = [_tableView makeViewWithIdentifier:@"conflict" owner:self];
+        view.statusTextField.stringValue = [NSString stringWithFormat:NSLocalizedString(@"This file has conflicts (%@)", nil), status];
+        view.openButton.tag = (uintptr_t)data;
+        view.mergeButton.tag = (uintptr_t)data;
+        view.resolveButton.tag = (uintptr_t)data;
+        return view;
+      }
     } else if (GC_FILE_MODE_IS_SUBMODULE(delta.oldFile.mode) || GC_FILE_MODE_IS_SUBMODULE(delta.newFile.mode)) {
       GISubmoduleDiffCellView* view = [_tableView makeViewWithIdentifier:@"submodule" owner:self];
       NSString* oldSHA1 = delta.oldFile ? delta.oldFile.SHA1 : nil;
@@ -660,6 +684,8 @@ - (CGFloat)tableView:(NSTableView*)tableView heightOfRow:(NSInteger)row {
       return [data.imageDiffView desiredHeightForWidth:[_tableView.tableColumns[0] width]];
     } else if (data.empty) {
       return _emptyViewHeight;
+    } else if (data.conflict && data.conflict.ancestorFileMode == kGCFileMode_Commit) {
+      return _submoduleConflictViewHeight;
     } else if (data.conflict) {
       return _conflictViewHeight;
     } else if (GC_FILE_MODE_IS_SUBMODULE(delta.oldFile.mode) || GC_FILE_MODE_IS_SUBMODULE(delta.newFile.mode)) {
@@ -730,4 +756,35 @@ - (IBAction)markAsResolved:(id)sender {
   [self markConflictAsResolved:data.conflict];
 }
 
+- (IBAction)chooseOurs:(id)sender {
+  GIDiffContentData* data = (__bridge GIDiffContentData*)(void*)[(NSButton*)sender tag];
+  NSError *error;
+
+  [self.repository updateSubmoduleReferenceAtPath:data.conflict.path toCommitSHA1:data.conflict.ourBlobSHA1 error:&error];
+
+  if (!error) {
+    [self markConflictAsResolved:data.conflict];
+  } else {
+    [self presentError:error];
+  }
+
+  [self.repository notifyWorkingDirectoryChanged];
+}
+
+- (IBAction)chooseTheirs:(id)sender {
+  GIDiffContentData* data = (__bridge GIDiffContentData*)(void*)[(NSButton*)sender tag];
+  NSError *error;
+
+
+  [self.repository updateSubmoduleReferenceAtPath:data.conflict.path toCommitSHA1:data.conflict.theirBlobSHA1 error:&error];
+
+  if (!error) {
+    [self markConflictAsResolved:data.conflict];
+  } else {
+    [self presentError:error];
+  }
+
+  [self.repository notifyWorkingDirectoryChanged];
+}
+
 @end

GitUpKit/Core/GCDiff.m

@@ -239,14 +239,14 @@ - (BOOL)isSubmodule {
     case kGCFileDiffChange_Ignored:
     case kGCFileDiffChange_Untracked:
     case kGCFileDiffChange_Unreadable:
+    case kGCFileDiffChange_Conflicted:
       return GC_FILE_MODE_IS_SUBMODULE(_oldFile.mode);
 
     case kGCFileDiffChange_Added:
     case kGCFileDiffChange_Modified:
     case kGCFileDiffChange_Renamed:
     case kGCFileDiffChange_Copied:
     case kGCFileDiffChange_TypeChanged:
-    case kGCFileDiffChange_Conflicted:
       return GC_FILE_MODE_IS_SUBMODULE(_newFile.mode);
   }
   XLOG_DEBUG_UNREACHABLE();
@@ -354,6 +354,27 @@ - (void)_cacheDeltasIfNeeded {
         XLOG_DEBUG_UNREACHABLE();
       }
     }
+
+    // Remove superfluous "untracked" deltas for conflicting submodules
+    // Needed when the input _deltas looks like this:
+    // 1: [Conflicted] "submodule"
+    // 2: [Untracked] "submodule/"
+    // Which happens every time there's a submodule entry that's a conflict
+    NSMutableArray<GCDiffDelta *> *deltasToFilterOut = [NSMutableArray array];
+    for (GCDiffDelta* delta in _deltas) {
+      if (delta.change == kGCFileDiffChange_Conflicted && delta.isSubmodule) {
+        // see if there's a superfluous untracked diff for that submodule and remove it
+        NSString *pathWithTrailingSlash = [NSString stringWithFormat:@"%@/", delta.canonicalPath];
+        for (GCDiffDelta* delta in _deltas) {
+          if (delta.isSubmodule && [delta.canonicalPath isEqualToString:pathWithTrailingSlash]) {
+            [deltasToFilterOut addObject:delta];
+            break; // there's only one so we can break out early if we've found it
+          }
+        }
+      }
+    }
+
+    [_deltas removeObjectsInArray:deltasToFilterOut];
   }
 }
 

GitUpKit/Core/GCIndex.m

@@ -39,11 +39,11 @@ - (id)initWithAncestor:(const git_index_entry*)ancestor our:(const git_index_ent
       _status = ancestor ? kGCIndexConflictStatus_BothModified : kGCIndexConflictStatus_BothAdded;
 
       git_oid_cpy(&_ourOID, &our->id);
-      XLOG_DEBUG_CHECK((our->mode == GIT_FILEMODE_BLOB) || (our->mode == GIT_FILEMODE_BLOB_EXECUTABLE) || (our->mode == GIT_FILEMODE_LINK));
+      XLOG_DEBUG_CHECK((our->mode == GIT_FILEMODE_BLOB) || (our->mode == GIT_FILEMODE_BLOB_EXECUTABLE) || (our->mode == GIT_FILEMODE_LINK) || (our->mode == GIT_FILEMODE_COMMIT));
       _ourFileMode = GCFileModeFromMode(our->mode);
 
       git_oid_cpy(&_theirOID, &their->id);
-      XLOG_DEBUG_CHECK((their->mode == GIT_FILEMODE_BLOB) || (their->mode == GIT_FILEMODE_BLOB_EXECUTABLE) || (their->mode == GIT_FILEMODE_LINK));
+      XLOG_DEBUG_CHECK((their->mode == GIT_FILEMODE_BLOB) || (their->mode == GIT_FILEMODE_BLOB_EXECUTABLE) || (their->mode == GIT_FILEMODE_LINK) || (their->mode == GIT_FILEMODE_COMMIT));
       _theirFileMode = GCFileModeFromMode(their->mode);
     } else if (our) {
       XLOG_DEBUG_CHECK(!strcmp(our->path, ancestor->path));
@@ -64,7 +64,7 @@ - (id)initWithAncestor:(const git_index_entry*)ancestor our:(const git_index_ent
     }
     if (ancestor) {
       git_oid_cpy(&_ancestorOID, &ancestor->id);
-      XLOG_DEBUG_CHECK((ancestor->mode == GIT_FILEMODE_BLOB) || (ancestor->mode == GIT_FILEMODE_BLOB_EXECUTABLE) || (ancestor->mode == GIT_FILEMODE_LINK));
+      XLOG_DEBUG_CHECK((ancestor->mode == GIT_FILEMODE_BLOB) || (ancestor->mode == GIT_FILEMODE_BLOB_EXECUTABLE) || (ancestor->mode == GIT_FILEMODE_LINK) || (ancestor->mode == GIT_FILEMODE_COMMIT));
       _ancestorFileMode = GCFileModeFromMode(ancestor->mode);
     }
     if (our) {
@@ -301,6 +301,16 @@ - (BOOL)_addEntry:(const git_index_entry*)entry toIndex:(git_index*)index error:
   return YES;
 }
 
+// This function adapts to handle submodules by directly using the commit OID and setting the correct file mode for submodules.
+- (BOOL)_addSubmoduleEntry:(const git_index_entry*)entry toIndex:(git_index*)index withCommitOid:(const git_oid *)commitOid error:(NSError**)error {
+  git_index_entry copyEntry;
+  bcopy(entry, &copyEntry, sizeof(git_index_entry));
+  git_oid_cpy(&copyEntry.id, commitOid);
+  copyEntry.mode = GIT_FILEMODE_COMMIT;
+  CALL_LIBGIT2_FUNCTION_RETURN(NO, git_index_add, index, &copyEntry);
+  return YES;
+}
+
 - (BOOL)addFile:(NSString*)path withContents:(NSData*)contents toIndex:(GCIndex*)index error:(NSError**)error {
   git_index_entry entry;
   bzero(&entry, sizeof(git_index_entry));
@@ -316,7 +326,32 @@ - (BOOL)addFileInWorkingDirectory:(NSString*)path toIndex:(GCIndex*)index error:
   bzero(&entry, sizeof(git_index_entry));
   entry.path = GCGitPathFromFileSystemPath(path);
   git_index_entry__init_from_stat(&entry, &info, true);
-  return [self _addEntry:&entry toIndex:index.private error:error];
+
+  if (entry.mode == GIT_FILEMODE_COMMIT) {
+    GCSubmodule *submodule = [self lookupSubmoduleWithName:path error:error];
+    if (!submodule) {
+      return NO;
+    }
+
+    GCRepository *submoduleRepository = [[GCRepository alloc] initWithSubmodule:submodule error:error];
+    if (!submoduleRepository) {
+      return NO;
+    }
+
+    GCCommit *headCommit;
+    if (![submoduleRepository lookupHEADCurrentCommit:&headCommit branch:NULL error:error]) {
+      return NO;
+    }
+
+    git_oid oid;
+    if (!GCGitOIDFromSHA1(headCommit.SHA1, &oid, error)) {
+      return NO;
+    }
+
+    return [self _addSubmoduleEntry:&entry toIndex:index.private withCommitOid:&oid error:error];
+  } else {
+    return [self _addEntry:&entry toIndex:index.private error:error];
+  }
 }
 
 - (BOOL)addLinesInWorkingDirectoryFile:(NSString*)path toIndex:(GCIndex*)index error:(NSError**)error usingFilter:(GCIndexLineFilter)filter {

GitUpKit/Core/GCRepository+HEAD.h

@@ -32,6 +32,7 @@ typedef NS_OPTIONS(NSUInteger, GCCheckoutOptions) {
 - (BOOL)setDetachedHEADToCommit:(GCCommit*)commit error:(NSError**)error;  // git update-ref HEAD {commit}
 
 - (BOOL)moveHEADToCommit:(GCCommit*)commit reflogMessage:(NSString*)message error:(NSError**)error;  // git reset --soft {commit} (but with custom reflog message)
+- (BOOL)updateSubmoduleReferenceAtPath:(NSString *)submodulePath toCommitSHA1:(NSString *)commitSHA1 error:(NSError **)error;
 
 - (BOOL)checkoutCommit:(GCCommit*)commit options:(GCCheckoutOptions)options error:(NSError**)error;  // git checkout {commit}
 - (BOOL)checkoutLocalBranch:(GCLocalBranch*)branch options:(GCCheckoutOptions)options error:(NSError**)error;  // git checkout {branch}

GitUpKit/Core/GCRepository+HEAD.m

@@ -221,6 +221,25 @@ - (BOOL)checkoutCommit:(GCCommit*)commit options:(GCCheckoutOptions)options erro
   return YES;
 }
 
+- (BOOL)updateSubmoduleReferenceAtPath:(NSString*)submodulePath toCommitSHA1:(NSString*)commitSHA1 error:(NSError**)error {
+  GCSubmodule *submodule = [self lookupSubmoduleWithName:submodulePath error:error];
+  if (!submodule) {
+    return NO;
+  }
+
+  GCRepository *submoduleRepository = [[GCRepository alloc] initWithSubmodule:submodule error:error];
+  if (!submoduleRepository) {
+    return NO;
+  }
+
+  GCCommit *targetCommit = [submoduleRepository findCommitWithSHA1:commitSHA1 error:error];
+  if (!targetCommit) {
+    return NO;
+  }
+
+  return [submoduleRepository checkoutCommit:targetCommit options:kGCCheckoutOption_UpdateSubmodulesRecursively error:error];
+}
+
 // Because by default git_checkout_tree() assumes the baseline (i.e. expected content of workdir) is HEAD we must checkout first, then update HEAD
 - (BOOL)checkoutLocalBranch:(GCLocalBranch*)branch options:(GCCheckoutOptions)options error:(NSError**)error {
   GCCommit* tipCommit = [self lookupTipCommitForBranch:branch error:error];

GitUpKit/Core/GCSubmodule.m

@@ -310,7 +310,23 @@ - (BOOL)updateAllSubmodulesResursively:(BOOL)force error:(NSError**)error {
   if (submodules == nil) {
     return NO;
   }
+
+  NSArray<NSString *> *conflictPaths = @[];
+
+  git_index* index = [self reloadRepositoryIndex:error];
+
+  if (index && git_index_has_conflicts(index)) {
+    conflictPaths = [self checkConflicts:nil].allKeys;
+  }
+
+  git_index_free(index);
+
   for (GCSubmodule* submodule in submodules) {
+    if ([conflictPaths containsObject:submodule.path]) {
+      // conflict needs to be resolved first, will be handled elsewhere but we shouldn't return an error
+      continue;
+    }
+
     NSError* localError;
     if (![self updateSubmodule:submodule force:force error:&localError]) {
       if ([localError.domain isEqualToString:GCErrorDomain] && (localError.code == kGCErrorCode_NotFound)) {