Find git-lfs in the user's PATH instead of hardcoding /usr/local/bin

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<NSString*>* GCDefaultExecutableDirectories(void);  // Homebrew (Apple Silicon and Intel) and MacPorts
+extern NSString* GCFindExecutableInPATH(NSString* name, NSString* PATH, NSArray<NSString*>* fallbackDirectories);  // Returns nil if not found
+
 #endif
 
 @interface GCObject () {

GitUpKit/Core/GCRepository-Tests.m

@@ -284,4 +284,69 @@ - (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)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];
+  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"];
+  [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

GitUpKit/Core/GCRepository.m

@@ -21,15 +21,10 @@
 #import <pthread.h>
 #if !TARGET_OS_IPHONE
 #import <git2/sys/filter.h>
-#import <sys/stat.h>
 #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,70 @@ 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]];
+}
+
+NSArray<NSString*>* GCDefaultExecutableDirectories(void) {
+  return @[ @"/opt/homebrew/bin", @"/usr/local/bin", @"/opt/local/bin" ];
+}
+
+NSString* GCFindExecutableInPATH(NSString* name, NSString* PATH, NSArray<NSString*>* 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.
+// 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);
+      executablePath = GCFindExecutableInPATH(@"git-lfs", PATH, GCDefaultExecutableDirectories());
+      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 +130,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 +187,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 +430,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 {