From 06b1be6735a555c5d9277f00f08ff58362669a4e Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Fri, 16 Oct 2026 13:58:04 +0100 Subject: [PATCH 1/2] 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/2] 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 {