Faster multiple file staging (#629)

lapfelix · lucasderraugh · commit 3f5abaf6f724 · 2020-01-12T10:03:32.000-08:00
* Improve performance when staging/unstaging multiple files at once

* Optimize multiple files discarding

* Run ClangFormat

* Changes following PR feedback

* Fix bug caused by bad merge and add comments

The selectedFiles array would always be empty because the `else` should've been outside the `if (submodule)`

* Correctly allocate and free multiple path strings
diff --git a/GitUpKit/Core/GCDiff-Tests.m b/GitUpKit/Core/GCDiff-Tests.m
@@ -165,6 +165,16 @@ - (void)testUnifiedDiff {
   XCTAssertTrue([self.repository addFileToIndex:@"renamed1.txt" error:NULL]);
   [self updateFileAtPath:@"type-changed.txt" withString:@""];
   XCTAssertTrue([self.repository addFileToIndex:@"type-changed.txt" error:NULL]);
+
+  NSArray* files = @[ @".gitignore", @"modified.txt", @"deleted.txt", @"renamed1.txt", @"type-changed.txt" ];
+
+  // Test adding and removing multiple files
+  XCTAssertTrue([self.repository removeFilesFromIndex:files error:NULL]);
+  XCTAssertEqual([self.repository checkIndexStatus:NULL].deltas.count, 0);
+
+  XCTAssertTrue([self.repository addFilesToIndex:files error:NULL]);
+  XCTAssertEqual([self.repository checkIndexStatus:NULL].deltas.count, 5);
+
   XCTAssertNotNil([self.repository createCommitFromHEADWithMessage:@"Update" error:NULL]);
 
   // Touch files
@@ -179,8 +189,8 @@ - (void)testUnifiedDiff {
 
   // Stage some files
   XCTAssertTrue([self.repository addFileToIndex:@"modified.txt" error:NULL]);
-  XCTAssertTrue([self.repository removeFileFromIndex:@"deleted.txt" error:NULL]);
-  XCTAssertTrue([self.repository removeFileFromIndex:@"renamed1.txt" error:NULL]);
+  files = @[ @"deleted.txt", @"renamed1.txt" ];
+  XCTAssertTrue([self.repository removeFilesFromIndex:files error:NULL]);
   XCTAssertTrue([self.repository addFileToIndex:@"renamed2.txt" error:NULL]);
   XCTAssertTrue([self.repository addFileToIndex:@"added.txt" error:NULL]);
 
diff --git a/GitUpKit/Core/GCIndex.h b/GitUpKit/Core/GCIndex.h
@@ -66,6 +66,7 @@ typedef BOOL (^GCIndexLineFilter)(GCLineDiffChange change, NSUInteger oldLineNum
 - (BOOL)resetLinesInFile:(NSString*)path index:(GCIndex*)index toCommit:(GCCommit*)commit error:(NSError**)error usingFilter:(GCIndexLineFilter)filter;
 
 - (BOOL)checkoutFileToWorkingDirectory:(NSString*)path fromIndex:(GCIndex*)index error:(NSError**)error;
+- (BOOL)checkoutFilesToWorkingDirectory:(NSArray<NSString*>*)paths fromIndex:(GCIndex*)index error:(NSError**)error;
 - (BOOL)checkoutLinesInFileToWorkingDirectory:(NSString*)path fromIndex:(GCIndex*)index error:(NSError**)error usingFilter:(GCIndexLineFilter)filter;
 
 - (BOOL)clearConflictForFile:(NSString*)path inIndex:(GCIndex*)index error:(NSError**)error;
diff --git a/GitUpKit/Core/GCIndex.m b/GitUpKit/Core/GCIndex.m
@@ -485,12 +485,24 @@ - (BOOL)resetLinesInFile:(NSString*)path index:(GCIndex*)index toCommit:(GCCommi
 }
 
 - (BOOL)checkoutFileToWorkingDirectory:(NSString*)path fromIndex:(GCIndex*)index error:(NSError**)error {
+  return [self checkoutFilesToWorkingDirectory:@[ path ]
+                                     fromIndex:index
+                                         error:error];
+}
+
+- (BOOL)checkoutFilesToWorkingDirectory:(NSArray<NSString*>*)paths fromIndex:(GCIndex*)index error:(NSError**)error {
   git_checkout_options options = GIT_CHECKOUT_OPTIONS_INIT;
   options.checkout_strategy = GIT_CHECKOUT_FORCE | GIT_CHECKOUT_DONT_UPDATE_INDEX | GIT_CHECKOUT_DISABLE_PATHSPEC_MATCH;  // There's no reason to update the index
-  options.paths.count = 1;
-  const char* filePath = GCGitPathFromFileSystemPath(path);
-  options.paths.strings = (char**)&filePath;
+  options.paths.count = paths.count;
+  char** pathStrings = malloc(paths.count * sizeof(char*));
+  options.paths.strings = pathStrings;
+  for (NSUInteger i = 0; i < paths.count; i++) {
+    const char* filePath = GCGitPathFromFileSystemPath(paths[i]);
+    options.paths.strings[i] = (char*)filePath;
+  }
+
   CALL_LIBGIT2_FUNCTION_RETURN(NO, git_checkout_index, self.private, index.private, &options);
+  free(pathStrings);
   return YES;
 }
 
diff --git a/GitUpKit/Extensions/GCRepository+Index-Tests.m b/GitUpKit/Extensions/GCRepository+Index-Tests.m
@@ -69,9 +69,33 @@ - (void)testIndex {
   XCTAssertTrue([self.repository addFileToIndex:@"test.txt" error:NULL]);
   [self assertGitCLTOutputEqualsString:@"A  test.txt\n" withRepository:self.repository command:@"status", @"--ignored", @"--porcelain", nil];
 
+  // Add multiple files to working directory
+  NSMutableArray* filePaths = [[NSMutableArray alloc] init];
+  NSString* expectedGitCLTOutput = [[NSString alloc] init];
+  for (int i = 0; i < 50; i++) {
+    NSString* filePath = [NSString stringWithFormat:@"hello_world%02d.txt", i];
+    [self updateFileAtPath:filePath withString:@"Bonjour le monde!\n"];
+    [filePaths addObject:filePath];
+    expectedGitCLTOutput = [expectedGitCLTOutput stringByAppendingFormat:@"A  %@\n", filePath];
+  }
+  expectedGitCLTOutput = [expectedGitCLTOutput stringByAppendingString:@"A  test.txt\n"];
+
+  // Add multiple files to index
+  XCTAssertTrue([self.repository addFilesToIndex:filePaths error:NULL]);
+  [self assertGitCLTOutputEqualsString:expectedGitCLTOutput withRepository:self.repository command:@"status", @"--ignored", @"--porcelain", nil];
+
+  // Add remove multiple files from index
+  XCTAssertTrue([self.repository removeFilesFromIndex:filePaths error:NULL]);
+  expectedGitCLTOutput = [expectedGitCLTOutput stringByReplacingOccurrencesOfString:@"A  test.txt\n" withString:@""];
+  expectedGitCLTOutput = [expectedGitCLTOutput stringByReplacingOccurrencesOfString:@"A  hello_world" withString:@"?? hello_world"];
+  expectedGitCLTOutput = [@"A  test.txt\n" stringByAppendingString:expectedGitCLTOutput];
+  [self assertGitCLTOutputEqualsString:expectedGitCLTOutput withRepository:self.repository command:@"status", @"--ignored", @"--porcelain", nil];
+
   // Reset index
   XCTAssertTrue([self.repository resetIndexToHEAD:NULL]);
-  [self assertGitCLTOutputEqualsString:@"?? test.txt\n" withRepository:self.repository command:@"status", @"--ignored", @"--porcelain", nil];
+  expectedGitCLTOutput = [expectedGitCLTOutput stringByReplacingOccurrencesOfString:@"A  test.txt\n" withString:@""];
+  expectedGitCLTOutput = [expectedGitCLTOutput stringByAppendingString:@"?? test.txt\n"];
+  [self assertGitCLTOutputEqualsString:expectedGitCLTOutput withRepository:self.repository command:@"status", @"--ignored", @"--porcelain", nil];
 }
 
 - (void)testIndex_Lines {
diff --git a/GitUpKit/Extensions/GCRepository+Index.h b/GitUpKit/Extensions/GCRepository+Index.h
@@ -19,10 +19,14 @@
 - (BOOL)resetIndexToHEAD:(NSError**)error;  // Like git reset --mixed HEAD but does not update reflog
 
 - (BOOL)removeFileFromIndex:(NSString*)path error:(NSError**)error;  // git rm --cached {file} - Delete file from index
+- (BOOL)removeFilesFromIndex:(NSArray<NSString*>*)paths error:(NSError**)error;  // git rm --cached {file} - Delete files from index
 
 - (BOOL)addFileToIndex:(NSString*)path error:(NSError**)error;  // git add {file} - Copy file from workdir to index (aka stage file)
+- (BOOL)addFilesToIndex:(NSArray<NSString*>*)paths error:(NSError**)error;
 - (BOOL)resetFileInIndexToHEAD:(NSString*)path error:(NSError**)error;  // git reset --mixed {file} - Copy file from HEAD to index (aka unstage file)
+- (BOOL)resetFilesInIndexToHEAD:(NSArray<NSString*>*)paths error:(NSError**)error;
 - (BOOL)checkoutFileFromIndex:(NSString*)path error:(NSError**)error;  // git checkout {file} - Copy file from index to workdir (aka discard file)
+- (BOOL)checkoutFilesFromIndex:(NSArray<NSString*>*)paths error:(NSError**)error;
 
 - (BOOL)addLinesFromFileToIndex:(NSString*)path error:(NSError**)error usingFilter:(GCIndexLineFilter)filter;  // git add -p {file} - Copy only some lines of file from workdir to index (aka stage lines)
 - (BOOL)resetLinesFromFileInIndexToHEAD:(NSString*)path error:(NSError**)error usingFilter:(GCIndexLineFilter)filter;  // git reset -p {file} - Copy only some lines of file from HEAD to index (aka unstage lines)
diff --git a/GitUpKit/Extensions/GCRepository+Index.m b/GitUpKit/Extensions/GCRepository+Index.m
@@ -39,22 +39,61 @@ - (BOOL)resetIndexToHEAD:(NSError**)error {
 }
 
 - (BOOL)removeFileFromIndex:(NSString*)path error:(NSError**)error {
+  return [self removeFilesFromIndex:@[ path ] error:error];
+}
+
+- (BOOL)removeFilesFromIndex:(NSArray<NSString*>*)paths error:(NSError**)error {
   GCIndex* index = [self readRepositoryIndex:error];
   if (index == nil) {
     return NO;
   }
-  return [self removeFile:path fromIndex:index error:error] && [self writeRepositoryIndex:index error:error];
+
+  for (NSString* path in paths) {
+    if (![self removeFile:path fromIndex:index error:error] || (error && *error != nil)) {
+      [self writeRepositoryIndex:index error:error];
+      return NO;
+    }
+  }
+
+  return [self writeRepositoryIndex:index error:error];
 }
 
 - (BOOL)addFileToIndex:(NSString*)path error:(NSError**)error {
+  return [self addFilesToIndex:@[ path ] error:error];
+}
+
+- (BOOL)addFilesToIndex:(NSArray<NSString*>*)paths error:(NSError**)error {
   GCIndex* index = [self readRepositoryIndex:error];
   if (index == nil) {
     return NO;
   }
-  return [self addFileInWorkingDirectory:path toIndex:index error:error] && [self writeRepositoryIndex:index error:error];
+
+  BOOL failed = NO;
+  BOOL shouldWriteRepository = NO;
+  for (NSString* path in paths) {
+    if (![self addFileInWorkingDirectory:path toIndex:index error:error] || (error && *error != nil)) {
+      failed = YES;
+      break;
+    }
+
+    shouldWriteRepository = YES;
+  }
+
+  if (failed && shouldWriteRepository) {
+    if (shouldWriteRepository) {
+      [self writeRepositoryIndex:index error:NULL];
+    }
+    return NO;
+  }
+
+  return [self writeRepositoryIndex:index error:error];
 }
 
 - (BOOL)resetFileInIndexToHEAD:(NSString*)path error:(NSError**)error {
+  return [self resetFilesInIndexToHEAD:@[ path ] error:error];
+}
+
+- (BOOL)resetFilesInIndexToHEAD:(NSArray<NSString*>*)paths error:(NSError**)error {
   GCIndex* index = [self readRepositoryIndex:error];
   if (index == nil) {
     return NO;
@@ -63,24 +102,34 @@ - (BOOL)resetFileInIndexToHEAD:(NSString*)path error:(NSError**)error {
   if (![self lookupHEADCurrentCommit:&headCommit branch:NULL error:error]) {
     return NO;
   }
-  if (headCommit) {
-    if (![self resetFile:path inIndex:index toCommit:headCommit error:error]) {
-      return NO;
-    }
-  } else {
-    if (![self removeFile:path fromIndex:index error:error]) {
-      return NO;
+
+  for (NSString* path in paths) {
+    if (headCommit) {
+      if (![self resetFile:path inIndex:index toCommit:headCommit error:error]) {
+        [self writeRepositoryIndex:index error:error];
+        return NO;
+      }
+    } else {
+      if (![self removeFile:path fromIndex:index error:error]) {
+        [self writeRepositoryIndex:index error:error];
+        return NO;
+      }
     }
   }
+
   return [self writeRepositoryIndex:index error:error];
 }
 
 - (BOOL)checkoutFileFromIndex:(NSString*)path error:(NSError**)error {
+  return [self checkoutFilesFromIndex:@[ path ] error:error];
+}
+
+- (BOOL)checkoutFilesFromIndex:(NSArray<NSString*>*)paths error:(NSError**)error {
   GCIndex* index = [self readRepositoryIndex:error];
   if (index == nil) {
     return NO;
   }
-  return [self checkoutFileToWorkingDirectory:path fromIndex:index error:error];
+  return [self checkoutFilesToWorkingDirectory:paths fromIndex:index error:error];
 }
 
 - (BOOL)addLinesFromFileToIndex:(NSString*)path error:(NSError**)error usingFilter:(GCIndexLineFilter)filter {
diff --git a/GitUpKit/Utilities/GIViewController+Utilities.h b/GitUpKit/Utilities/GIViewController+Utilities.h
@@ -48,11 +48,14 @@ extern NSString* const GIViewController_TerminalTool_iTerm;
 - (void)discardSubmoduleAtPath:(NSString*)path resetIndex:(BOOL)resetIndex;  // Prompts user
 
 - (void)stageAllChangesForFile:(NSString*)path;
+- (void)stageAllChangesForFiles:(NSArray<NSString*>*)paths;
 - (void)stageSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldLines newLines:(NSIndexSet*)newLines;
 - (void)unstageAllChangesForFile:(NSString*)path;
+- (void)unstageAllChangesForFiles:(NSArray<NSString*>*)paths;
 - (void)unstageSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldLines newLines:(NSIndexSet*)newLines;
 
 - (BOOL)discardAllChangesForFile:(NSString*)path resetIndex:(BOOL)resetIndex error:(NSError**)error;
+- (BOOL)discardAllChangesForFiles:(NSArray<NSString*>*)paths resetIndex:(BOOL)resetIndex error:(NSError**)error;
 - (void)discardAllChangesForFile:(NSString*)path resetIndex:(BOOL)resetIndex;  // Prompts user
 - (BOOL)discardSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldLines newLines:(NSIndexSet*)newLines resetIndex:(BOOL)resetIndex error:(NSError**)error;
 - (void)discardSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldLines newLines:(NSIndexSet*)newLines resetIndex:(BOOL)resetIndex;  // Prompts user
diff --git a/GitUpKit/Utilities/GIViewController+Utilities.m b/GitUpKit/Utilities/GIViewController+Utilities.m
@@ -111,7 +111,7 @@ + (void)initialize {
     GIViewController_TerminalTool : GIViewController_TerminalTool_Terminal,
   };
   [[NSUserDefaults standardUserDefaults] registerDefaults:defaults];
-  
+
   NSDictionary* installedApps = [GILaunchServicesLocator installedAppsDictionary];
   [[NSUserDefaults standardUserDefaults] registerDefaults:installedApps];
 
@@ -198,13 +198,34 @@ - (void)discardSubmoduleAtPath:(NSString*)path resetIndex:(BOOL)resetIndex {
 }
 
 - (void)stageAllChangesForFile:(NSString*)path {
+  return [self stageAllChangesForFiles:@[ path ]];
+}
+
+- (void)stageAllChangesForFiles:(NSArray<NSString*>*)paths {
   NSError* error;
-  BOOL fileExists = [[NSFileManager defaultManager] fileExistsAtPath:[self.repository absolutePathForFile:path] followLastSymlink:NO];
-  if ((fileExists && [self.repository addFileToIndex:path error:&error]) || (!fileExists && [self.repository removeFileFromIndex:path error:&error])) {
-    [self.repository notifyRepositoryChanged];
-  } else {
-    [self presentError:error];
+  NSMutableArray* existingFiles = [NSMutableArray array];
+  NSMutableArray* nonExistingFiles = [NSMutableArray array];
+  for (NSString* path in paths) {
+    if ([[NSFileManager defaultManager] fileExistsAtPath:[self.repository absolutePathForFile:path]]) {
+      [existingFiles addObject:path];
+    } else {
+      [nonExistingFiles addObject:path];
+    }
+  }
+
+  if (existingFiles.count > 0) {
+    if (![self.repository addFilesToIndex:existingFiles error:&error]) {
+      [self presentError:error];
+    }
+  }
+
+  if (nonExistingFiles.count > 0) {
+    if (![self.repository removeFilesFromIndex:nonExistingFiles error:&error]) {
+      [self presentError:error];
+    }
   }
+
+  [self.repository notifyRepositoryChanged];
 }
 
 - (void)stageSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldLines newLines:(NSIndexSet*)newLines {
@@ -227,8 +248,12 @@ - (void)stageSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldLin
 }
 
 - (void)unstageAllChangesForFile:(NSString*)path {
+  [self unstageAllChangesForFiles:@[ path ]];
+}
+
+- (void)unstageAllChangesForFiles:(NSArray<NSString*>*)paths {
   NSError* error;
-  if ([self.repository resetFileInIndexToHEAD:path error:&error]) {
+  if ([self.repository resetFilesInIndexToHEAD:paths error:&error]) {
     [self.repository notifyWorkingDirectoryChanged];
   } else {
     [self presentError:error];
@@ -255,18 +280,36 @@ - (void)unstageSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldL
 }
 
 - (BOOL)discardAllChangesForFile:(NSString*)path resetIndex:(BOOL)resetIndex error:(NSError**)error {
+  return [self discardAllChangesForFiles:@[ path ]
+                              resetIndex:resetIndex
+                                   error:error];
+}
+
+- (BOOL)discardAllChangesForFiles:(NSArray<NSString*>*)paths resetIndex:(BOOL)resetIndex error:(NSError**)error {
   BOOL success = NO;
   if (resetIndex) {
     GCCommit* commit;
-    if ([self.repository lookupHEADCurrentCommit:&commit branch:NULL error:error] && [self.repository resetFileInIndexToHEAD:path error:error]) {
-      if (commit && [self.repository checkTreeForCommit:commit containsFile:path error:NULL]) {
-        success = [self.repository safeDeleteFileIfExists:path error:error] && [self.repository checkoutFileFromIndex:path error:error];
-      } else {
-        success = [self.repository safeDeleteFile:path error:error];
+    if ([self.repository lookupHEADCurrentCommit:&commit branch:NULL error:error] && [self.repository resetFilesInIndexToHEAD:paths error:error]) {
+      success = YES;
+      for (NSString* path in paths) {
+        if (commit && [self.repository checkTreeForCommit:commit containsFile:path error:NULL]) {
+          if (![self.repository safeDeleteFileIfExists:path error:error] && [self.repository checkoutFileFromIndex:path error:error]) {
+            return NO;
+          }
+        } else {
+          if (![self.repository safeDeleteFileIfExists:path error:error]) {
+            return NO;
+          }
+        }
       }
     }
   } else {
-    success = [self.repository safeDeleteFileIfExists:path error:error] && [self.repository checkoutFileFromIndex:path error:error];
+    for (NSString* path in paths) {
+      if (![self.repository safeDeleteFileIfExists:path error:error]) {
+        return NO;
+      }
+    }
+    success = [self.repository checkoutFilesFromIndex:paths error:error];
   }
   return success;
 }
@@ -922,7 +965,7 @@ - (void)launchDiffToolWithCommit:(GCCommit*)commit otherCommit:(GCCommit*)otherC
   } else if ([identifier isEqualToString:GIViewControllerTool_BeyondCompare]) {
     [self _runBeyondCompareWithArguments:@[ [NSString stringWithFormat:@"-title1=%@", oldTitle], [NSString stringWithFormat:@"-title2=%@", newTitle], oldPath, newPath ]];
   } else if ([identifier isEqualToString:GIViewControllerTool_P4Merge] || [identifier isEqualToString:GIViewControllerTool_GitTool]) {
-    ;  // Handled above
+    // Handled above
   } else if ([identifier isEqualToString:GIViewControllerTool_DiffMerge]) {
     [self _runDiffMergeToolWithArguments:@[ [NSString stringWithFormat:@"-t1=%@", oldTitle], [NSString stringWithFormat:@"-t2=%@", newTitle], oldPath, newPath ]];
   } else {
diff --git a/GitUpKit/Views/GIAdvancedCommitViewController.m b/GitUpKit/Views/GIAdvancedCommitViewController.m
