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