From ade9b51a1fd9be0bcb3fc9403e07dbc53de10baa Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 29 Apr 2020 16:13:04 -0700
Subject: [PATCH] Detect LFS by looking for tracks in ".gitattributes" instead
 of using "ls-tree"

Summary:
See PHI1718. See also <https://discourse.phabricator-community.org/t/arc-diff-fails-due-to-git-cmd-fails/3680/>.

Currently, `arc diff` detects Git LFS with `git ls-files -z -- ':(attr:filter=lfs)'` magic. This is an accurate test, but does not work on older Git.

Try a simpler, dumber test and see if that will work. If this also has issues, we can try this stuff:

  - do version detection;
  - pipe the whole tree to `git check-attr`;
  - try a command like `git lfs ls-files` instead, which is probably a wrapper on one of these other commands.

Test Plan:
  - In a non-LFS repository, ran "arc diff" and saw the repository detect as non-LFS.
  - In an LFS repository, ran "arc diff" and saw the repository detect as LFS.

Differential Revision: https://secure.phabricator.com/D21190
---
 src/repository/api/ArcanistGitAPI.php | 28 +++++++++++++++++++++++++++
 src/workflow/ArcanistDiffWorkflow.php |  3 +--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php
index b69c30b4..f19b5e12 100644
--- a/src/repository/api/ArcanistGitAPI.php
+++ b/src/repository/api/ArcanistGitAPI.php
@@ -1678,4 +1678,32 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
     return 'HEAD';
   }
 
+  public function isGitLFSWorkingCopy() {
+    // NOTE: This test previously used the command:
+    //
+    //   $ git ls-files -z -- ':(attr:filter=lfs)'
+    //
+    // However, this does not work on old versions of Git (see PHI1718) and
+    // it potentially does a lot of unnecessary work when run in a large tree.
+    //
+    // Instead, just check if ".gitattributes" exists and looks like it sets
+    // up an "lfs" filter for any pattern. This isn't completely accurate:
+    //
+    //   - LFS can be configured with a global ".gitattributes" file, although
+    //     this is unusual and discouraged by the official LFS documentation.
+    //   - LFS may be configured only for files that don't actually exist in
+    //     the repository.
+    //
+    // These cases are presumably very rare.
+
+    $attributes_path = $this->getPath('.gitattributes');
+    if (!Filesystem::pathExists($attributes_path)) {
+      return false;
+    }
+
+    $attributes_data = Filesystem::readFile($attributes_path);
+
+    return (bool)preg_match('(\bfilter\s*=\s*lfs\b)', $attributes_data);
+  }
+
 }
diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php
index bd7310ac..028adbe9 100644
--- a/src/workflow/ArcanistDiffWorkflow.php
+++ b/src/workflow/ArcanistDiffWorkflow.php
@@ -2900,8 +2900,7 @@ EOTEXT
       'uri' => $staging_uri,
     );
 
-    list($stdout) = $api->execxLocal('ls-files -z -- %s', ':(attr:filter=lfs)');
-    $is_lfs = strpos($stdout, "\0") !== false;
+    $is_lfs = $api->isGitLFSWorkingCopy();
 
     // If the base commit is a real commit, we're going to push it. We don't
     // use this, but pushing it to a ref reduces the amount of redundant work