From 06b1be6735a555c5d9277f00f08ff58362669a4e Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Fri, 16 Oct 2026 13:58:04 +0100 Subject: [PATCH 1/4] Find git-lfs in the user's PATH instead of hardcoding /usr/local/bin The LFS filter was only registered if git-lfs existed at /usr/local/bin/git-lfs, so with Homebrew on Apple Silicon (/opt/homebrew/bin) or MacPorts (/opt/local/bin) LFS files were silently left unfiltered (#805). The filter is now always registered and git-lfs is resolved lazily, the first time a file with the lfs filter attribute is processed, from the login shell PATH with fallbacks to the common install locations. If git-lfs isn't found the filter passes through, preserving the previous behavior. git-lfs is also run with the login shell PATH so it uses the same git as the command line. Co-Authored-By: Claude Opus 5.5 --- GitUpKit/Core/GCRepository-Tests.m | 27 +++++++++ GitUpKit/Core/GCRepository.m | 94 ++++++++++++++++++++++-------- 2 files changed, 98 insertions(+), 23 deletions(-) diff --git a/GitUpKit/Core/GCRepository-Tests.m b/GitUpKit/Core/GCRepository-Tests.m index b96a0aff..9b380888 100644 --- a/GitUpKit/Core/GCRepository-Tests.m +++ b/GitUpKit/Core/GCRepository-Tests.m @@ -284,4 +284,31 @@ - (void)testPathForHookWithName_FoundFromLinkedWorktree { [[NSFileManager defaultManager] removeItemAtPath:worktreePath error:NULL]; } +// The git-lfs tool used to be expected at /usr/local/bin/git-lfs only, so LFS files were +// silently left unfiltered when it was installed elsewhere e.g. /opt/homebrew/bin (issue #805). +- (void)testGitLFSCleanFilter { + NSString* shell = NSProcessInfo.processInfo.environment[@"SHELL"] ?: @"/bin/sh"; + NSString* PATH = [self.repository getPATHUsingShell:shell error:NULL]; + BOOL found = NO; + for (NSString* directory in [[PATH componentsSeparatedByString:@":"] arrayByAddingObjectsFromArray:@[ @"/opt/homebrew/bin", @"/usr/local/bin", @"/opt/local/bin" ]]) { + if (directory.length && [[NSFileManager defaultManager] isExecutableFileAtPath:[directory stringByAppendingPathComponent:@"git-lfs"]]) { + found = YES; + break; + } + } + if (!found) { + XCTSkip(@"git-lfs is not installed"); + } + + [self updateFileAtPath:@".gitattributes" withString:@"*.bin filter=lfs diff=lfs merge=lfs -text\n"]; + [self updateFileAtPath:@"large.bin" withString:@"Hello LFS\n"]; + + GCIndex* index = [self.repository readRepositoryIndex:NULL]; + XCTAssertNotNil(index); + XCTAssertTrue([self.repository addFileInWorkingDirectory:@"large.bin" toIndex:index error:NULL]); + NSData* data = [self.repository readContentsForFile:@"large.bin" inIndex:index error:NULL]; + NSString* contents = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; + XCTAssertTrue([contents hasPrefix:@"version https://git-lfs.github.com/spec/v1\n"], @"%@", contents); +} + @end diff --git a/GitUpKit/Core/GCRepository.m b/GitUpKit/Core/GCRepository.m index 8ee27942..26bc6a42 100644 --- a/GitUpKit/Core/GCRepository.m +++ b/GitUpKit/Core/GCRepository.m @@ -21,15 +21,10 @@ #import #if !TARGET_OS_IPHONE #import -#import #endif #import "GCPrivate.h" -#if !TARGET_OS_IPHONE -static const char* _GitLFSPath = "/usr/local/bin/git-lfs"; -#endif - static inline BOOL _IsDirectoryWritable(const char* path) { int status = access(path, W_OK); if (status == 0) { @@ -56,6 +51,61 @@ static inline BOOL _IsDirectoryWritable(const char* path) { #if !TARGET_OS_IPHONE +static NSString* _GetPATHUsingShell(NSString* shell, NSError** error) { + GCTask* task = [[GCTask alloc] initWithExecutablePath:shell]; + NSData* data; + // `-l` is not supported with `-c` in all shells (tcsh), so try without. + // Some shells use quoting of $PATH to trigger POSIX compatibility behavior (fish). + // Not all shells support `-n` on `echo`. + if (![task runWithArguments:@[ @"-l", @"-c", @"echo \"$PATH\"" ] stdin:NULL stdout:&data stderr:NULL exitStatus:NULL error:error] && ![task runWithArguments:@[ @"-c", @"echo \"$PATH\"" ] stdin:NULL stdout:&data stderr:NULL exitStatus:NULL error:error]) { + return nil; + } + return [[[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding] stringByTrimmingCharactersInSet:[NSCharacterSet newlineCharacterSet]]; +} + +// GUI apps don't inherit the user's shell PATH, so git-lfs is looked up in the login shell PATH first +// and then in the usual install locations (Homebrew on Apple Silicon and Intel, MacPorts). +// This is resolved lazily the first time a file with the "lfs" filter attribute is processed. +static NSString* _GitLFSExecutablePath(NSString** outPATH) { + static NSString* executablePath = nil; + static NSString* PATH = nil; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + @autoreleasepool { + PATH = _GetPATHUsingShell(NSProcessInfo.processInfo.environment[@"SHELL"], NULL) ?: _GetPATHUsingShell(@"/bin/sh", NULL); + NSMutableArray* directories = [[NSMutableArray alloc] init]; + for (NSString* directory in [PATH componentsSeparatedByString:@":"]) { + if (directory.length) { + [directories addObject:directory]; + } + } + [directories addObjectsFromArray:@[ @"/opt/homebrew/bin", @"/usr/local/bin", @"/opt/local/bin" ]]; + NSFileManager* fileManager = [NSFileManager defaultManager]; + for (NSString* directory in directories) { + NSString* path = [directory stringByAppendingPathComponent:@"git-lfs"]; + if ([fileManager isExecutableFileAtPath:path]) { + executablePath = path; + break; + } + } + if (executablePath) { + XLOG_VERBOSE(@"Using git-lfs tool at \"%@\"", executablePath); + } else { + XLOG_WARNING(@"git-lfs tool not found"); + } + } + }); + if (outPATH) { + *outPATH = PATH; + } + return executablePath; +} + +// Leave LFS files untouched (i.e. as pointer files) if git-lfs is not installed +static int _GitLFSCheck(git_filter* self, void** payload, const git_filter_source* src, const char** attr_values) { + return _GitLFSExecutablePath(NULL) ? 0 : GIT_PASSTHROUGH; +} + static int _GitLFSApply(git_filter* self, void** payload, git_buf* to, const git_buf* from, const git_filter_source* src) { if (from->size > 0) { @autoreleasepool { @@ -71,8 +121,16 @@ static int _GitLFSApply(git_filter* self, void** payload, git_buf* to, const git } [arguments addObject:@"--"]; [arguments addObject:GCFileSystemPathFromGitPath(git_filter_source_path(src))]; - GCTask* task = [[GCTask alloc] initWithExecutablePath:[NSString stringWithUTF8String:_GitLFSPath]]; + NSString* PATH; + NSString* executablePath = _GitLFSExecutablePath(&PATH); + if (!executablePath) { + return GIT_PASSTHROUGH; + } + GCTask* task = [[GCTask alloc] initWithExecutablePath:executablePath]; task.currentDirectoryPath = _MakeDirectoryPath(git_repository_workdir(git_filter_source_repo(src))); + if (PATH) { + task.additionalEnvironment = @{@"PATH" : PATH}; // git-lfs runs git itself so use the same PATH as the command line + } int status; NSData* stdinData = [[NSData alloc] initWithBytesNoCopy:from->ptr length:from->size freeWhenDone:NO]; NSData* stdoutData; @@ -120,14 +178,12 @@ + (void)load { assert(git_libgit2_init() >= 1); #if !TARGET_OS_IPHONE - struct stat info; - if (lstat(_GitLFSPath, &info) == 0) { - git_filter* filter = calloc(1, sizeof(git_filter)); - filter->version = GIT_FILTER_VERSION; - filter->attributes = "filter=lfs"; - filter->apply = _GitLFSApply; - assert(git_filter_register("lfs", filter, -1) == 0); // Priority must be lower than CRLF and IDENT built-in filters - } + git_filter* filter = calloc(1, sizeof(git_filter)); + filter->version = GIT_FILTER_VERSION; + filter->attributes = "filter=lfs"; + filter->check = _GitLFSCheck; + filter->apply = _GitLFSApply; + assert(git_filter_register("lfs", filter, -1) == 0); // Priority must be lower than CRLF and IDENT built-in filters #endif } @@ -365,15 +421,7 @@ - (NSString*)pathForHookWithName:(NSString*)name { } - (NSString*)getPATHUsingShell:(NSString*)shell error:(NSError**)error { - GCTask* task = [[GCTask alloc] initWithExecutablePath:shell]; - NSData* data; - // `-l` is not supported with `-c` in all shells (tcsh), so try without. - // Some shells use quoting of $PATH to trigger POSIX compatibility behavior (fish). - // Not all shells support `-n` on `echo`. - if (![task runWithArguments:@[ @"-l", @"-c", @"echo \"$PATH\"" ] stdin:NULL stdout:&data stderr:NULL exitStatus:NULL error:error] && ![task runWithArguments:@[ @"-c", @"echo \"$PATH\"" ] stdin:NULL stdout:&data stderr:NULL exitStatus:NULL error:error]) { - return nil; - } - return [[[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding] stringByTrimmingCharactersInSet:[NSCharacterSet newlineCharacterSet]]; + return _GetPATHUsingShell(shell, error); } - (BOOL)runHookWithName:(NSString*)name arguments:(NSArray*)arguments standardInput:(NSString*)standardInput error:(NSError**)error { From 827fa845bcd06fe7d06e1d81346b7a8e9b825460 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Fri, 16 Oct 2026 14:33:49 +0100 Subject: [PATCH 2/4] Add deterministic test coverage for the git-lfs PATH lookup Extract the lookup into GCFindExecutableInPATH() so it can be tested with a controlled PATH and fake executables, independent of whether git-lfs is installed or present at /usr/local/bin. Also ignore directories named like the tool, which -isExecutableFileAtPath: would otherwise accept. The real git-lfs test remains as a smoke test. Co-Authored-By: Claude Opus 5.5 --- GitUpKit/Core/GCPrivate.h | 3 ++ GitUpKit/Core/GCRepository-Tests.m | 54 +++++++++++++++++++++++++----- GitUpKit/Core/GCRepository.m | 41 ++++++++++++++--------- 3 files changed, 74 insertions(+), 24 deletions(-) diff --git a/GitUpKit/Core/GCPrivate.h b/GitUpKit/Core/GCPrivate.h index b915102f..f95936c4 100644 --- a/GitUpKit/Core/GCPrivate.h +++ b/GitUpKit/Core/GCPrivate.h @@ -146,6 +146,9 @@ extern int git_submodule_foreach_block(git_repository* repo, int (^block)(git_su - (BOOL)runWithArguments:(NSArray*)arguments stdin:(NSData*)stdin stdout:(NSData**)stdout stderr:(NSData**)stderr exitStatus:(int*)exitStatus error:(NSError**)error; // Returns NO if "exitStatus" is NULL and executable exits with a non-zero status @end +extern NSArray* GCDefaultExecutableDirectories(void); // Homebrew (Apple Silicon and Intel) and MacPorts +extern NSString* GCFindExecutableInPATH(NSString* name, NSString* PATH, NSArray* fallbackDirectories); // Returns nil if not found + #endif @interface GCObject () { diff --git a/GitUpKit/Core/GCRepository-Tests.m b/GitUpKit/Core/GCRepository-Tests.m index 9b380888..d6b6fe41 100644 --- a/GitUpKit/Core/GCRepository-Tests.m +++ b/GitUpKit/Core/GCRepository-Tests.m @@ -286,17 +286,55 @@ - (void)testPathForHookWithName_FoundFromLinkedWorktree { // The git-lfs tool used to be expected at /usr/local/bin/git-lfs only, so LFS files were // silently left unfiltered when it was installed elsewhere e.g. /opt/homebrew/bin (issue #805). +- (void)testFindExecutableInPATH { + NSFileManager* fileManager = [NSFileManager defaultManager]; + NSString* root = [NSTemporaryDirectory() stringByAppendingPathComponent:[[NSProcessInfo processInfo] globallyUniqueString]]; + NSString* emptyDirectory = [root stringByAppendingPathComponent:@"empty"]; + NSString* firstDirectory = [root stringByAppendingPathComponent:@"first"]; + NSString* secondDirectory = [root stringByAppendingPathComponent:@"second"]; + NSString* fallbackDirectory = [root stringByAppendingPathComponent:@"fallback"]; + for (NSString* directory in @[ emptyDirectory, firstDirectory, secondDirectory, fallbackDirectory ]) { + XCTAssertTrue([fileManager createDirectoryAtPath:directory withIntermediateDirectories:YES attributes:nil error:NULL]); + } + NSData* script = [@"#!/bin/sh\nexit 0\n" dataUsingEncoding:NSUTF8StringEncoding]; + NSString* firstPath = [firstDirectory stringByAppendingPathComponent:@"git-lfs"]; + NSString* secondPath = [secondDirectory stringByAppendingPathComponent:@"git-lfs"]; + NSString* fallbackPath = [fallbackDirectory stringByAppendingPathComponent:@"git-lfs"]; + XCTAssertTrue([fileManager createFileAtPath:secondPath contents:script attributes:@{NSFilePosixPermissions : @0o755}]); + XCTAssertTrue([fileManager createFileAtPath:fallbackPath contents:script attributes:@{NSFilePosixPermissions : @0o755}]); + + // Found in PATH, skipping empty components and directories without the tool + NSString* PATH = [NSString stringWithFormat:@"::%@:%@:%@", emptyDirectory, firstDirectory, secondDirectory]; + XCTAssertEqualObjects(GCFindExecutableInPATH(@"git-lfs", PATH, @[ fallbackDirectory ]), secondPath); + + // PATH order wins over later PATH entries and fallbacks + XCTAssertTrue([fileManager createFileAtPath:firstPath contents:script attributes:@{NSFilePosixPermissions : @0o755}]); + XCTAssertEqualObjects(GCFindExecutableInPATH(@"git-lfs", PATH, @[ fallbackDirectory ]), firstPath); + + // Non-executable files are ignored + XCTAssertTrue([fileManager setAttributes:@{NSFilePosixPermissions : @0o644} ofItemAtPath:firstPath error:NULL]); + XCTAssertEqualObjects(GCFindExecutableInPATH(@"git-lfs", PATH, @[ fallbackDirectory ]), secondPath); + + // Falls back to the extra directories when not in PATH, or when PATH couldn't be determined + XCTAssertEqualObjects(GCFindExecutableInPATH(@"git-lfs", emptyDirectory, @[ fallbackDirectory ]), fallbackPath); + XCTAssertEqualObjects(GCFindExecutableInPATH(@"git-lfs", nil, @[ fallbackDirectory ]), fallbackPath); + + // Not found anywhere + XCTAssertNil(GCFindExecutableInPATH(@"git-lfs", emptyDirectory, @[])); + XCTAssertNil(GCFindExecutableInPATH(@"git-lfs", nil, @[ emptyDirectory ])); + + // Directories with the tool's name are ignored + XCTAssertTrue([fileManager createDirectoryAtPath:[emptyDirectory stringByAppendingPathComponent:@"git-lfs"] withIntermediateDirectories:NO attributes:nil error:NULL]); + XCTAssertEqualObjects(GCFindExecutableInPATH(@"git-lfs", emptyDirectory, @[ fallbackDirectory ]), fallbackPath); + + XCTAssertTrue([fileManager removeItemAtPath:root error:NULL]); +} + +// Smoke test with the real git-lfs tool - (void)testGitLFSCleanFilter { NSString* shell = NSProcessInfo.processInfo.environment[@"SHELL"] ?: @"/bin/sh"; NSString* PATH = [self.repository getPATHUsingShell:shell error:NULL]; - BOOL found = NO; - for (NSString* directory in [[PATH componentsSeparatedByString:@":"] arrayByAddingObjectsFromArray:@[ @"/opt/homebrew/bin", @"/usr/local/bin", @"/opt/local/bin" ]]) { - if (directory.length && [[NSFileManager defaultManager] isExecutableFileAtPath:[directory stringByAppendingPathComponent:@"git-lfs"]]) { - found = YES; - break; - } - } - if (!found) { + if (!GCFindExecutableInPATH(@"git-lfs", PATH, GCDefaultExecutableDirectories())) { XCTSkip(@"git-lfs is not installed"); } diff --git a/GitUpKit/Core/GCRepository.m b/GitUpKit/Core/GCRepository.m index 26bc6a42..5157953e 100644 --- a/GitUpKit/Core/GCRepository.m +++ b/GitUpKit/Core/GCRepository.m @@ -63,8 +63,31 @@ static inline BOOL _IsDirectoryWritable(const char* path) { return [[[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding] stringByTrimmingCharactersInSet:[NSCharacterSet newlineCharacterSet]]; } +NSArray* GCDefaultExecutableDirectories(void) { + return @[ @"/opt/homebrew/bin", @"/usr/local/bin", @"/opt/local/bin" ]; +} + +NSString* GCFindExecutableInPATH(NSString* name, NSString* PATH, NSArray* fallbackDirectories) { + NSMutableArray* directories = [[NSMutableArray alloc] init]; + for (NSString* directory in [PATH componentsSeparatedByString:@":"]) { + if (directory.length) { + [directories addObject:directory]; + } + } + [directories addObjectsFromArray:fallbackDirectories]; + NSFileManager* fileManager = [NSFileManager defaultManager]; + for (NSString* directory in directories) { + NSString* path = [directory stringByAppendingPathComponent:name]; + BOOL isDirectory; + if ([fileManager fileExistsAtPath:path isDirectory:&isDirectory] && !isDirectory && [fileManager isExecutableFileAtPath:path]) { + return path; + } + } + return nil; +} + // GUI apps don't inherit the user's shell PATH, so git-lfs is looked up in the login shell PATH first -// and then in the usual install locations (Homebrew on Apple Silicon and Intel, MacPorts). +// and then in the usual install locations. // This is resolved lazily the first time a file with the "lfs" filter attribute is processed. static NSString* _GitLFSExecutablePath(NSString** outPATH) { static NSString* executablePath = nil; @@ -73,21 +96,7 @@ static inline BOOL _IsDirectoryWritable(const char* path) { dispatch_once(&onceToken, ^{ @autoreleasepool { PATH = _GetPATHUsingShell(NSProcessInfo.processInfo.environment[@"SHELL"], NULL) ?: _GetPATHUsingShell(@"/bin/sh", NULL); - NSMutableArray* directories = [[NSMutableArray alloc] init]; - for (NSString* directory in [PATH componentsSeparatedByString:@":"]) { - if (directory.length) { - [directories addObject:directory]; - } - } - [directories addObjectsFromArray:@[ @"/opt/homebrew/bin", @"/usr/local/bin", @"/opt/local/bin" ]]; - NSFileManager* fileManager = [NSFileManager defaultManager]; - for (NSString* directory in directories) { - NSString* path = [directory stringByAppendingPathComponent:@"git-lfs"]; - if ([fileManager isExecutableFileAtPath:path]) { - executablePath = path; - break; - } - } + executablePath = GCFindExecutableInPATH(@"git-lfs", PATH, GCDefaultExecutableDirectories()); if (executablePath) { XLOG_VERBOSE(@"Using git-lfs tool at \"%@\"", executablePath); } else { From 93a80eb765a9db8d765e5e273c678547a3b664da Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Mon, 2 Nov 2026 16:08:36 +0000 Subject: [PATCH 3/4] Run git-lfs as a long-running filter process Executing "git-lfs clean" or "git-lfs smudge" for each file costs ~200ms as git-lfs runs several git commands on startup, so checking out hundreds of LFS files (e.g. when rebasing onto a commit that changed many snapshot images) took minutes instead of the couple of seconds the git command line needs. Like the git command line, talk to a single "git-lfs filter-process" per working directory using the long-running filter protocol version 2 and terminate it once idle. Fall back to executing git-lfs per file if the process can't be used. Co-Authored-By: Claude Opus 5.5 --- GitUpKit/Core/GCRepository-Tests.m | 30 +++ GitUpKit/Core/GCRepository.m | 383 +++++++++++++++++++++++++++-- 2 files changed, 390 insertions(+), 23 deletions(-) diff --git a/GitUpKit/Core/GCRepository-Tests.m b/GitUpKit/Core/GCRepository-Tests.m index d6b6fe41..26f6df71 100644 --- a/GitUpKit/Core/GCRepository-Tests.m +++ b/GitUpKit/Core/GCRepository-Tests.m @@ -349,4 +349,34 @@ - (void)testGitLFSCleanFilter { XCTAssertTrue([contents hasPrefix:@"version https://git-lfs.github.com/spec/v1\n"], @"%@", contents); } +// Smoke test with the real git-lfs tool: many files go through the same long-running filter process, +// including content spanning several protocol packets +- (void)testGitLFSFilterRoundTrip { + NSString* shell = NSProcessInfo.processInfo.environment[@"SHELL"] ?: @"/bin/sh"; + NSString* PATH = [self.repository getPATHUsingShell:shell error:NULL]; + if (!GCFindExecutableInPATH(@"git-lfs", PATH, GCDefaultExecutableDirectories())) { + XCTSkip(@"git-lfs is not installed"); + } + + [self updateFileAtPath:@".gitattributes" withString:@"*.bin filter=lfs diff=lfs merge=lfs -text\n"]; + NSMutableDictionary* files = [[NSMutableDictionary alloc] init]; + for (int i = 0; i < 20; ++i) { + files[[NSString stringWithFormat:@"file-%i.bin", i]] = [@"" stringByPaddingToLength:(i * 10000 + 1) withString:[NSString stringWithFormat:@"Hello LFS %i\n", i] startingAtIndex:0]; + } + GCIndex* index = [self.repository readRepositoryIndex:NULL]; + XCTAssertNotNil(index); + for (NSString* path in files) { + [self updateFileAtPath:path withString:files[path]]; + XCTAssertTrue([self.repository addFileInWorkingDirectory:path toIndex:index error:NULL]); + NSString* contents = [[NSString alloc] initWithData:[self.repository readContentsForFile:path inIndex:index error:NULL] encoding:NSUTF8StringEncoding]; + XCTAssertTrue([contents hasPrefix:@"version https://git-lfs.github.com/spec/v1\n"], @"%@", contents); + } + + for (NSString* path in files) { + [self deleteFileAtPath:path]; + XCTAssertTrue([self.repository checkoutFileToWorkingDirectory:path fromIndex:index error:NULL]); + [self assertContentsOfFileAtPath:path equalsString:files[path]]; + } +} + @end diff --git a/GitUpKit/Core/GCRepository.m b/GitUpKit/Core/GCRepository.m index 5157953e..b1021f77 100644 --- a/GitUpKit/Core/GCRepository.m +++ b/GitUpKit/Core/GCRepository.m @@ -115,48 +115,385 @@ static int _GitLFSCheck(git_filter* self, void** payload, const git_filter_sourc return _GitLFSExecutablePath(NULL) ? 0 : GIT_PASSTHROUGH; } +#define kLFSFilterProcessIdleTimeout 10.0 +#define kPacketMaxDataLength 65516 + +typedef NS_ENUM(NSUInteger, GCFilterProcessResult) { + kGCFilterProcessResult_Success = 0, + kGCFilterProcessResult_Error, // The filter reported it couldn't process the file + kGCFilterProcessResult_Broken // Communicating with the filter process failed +}; + +// Executing "git-lfs clean" or "git-lfs smudge" costs ~200ms per file as git-lfs runs several git commands on startup, +// which adds up to minutes when checking out hundreds of LFS files. So like the git command line, use a single +// long-running "git-lfs filter-process" per working directory instead, talking the long-running filter protocol +// version 2 (see "Long Running Filter Process" in gitattributes(5)), and terminate it once idle. +@interface GCLFSFilterProcess : NSObject +@property(nonatomic) NSUInteger useCount; +- (instancetype)initWithExecutablePath:(NSString*)executablePath PATH:(NSString*)PATH workingDirectory:(NSString*)workingDirectory handshakeFailed:(BOOL*)handshakeFailed; +- (GCFilterProcessResult)runCommand:(NSString*)command pathname:(const char*)pathname input:(const git_buf*)input output:(git_buf*)output; +- (void)terminate; +@end + +@implementation GCLFSFilterProcess { + NSTask* _task; + NSPipe* _inPipe; + NSPipe* _outPipe; + int _writeFD; + int _readFD; +} + +- (instancetype)initWithExecutablePath:(NSString*)executablePath PATH:(NSString*)PATH workingDirectory:(NSString*)workingDirectory handshakeFailed:(BOOL*)handshakeFailed { + if ((self = [super init])) { + _inPipe = [[NSPipe alloc] init]; + _outPipe = [[NSPipe alloc] init]; + _task = [[NSTask alloc] init]; + _task.executableURL = [NSURL fileURLWithPath:executablePath]; + _task.arguments = @[ @"filter-process" ]; + _task.currentDirectoryURL = [NSURL fileURLWithPath:workingDirectory isDirectory:YES]; + if (PATH) { + NSMutableDictionary* environment = [[NSMutableDictionary alloc] initWithDictionary:[[NSProcessInfo processInfo] environment]]; + environment[@"PATH"] = PATH; // git-lfs runs git itself so use the same PATH as the command line + _task.environment = environment; + } + _task.standardInput = _inPipe; + _task.standardOutput = _outPipe; + _task.standardError = [NSFileHandle fileHandleWithNullDevice]; // Not read so must not be a pipe that could fill up + NSError* error; + if (![_task launchAndReturnError:&error]) { + XLOG_ERROR(@"Failed launching git-lfs filter process: %@", error); + return nil; + } + _writeFD = _inPipe.fileHandleForWriting.fileDescriptor; + _readFD = _outPipe.fileHandleForReading.fileDescriptor; + fcntl(_writeFD, F_SETNOSIGPIPE, 1); // Get EPIPE instead of a SIGPIPE if the process exits + + NSArray* lines; + if (![self _writeTextPacket:@"git-filter-client"] || ![self _writeTextPacket:@"version=2"] || ![self _writeFlushPacket] || + !(lines = [self _readTextPacketList]) || ![lines containsObject:@"git-filter-server"] || ![lines containsObject:@"version=2"] || + ![self _writeTextPacket:@"capability=clean"] || ![self _writeTextPacket:@"capability=smudge"] || ![self _writeFlushPacket] || + !(lines = [self _readTextPacketList]) || ![lines containsObject:@"capability=clean"] || ![lines containsObject:@"capability=smudge"]) { + XLOG_ERROR(@"Failed handshake with git-lfs filter process"); + [self terminate]; + *handshakeFailed = YES; + return nil; + } + } + return self; +} + +- (void)dealloc { + [self terminate]; +} + +// Closing stdin makes the process exit on its own +- (void)terminate { + if (_inPipe) { + [_inPipe.fileHandleForWriting closeFile]; + [_outPipe.fileHandleForReading closeFile]; + _inPipe = nil; + _outPipe = nil; + } +} + +- (BOOL)_writeBytes:(const void*)bytes length:(size_t)length { + while (length > 0) { + ssize_t count = write(_writeFD, bytes, length); + if (count < 0) { + if (errno == EINTR) { + continue; + } + return NO; + } + bytes = (const char*)bytes + count; + length -= count; + } + return YES; +} + +- (BOOL)_readBytes:(void*)bytes length:(size_t)length { + while (length > 0) { + ssize_t count = read(_readFD, bytes, length); + if (count < 0) { + if (errno == EINTR) { + continue; + } + return NO; + } + if (count == 0) { // EOF + return NO; + } + bytes = (char*)bytes + count; + length -= count; + } + return YES; +} + +- (BOOL)_writePacket:(const void*)bytes length:(size_t)length { + XLOG_DEBUG_CHECK((length > 0) && (length <= kPacketMaxDataLength)); + char header[5]; + snprintf(header, sizeof(header), "%04zx", length + 4); + return [self _writeBytes:header length:4] && [self _writeBytes:bytes length:length]; +} + +- (BOOL)_writeFlushPacket { + return [self _writeBytes:"0000" length:4]; +} + +- (BOOL)_writeTextPacket:(NSString*)text { + NSData* data = [[text stringByAppendingString:@"\n"] dataUsingEncoding:NSUTF8StringEncoding]; + return [self _writePacket:data.bytes length:data.length]; +} + +// Returns -1 on error, 0 for a flush packet or the length of the data read into "buffer" which must be at least kPacketMaxDataLength long +- (ssize_t)_readPacket:(char*)buffer { + char header[5] = {0}; + if (![self _readBytes:header length:4]) { + return -1; + } + char* end; + unsigned long length = strtoul(header, &end, 16); + if ((end != header + 4) || ((length > 0) && (length < 4)) || (length > kPacketMaxDataLength + 4)) { + XLOG_ERROR(@"Invalid packet from git-lfs filter process"); + return -1; + } + if (length == 0) { + return 0; + } + length -= 4; + if (![self _readBytes:buffer length:length]) { + return -1; + } + return length; +} + +// Returns nil on error +- (NSArray*)_readTextPacketList { + NSMutableArray* lines = [[NSMutableArray alloc] init]; + char buffer[kPacketMaxDataLength]; + while (1) { + ssize_t length = [self _readPacket:buffer]; + if (length < 0) { + return nil; + } + if (length == 0) { + break; + } + if (buffer[length - 1] == '\n') { + --length; + } + NSString* line = [[NSString alloc] initWithBytes:buffer length:length encoding:NSUTF8StringEncoding]; + if (line) { + [lines addObject:line]; + } + } + return lines; +} + +// Returns the last status in the list or the empty string if there is none +static NSString* _StatusFromTextPacketList(NSArray* lines) { + NSString* status = @""; + for (NSString* line in lines) { + if ([line hasPrefix:@"status="]) { + status = [line substringFromIndex:7]; + } + } + return status; +} + +- (GCFilterProcessResult)runCommand:(NSString*)command pathname:(const char*)pathname input:(const git_buf*)input output:(git_buf*)output { + if (!_inPipe) { + return kGCFilterProcessResult_Broken; + } + NSArray* lines; + NSString* status; + char* buffer; + NSMutableData* data; + + NSMutableData* pathnameData = [[NSMutableData alloc] initWithBytes:"pathname=" length:9]; + [pathnameData appendBytes:pathname length:strlen(pathname)]; + [pathnameData appendBytes:"\n" length:1]; + if (pathnameData.length > kPacketMaxDataLength) { + return kGCFilterProcessResult_Broken; + } + if (![self _writeTextPacket:[@"command=" stringByAppendingString:command]] || ![self _writePacket:pathnameData.bytes length:pathnameData.length] || ![self _writeFlushPacket]) { + goto broken; + } + for (size_t offset = 0; offset < input->size; offset += kPacketMaxDataLength) { + if (![self _writePacket:(input->ptr + offset) length:MIN(input->size - offset, kPacketMaxDataLength)]) { + goto broken; + } + } + if (![self _writeFlushPacket]) { + goto broken; + } + + lines = [self _readTextPacketList]; + if (!lines) { + goto broken; + } + status = _StatusFromTextPacketList(lines); + if (![status isEqualToString:@"success"]) { + XLOG_ERROR(@"git-lfs filter process failed to %@ \"%s\" (%@)", command, pathname, status); + return [status isEqualToString:@"error"] || [status isEqualToString:@"abort"] ? kGCFilterProcessResult_Error : kGCFilterProcessResult_Broken; + } + + buffer = (char*)malloc(kPacketMaxDataLength); + data = [[NSMutableData alloc] init]; + while (1) { + ssize_t length = [self _readPacket:buffer]; + if (length < 0) { + free(buffer); + goto broken; + } + if (length == 0) { + free(buffer); + break; + } + [data appendBytes:buffer length:length]; + } + + lines = [self _readTextPacketList]; // An empty list means the status is unchanged + if (!lines) { + goto broken; + } + status = _StatusFromTextPacketList(lines); + if (status.length && ![status isEqualToString:@"success"]) { + XLOG_ERROR(@"git-lfs filter process failed to %@ \"%s\" (%@)", command, pathname, status); + return kGCFilterProcessResult_Error; + } + if (git_buf_set(output, data.bytes, data.length) < 0) { // TODO: Avoid copying data + return kGCFilterProcessResult_Error; + } + return kGCFilterProcessResult_Success; + +broken: + XLOG_ERROR(@"Lost communication with git-lfs filter process"); + [self terminate]; + return kGCFilterProcessResult_Broken; +} + +@end + +static NSMutableDictionary* _lfsFilterProcesses = nil; // Idle processes keyed by working directory +static BOOL _lfsFilterProcessUnsupported = NO; // git-lfs versions before 2.0 don't support "filter-process" + +// Takes an idle process out of the cache so concurrent filtering in the same working directory gets its own process +static GCLFSFilterProcess* _CheckOutLFSFilterProcess(NSString* executablePath, NSString* PATH, NSString* workingDirectory) { + GCLFSFilterProcess* process; + @synchronized([GCLFSFilterProcess class]) { + if (_lfsFilterProcessUnsupported) { + return nil; + } + process = _lfsFilterProcesses[workingDirectory]; + [_lfsFilterProcesses removeObjectForKey:workingDirectory]; + } + if (!process) { + BOOL handshakeFailed = NO; + process = [[GCLFSFilterProcess alloc] initWithExecutablePath:executablePath PATH:PATH workingDirectory:workingDirectory handshakeFailed:&handshakeFailed]; + if (handshakeFailed) { + @synchronized([GCLFSFilterProcess class]) { + _lfsFilterProcessUnsupported = YES; + } + } + } + return process; +} + +static void _CheckInLFSFilterProcess(GCLFSFilterProcess* process, NSString* workingDirectory) { + NSUInteger useCount; + @synchronized([GCLFSFilterProcess class]) { + useCount = ++process.useCount; + if (!_lfsFilterProcesses) { + _lfsFilterProcesses = [[NSMutableDictionary alloc] init]; + } + if (_lfsFilterProcesses[workingDirectory]) { // Another thread already checked in a process for this working directory + [process terminate]; + return; + } + _lfsFilterProcesses[workingDirectory] = process; + } + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(kLFSFilterProcessIdleTimeout * NSEC_PER_SEC)), dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + @synchronized([GCLFSFilterProcess class]) { + if ((_lfsFilterProcesses[workingDirectory] == process) && (process.useCount == useCount)) { + [_lfsFilterProcesses removeObjectForKey:workingDirectory]; + [process terminate]; + } + } + }); +} + +// Fallback executing git-lfs for this file only +static int _GitLFSApplyWithTask(NSString* executablePath, NSString* PATH, NSString* workingDirectory, NSString* command, const git_buf* from, git_buf* to, const git_filter_source* src) { + GCTask* task = [[GCTask alloc] initWithExecutablePath:executablePath]; + task.currentDirectoryPath = workingDirectory; + if (PATH) { + task.additionalEnvironment = @{@"PATH" : PATH}; // git-lfs runs git itself so use the same PATH as the command line + } + int status; + NSData* stdinData = [[NSData alloc] initWithBytesNoCopy:from->ptr length:from->size freeWhenDone:NO]; + NSData* stdoutData; + if (![task runWithArguments:@[ command, @"--", GCFileSystemPathFromGitPath(git_filter_source_path(src)) ] stdin:stdinData stdout:&stdoutData stderr:NULL exitStatus:&status error:NULL]) { + XLOG_ERROR(@"git-lfs tool failed executing"); + giterr_set_str(GITERR_FILTER, "git-lfs tool failed executing"); + return -1; + } + if (status != 0) { + XLOG_ERROR(@"git-lfs tool exited with non-zero status (%i)", status); + giterr_set_str(GITERR_FILTER, "git-lfs tool exited with non-zero status"); + return -1; + } + if (git_buf_set(to, stdoutData.bytes, stdoutData.length) < 0) { // TODO: Avoid copying data + return -1; + } + return 0; +} + static int _GitLFSApply(git_filter* self, void** payload, git_buf* to, const git_buf* from, const git_filter_source* src) { if (from->size > 0) { @autoreleasepool { CFAbsoluteTime time = CFAbsoluteTimeGetCurrent(); - NSMutableArray* arguments = [[NSMutableArray alloc] init]; + NSString* command; if (git_filter_source_mode(src) == GIT_FILTER_SMUDGE) { // ODB -> Worktree - [arguments addObject:@"smudge"]; + command = @"smudge"; } else if (git_filter_source_mode(src) == GIT_FILTER_CLEAN) { // Worktree -> ODB - [arguments addObject:@"clean"]; + command = @"clean"; } else { XLOG_DEBUG_UNREACHABLE(); return -1; } - [arguments addObject:@"--"]; - [arguments addObject:GCFileSystemPathFromGitPath(git_filter_source_path(src))]; NSString* PATH; NSString* executablePath = _GitLFSExecutablePath(&PATH); if (!executablePath) { return GIT_PASSTHROUGH; } - GCTask* task = [[GCTask alloc] initWithExecutablePath:executablePath]; - task.currentDirectoryPath = _MakeDirectoryPath(git_repository_workdir(git_filter_source_repo(src))); - if (PATH) { - task.additionalEnvironment = @{@"PATH" : PATH}; // git-lfs runs git itself so use the same PATH as the command line - } - int status; - NSData* stdinData = [[NSData alloc] initWithBytesNoCopy:from->ptr length:from->size freeWhenDone:NO]; - NSData* stdoutData; - if (![task runWithArguments:arguments stdin:stdinData stdout:&stdoutData stderr:NULL exitStatus:&status error:NULL]) { - XLOG_ERROR(@"git-lfs tool failed executing"); - giterr_set_str(GITERR_FILTER, "git-lfs tool failed executing"); + NSString* workingDirectory = _MakeDirectoryPath(git_repository_workdir(git_filter_source_repo(src))); + if (!workingDirectory) { + XLOG_DEBUG_UNREACHABLE(); return -1; } - XLOG_VERBOSE(@"Executed git-lfs tool in %.3f seconds", CFAbsoluteTimeGetCurrent() - time); - if (status != 0) { - XLOG_ERROR(@"git-lfs tool exited with non-zero status (%i)", status); - giterr_set_str(GITERR_FILTER, "git-lfs tool exited with non-zero status"); - return -1; + + GCLFSFilterProcess* process = _CheckOutLFSFilterProcess(executablePath, PATH, workingDirectory); + if (process) { + GCFilterProcessResult result = [process runCommand:command pathname:git_filter_source_path(src) input:from output:to]; + if (result != kGCFilterProcessResult_Broken) { + _CheckInLFSFilterProcess(process, workingDirectory); + } + if (result == kGCFilterProcessResult_Error) { + giterr_set_str(GITERR_FILTER, "git-lfs filter process failed"); + return -1; + } + if (result == kGCFilterProcessResult_Success) { + XLOG_VERBOSE(@"Executed git-lfs filter process in %.3f seconds", CFAbsoluteTimeGetCurrent() - time); + return 0; + } } - if (git_buf_set(to, stdoutData.bytes, stdoutData.length) < 0) { // TODO: Avoid copying data - return -1; + + int status = _GitLFSApplyWithTask(executablePath, PATH, workingDirectory, command, from, to, src); + if (status == 0) { + XLOG_VERBOSE(@"Executed git-lfs tool in %.3f seconds", CFAbsoluteTimeGetCurrent() - time); } + return status; } } return 0; From bd58500bd6ce447d973c7155692d2b214ba48b2f Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Mon, 2 Nov 2026 17:14:45 +0000 Subject: [PATCH 4/4] Report git-lfs errors and check that the filter process is reused Address review feedback on the long-running filter process: - Capture what git-lfs writes to stderr, draining it continuously so the pipe can't fill up, and include it in the error so e.g. a failed download says why instead of only "git-lfs filter process failed". The per-file fallback now does the same. - git-lfs replies "status=success" then exits when it can't download a file. Report that as an error for the file instead of retrying it by executing git-lfs again, which would repeat the failing download. - Retry with a new process if a cached idle one turns out to have exited, before falling back to executing git-lfs per file. - Count filter process launches and per-file fallbacks so the tests can check that many files go through a single process, and that a failure reports the git-lfs error without falling back. Co-Authored-By: Claude Opus 5.5 --- GitUpKit/Core/GCPrivate.h | 2 + GitUpKit/Core/GCRepository-Tests.m | 37 ++++++- GitUpKit/Core/GCRepository.m | 159 +++++++++++++++++++++++++---- 3 files changed, 175 insertions(+), 23 deletions(-) diff --git a/GitUpKit/Core/GCPrivate.h b/GitUpKit/Core/GCPrivate.h index f95936c4..0398a953 100644 --- a/GitUpKit/Core/GCPrivate.h +++ b/GitUpKit/Core/GCPrivate.h @@ -148,6 +148,8 @@ extern int git_submodule_foreach_block(git_repository* repo, int (^block)(git_su extern NSArray* GCDefaultExecutableDirectories(void); // Homebrew (Apple Silicon and Intel) and MacPorts extern NSString* GCFindExecutableInPATH(NSString* name, NSString* PATH, NSArray* fallbackDirectories); // Returns nil if not found +extern NSUInteger GCLFSFilterProcessLaunchCount(void); // Number of "git-lfs filter-process" processes launched so far +extern NSUInteger GCLFSFilterTaskCount(void); // Number of files filtered by executing git-lfs for that file only #endif diff --git a/GitUpKit/Core/GCRepository-Tests.m b/GitUpKit/Core/GCRepository-Tests.m index 26f6df71..57c9c7f3 100644 --- a/GitUpKit/Core/GCRepository-Tests.m +++ b/GitUpKit/Core/GCRepository-Tests.m @@ -350,7 +350,7 @@ - (void)testGitLFSCleanFilter { } // Smoke test with the real git-lfs tool: many files go through the same long-running filter process, -// including content spanning several protocol packets +// including content spanning several protocol packets, without falling back to executing git-lfs per file - (void)testGitLFSFilterRoundTrip { NSString* shell = NSProcessInfo.processInfo.environment[@"SHELL"] ?: @"/bin/sh"; NSString* PATH = [self.repository getPATHUsingShell:shell error:NULL]; @@ -363,6 +363,8 @@ - (void)testGitLFSFilterRoundTrip { for (int i = 0; i < 20; ++i) { files[[NSString stringWithFormat:@"file-%i.bin", i]] = [@"" stringByPaddingToLength:(i * 10000 + 1) withString:[NSString stringWithFormat:@"Hello LFS %i\n", i] startingAtIndex:0]; } + NSUInteger launchCount = GCLFSFilterProcessLaunchCount(); + NSUInteger taskCount = GCLFSFilterTaskCount(); GCIndex* index = [self.repository readRepositoryIndex:NULL]; XCTAssertNotNil(index); for (NSString* path in files) { @@ -377,6 +379,39 @@ - (void)testGitLFSFilterRoundTrip { XCTAssertTrue([self.repository checkoutFileToWorkingDirectory:path fromIndex:index error:NULL]); [self assertContentsOfFileAtPath:path equalsString:files[path]]; } + XCTAssertEqual(GCLFSFilterProcessLaunchCount() - launchCount, 1); + XCTAssertEqual(GCLFSFilterTaskCount() - taskCount, 0); +} + +// Smoke test with the real git-lfs tool: the reason it reports on stderr ends up in the error +- (void)testGitLFSFilterError { + NSString* shell = NSProcessInfo.processInfo.environment[@"SHELL"] ?: @"/bin/sh"; + NSString* PATH = [self.repository getPATHUsingShell:shell error:NULL]; + if (!GCFindExecutableInPATH(@"git-lfs", PATH, GCDefaultExecutableDirectories())) { + XCTSkip(@"git-lfs is not installed"); + } + + // Stage a pointer to an LFS object that doesn't exist locally, and there is no remote to download it from + NSString* pointer = [NSString stringWithFormat:@"version https://git-lfs.github.com/spec/v1\noid sha256:%@\nsize 12\n", [@"" stringByPaddingToLength:64 withString:@"a" startingAtIndex:0]]; + [self updateFileAtPath:@"missing.bin" withString:pointer]; + GCIndex* index = [self.repository readRepositoryIndex:NULL]; + XCTAssertNotNil(index); + XCTAssertTrue([self.repository addFileInWorkingDirectory:@"missing.bin" toIndex:index error:NULL]); + [self updateFileAtPath:@".gitattributes" withString:@"*.bin filter=lfs diff=lfs merge=lfs -text\n"]; + [self deleteFileAtPath:@"missing.bin"]; + + NSUInteger taskCount = GCLFSFilterTaskCount(); + NSError* error; + XCTAssertFalse([self.repository checkoutFileToWorkingDirectory:@"missing.bin" fromIndex:index error:&error]); + XCTAssertTrue([error.localizedDescription containsString:@"git-lfs filter process failed: "], @"%@", error); + XCTAssertTrue([error.localizedDescription containsString:@"missing.bin"], @"%@", error); + + // git-lfs exits after such a failure so the next file gets a new process + NSUInteger launchCount = GCLFSFilterProcessLaunchCount(); + [self updateFileAtPath:@"present.bin" withString:@"Hello LFS\n"]; + XCTAssertTrue([self.repository addFileInWorkingDirectory:@"present.bin" toIndex:index error:NULL]); + XCTAssertEqual(GCLFSFilterProcessLaunchCount() - launchCount, 1); + XCTAssertEqual(GCLFSFilterTaskCount() - taskCount, 0); } @end diff --git a/GitUpKit/Core/GCRepository.m b/GitUpKit/Core/GCRepository.m index b1021f77..a106aec0 100644 --- a/GitUpKit/Core/GCRepository.m +++ b/GitUpKit/Core/GCRepository.m @@ -117,11 +117,12 @@ static int _GitLFSCheck(git_filter* self, void** payload, const git_filter_sourc #define kLFSFilterProcessIdleTimeout 10.0 #define kPacketMaxDataLength 65516 +#define kStandardErrorMaxLength 16384 typedef NS_ENUM(NSUInteger, GCFilterProcessResult) { kGCFilterProcessResult_Success = 0, - kGCFilterProcessResult_Error, // The filter reported it couldn't process the file - kGCFilterProcessResult_Broken // Communicating with the filter process failed + kGCFilterProcessResult_Error, // The filter couldn't process the file + kGCFilterProcessResult_Broken // Communicating with the filter process failed before it accepted the request }; // Executing "git-lfs clean" or "git-lfs smudge" costs ~200ms per file as git-lfs runs several git commands on startup, @@ -130,23 +131,47 @@ typedef NS_ENUM(NSUInteger, GCFilterProcessResult) { // version 2 (see "Long Running Filter Process" in gitattributes(5)), and terminate it once idle. @interface GCLFSFilterProcess : NSObject @property(nonatomic) NSUInteger useCount; +@property(nonatomic, readonly, getter=isRunning) BOOL running; - (instancetype)initWithExecutablePath:(NSString*)executablePath PATH:(NSString*)PATH workingDirectory:(NSString*)workingDirectory handshakeFailed:(BOOL*)handshakeFailed; -- (GCFilterProcessResult)runCommand:(NSString*)command pathname:(const char*)pathname input:(const git_buf*)input output:(git_buf*)output; +- (GCFilterProcessResult)runCommand:(NSString*)command pathname:(const char*)pathname input:(const git_buf*)input output:(git_buf*)output errorOutput:(NSString**)errorOutput; // "errorOutput" is set to what the process wrote to stderr on kGCFilterProcessResult_Error - (void)terminate; @end +// Reads whatever is available from the non-blocking "fd" and returns NO once it reaches EOF +static BOOL _DrainStandardError(int fd, NSMutableData* data) { + char buffer[4096]; + while (1) { + ssize_t count = read(fd, buffer, sizeof(buffer)); + if (count > 0) { + if (data.length < kStandardErrorMaxLength) { + [data appendBytes:buffer length:MIN((NSUInteger)count, kStandardErrorMaxLength - data.length)]; + } + } else if ((count < 0) && (errno == EINTR)) { + continue; + } else { + return (count < 0) && (errno == EAGAIN); + } + } +} + @implementation GCLFSFilterProcess { NSTask* _task; NSPipe* _inPipe; NSPipe* _outPipe; + NSPipe* _errorPipe; int _writeFD; int _readFD; + int _errorFD; + dispatch_queue_t _errorQueue; + dispatch_source_t _errorSource; + NSMutableData* _errorData; } - (instancetype)initWithExecutablePath:(NSString*)executablePath PATH:(NSString*)PATH workingDirectory:(NSString*)workingDirectory handshakeFailed:(BOOL*)handshakeFailed { if ((self = [super init])) { _inPipe = [[NSPipe alloc] init]; _outPipe = [[NSPipe alloc] init]; + _errorPipe = [[NSPipe alloc] init]; _task = [[NSTask alloc] init]; _task.executableURL = [NSURL fileURLWithPath:executablePath]; _task.arguments = @[ @"filter-process" ]; @@ -158,7 +183,7 @@ - (instancetype)initWithExecutablePath:(NSString*)executablePath PATH:(NSString* } _task.standardInput = _inPipe; _task.standardOutput = _outPipe; - _task.standardError = [NSFileHandle fileHandleWithNullDevice]; // Not read so must not be a pipe that could fill up + _task.standardError = _errorPipe; NSError* error; if (![_task launchAndReturnError:&error]) { XLOG_ERROR(@"Failed launching git-lfs filter process: %@", error); @@ -168,12 +193,32 @@ - (instancetype)initWithExecutablePath:(NSString*)executablePath PATH:(NSString* _readFD = _outPipe.fileHandleForReading.fileDescriptor; fcntl(_writeFD, F_SETNOSIGPIPE, 1); // Get EPIPE instead of a SIGPIPE if the process exits + // git-lfs reports why a file failed on stderr, which is drained continuously so the pipe can't fill up and block it + _errorFD = _errorPipe.fileHandleForReading.fileDescriptor; + fcntl(_errorFD, F_SETFL, fcntl(_errorFD, F_GETFL) | O_NONBLOCK); + _errorData = [[NSMutableData alloc] init]; + _errorQueue = dispatch_queue_create("co.gitup.lfs-filter-process.stderr", DISPATCH_QUEUE_SERIAL); + _errorSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_READ, _errorFD, 0, _errorQueue); + int errorFD = _errorFD; + NSMutableData* errorData = _errorData; + NSFileHandle* errorHandle = _errorPipe.fileHandleForReading; + __weak dispatch_source_t weakSource = _errorSource; + dispatch_source_set_event_handler(_errorSource, ^{ + if (!_DrainStandardError(errorFD, errorData)) { + dispatch_source_cancel(weakSource); + } + }); + dispatch_source_set_cancel_handler(_errorSource, ^{ + [errorHandle closeFile]; + }); + dispatch_resume(_errorSource); + NSArray* lines; if (![self _writeTextPacket:@"git-filter-client"] || ![self _writeTextPacket:@"version=2"] || ![self _writeFlushPacket] || !(lines = [self _readTextPacketList]) || ![lines containsObject:@"git-filter-server"] || ![lines containsObject:@"version=2"] || ![self _writeTextPacket:@"capability=clean"] || ![self _writeTextPacket:@"capability=smudge"] || ![self _writeFlushPacket] || !(lines = [self _readTextPacketList]) || ![lines containsObject:@"capability=clean"] || ![lines containsObject:@"capability=smudge"]) { - XLOG_ERROR(@"Failed handshake with git-lfs filter process"); + XLOG_ERROR(@"Failed handshake with git-lfs filter process: %@", [self _takeStandardError]); [self terminate]; *handshakeFailed = YES; return nil; @@ -186,6 +231,10 @@ - (void)dealloc { [self terminate]; } +- (BOOL)isRunning { + return _inPipe != nil; +} + // Closing stdin makes the process exit on its own - (void)terminate { if (_inPipe) { @@ -194,6 +243,28 @@ - (void)terminate { _inPipe = nil; _outPipe = nil; } + if (_errorSource) { + dispatch_source_cancel(_errorSource); // Closes the pipe once the source is done with it + _errorSource = nil; + } +} + +// Returns what the process wrote to stderr since the last call: git-lfs writes it before replying with the +// status, so everything about the last request is already in the pipe even if the source hasn't read it yet +- (NSString*)_takeStandardError { + if (!_errorQueue) { + return nil; + } + __block NSString* string; + dispatch_source_t source = _errorSource; + dispatch_sync(_errorQueue, ^{ + if (source && !dispatch_source_testcancel(source)) { + _DrainStandardError(_errorFD, _errorData); + } + string = [[[NSString alloc] initWithData:_errorData encoding:NSUTF8StringEncoding] stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]]; + _errorData.length = 0; + }); + return string; } - (BOOL)_writeBytes:(const void*)bytes length:(size_t)length { @@ -301,10 +372,11 @@ - (NSArray*)_readTextPacketList { return status; } -- (GCFilterProcessResult)runCommand:(NSString*)command pathname:(const char*)pathname input:(const git_buf*)input output:(git_buf*)output { +- (GCFilterProcessResult)runCommand:(NSString*)command pathname:(const char*)pathname input:(const git_buf*)input output:(git_buf*)output errorOutput:(NSString**)errorOutput { if (!_inPipe) { return kGCFilterProcessResult_Broken; } + [self _takeStandardError]; // Discard anything left over from previous requests NSArray* lines; NSString* status; char* buffer; @@ -334,8 +406,12 @@ - (GCFilterProcessResult)runCommand:(NSString*)command pathname:(const char*)pat } status = _StatusFromTextPacketList(lines); if (![status isEqualToString:@"success"]) { - XLOG_ERROR(@"git-lfs filter process failed to %@ \"%s\" (%@)", command, pathname, status); - return [status isEqualToString:@"error"] || [status isEqualToString:@"abort"] ? kGCFilterProcessResult_Error : kGCFilterProcessResult_Broken; + if (![status isEqualToString:@"error"] && ![status isEqualToString:@"abort"]) { + goto broken; + } + *errorOutput = [self _takeStandardError]; + XLOG_ERROR(@"git-lfs filter process failed to %@ \"%s\" (%@): %@", command, pathname, status, *errorOutput); + return kGCFilterProcessResult_Error; } buffer = (char*)malloc(kPacketMaxDataLength); @@ -344,7 +420,7 @@ - (GCFilterProcessResult)runCommand:(NSString*)command pathname:(const char*)pat ssize_t length = [self _readPacket:buffer]; if (length < 0) { free(buffer); - goto broken; + goto failed; } if (length == 0) { free(buffer); @@ -355,11 +431,12 @@ - (GCFilterProcessResult)runCommand:(NSString*)command pathname:(const char*)pat lines = [self _readTextPacketList]; // An empty list means the status is unchanged if (!lines) { - goto broken; + goto failed; } status = _StatusFromTextPacketList(lines); if (status.length && ![status isEqualToString:@"success"]) { - XLOG_ERROR(@"git-lfs filter process failed to %@ \"%s\" (%@)", command, pathname, status); + *errorOutput = [self _takeStandardError]; + XLOG_ERROR(@"git-lfs filter process failed to %@ \"%s\" (%@): %@", command, pathname, status, *errorOutput); return kGCFilterProcessResult_Error; } if (git_buf_set(output, data.bytes, data.length) < 0) { // TODO: Avoid copying data @@ -371,15 +448,35 @@ - (GCFilterProcessResult)runCommand:(NSString*)command pathname:(const char*)pat XLOG_ERROR(@"Lost communication with git-lfs filter process"); [self terminate]; return kGCFilterProcessResult_Broken; + +failed: // git-lfs exits after accepting a request it can't complete e.g. when downloading the file fails, so retrying wouldn't help + *errorOutput = [self _takeStandardError]; + XLOG_ERROR(@"git-lfs filter process exited while trying to %@ \"%s\": %@", command, pathname, *errorOutput); + [self terminate]; + return kGCFilterProcessResult_Error; } @end static NSMutableDictionary* _lfsFilterProcesses = nil; // Idle processes keyed by working directory static BOOL _lfsFilterProcessUnsupported = NO; // git-lfs versions before 2.0 don't support "filter-process" +static NSUInteger _lfsFilterProcessLaunchCount = 0; +static NSUInteger _lfsFilterTaskCount = 0; + +NSUInteger GCLFSFilterProcessLaunchCount(void) { + @synchronized([GCLFSFilterProcess class]) { + return _lfsFilterProcessLaunchCount; + } +} + +NSUInteger GCLFSFilterTaskCount(void) { + @synchronized([GCLFSFilterProcess class]) { + return _lfsFilterTaskCount; + } +} // Takes an idle process out of the cache so concurrent filtering in the same working directory gets its own process -static GCLFSFilterProcess* _CheckOutLFSFilterProcess(NSString* executablePath, NSString* PATH, NSString* workingDirectory) { +static GCLFSFilterProcess* _CheckOutLFSFilterProcess(NSString* executablePath, NSString* PATH, NSString* workingDirectory, BOOL* reused) { GCLFSFilterProcess* process; @synchronized([GCLFSFilterProcess class]) { if (_lfsFilterProcessUnsupported) { @@ -388,11 +485,14 @@ - (GCFilterProcessResult)runCommand:(NSString*)command pathname:(const char*)pat process = _lfsFilterProcesses[workingDirectory]; [_lfsFilterProcesses removeObjectForKey:workingDirectory]; } + *reused = process != nil; if (!process) { BOOL handshakeFailed = NO; process = [[GCLFSFilterProcess alloc] initWithExecutablePath:executablePath PATH:PATH workingDirectory:workingDirectory handshakeFailed:&handshakeFailed]; - if (handshakeFailed) { - @synchronized([GCLFSFilterProcess class]) { + @synchronized([GCLFSFilterProcess class]) { + if (process) { + _lfsFilterProcessLaunchCount += 1; + } else if (handshakeFailed) { _lfsFilterProcessUnsupported = YES; } } @@ -423,8 +523,15 @@ static void _CheckInLFSFilterProcess(GCLFSFilterProcess* process, NSString* work }); } +static NSString* _FilterErrorMessage(NSString* message, NSString* errorOutput) { + return errorOutput.length ? [NSString stringWithFormat:@"%@: %@", message, errorOutput] : message; +} + // Fallback executing git-lfs for this file only static int _GitLFSApplyWithTask(NSString* executablePath, NSString* PATH, NSString* workingDirectory, NSString* command, const git_buf* from, git_buf* to, const git_filter_source* src) { + @synchronized([GCLFSFilterProcess class]) { + _lfsFilterTaskCount += 1; + } GCTask* task = [[GCTask alloc] initWithExecutablePath:executablePath]; task.currentDirectoryPath = workingDirectory; if (PATH) { @@ -433,14 +540,16 @@ static int _GitLFSApplyWithTask(NSString* executablePath, NSString* PATH, NSStri int status; NSData* stdinData = [[NSData alloc] initWithBytesNoCopy:from->ptr length:from->size freeWhenDone:NO]; NSData* stdoutData; - if (![task runWithArguments:@[ command, @"--", GCFileSystemPathFromGitPath(git_filter_source_path(src)) ] stdin:stdinData stdout:&stdoutData stderr:NULL exitStatus:&status error:NULL]) { + NSData* stderrData; + if (![task runWithArguments:@[ command, @"--", GCFileSystemPathFromGitPath(git_filter_source_path(src)) ] stdin:stdinData stdout:&stdoutData stderr:&stderrData exitStatus:&status error:NULL]) { XLOG_ERROR(@"git-lfs tool failed executing"); giterr_set_str(GITERR_FILTER, "git-lfs tool failed executing"); return -1; } if (status != 0) { - XLOG_ERROR(@"git-lfs tool exited with non-zero status (%i)", status); - giterr_set_str(GITERR_FILTER, "git-lfs tool exited with non-zero status"); + NSString* errorOutput = [[[NSString alloc] initWithData:stderrData encoding:NSUTF8StringEncoding] stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]]; + XLOG_ERROR(@"git-lfs tool exited with non-zero status (%i): %@", status, errorOutput); + giterr_set_str(GITERR_FILTER, _FilterErrorMessage(@"git-lfs tool exited with non-zero status", errorOutput).UTF8String); return -1; } if (git_buf_set(to, stdoutData.bytes, stdoutData.length) < 0) { // TODO: Avoid copying data @@ -473,14 +582,20 @@ static int _GitLFSApply(git_filter* self, void** payload, git_buf* to, const git return -1; } - GCLFSFilterProcess* process = _CheckOutLFSFilterProcess(executablePath, PATH, workingDirectory); - if (process) { - GCFilterProcessResult result = [process runCommand:command pathname:git_filter_source_path(src) input:from output:to]; - if (result != kGCFilterProcessResult_Broken) { + // An idle process may have exited in the meantime (e.g. if the working directory was deleted) so retry with a new one + BOOL reused = YES; + while (reused) { + GCLFSFilterProcess* process = _CheckOutLFSFilterProcess(executablePath, PATH, workingDirectory, &reused); + if (!process) { + break; + } + NSString* errorOutput = nil; + GCFilterProcessResult result = [process runCommand:command pathname:git_filter_source_path(src) input:from output:to errorOutput:&errorOutput]; + if (process.running) { _CheckInLFSFilterProcess(process, workingDirectory); } if (result == kGCFilterProcessResult_Error) { - giterr_set_str(GITERR_FILTER, "git-lfs filter process failed"); + giterr_set_str(GITERR_FILTER, _FilterErrorMessage(@"git-lfs filter process failed", errorOutput).UTF8String); return -1; } if (result == kGCFilterProcessResult_Success) {