Fixing git-lfs checkout fail

For some reason after fetch+checkout git-lfs may not download actual files, leaving pointers in working directory. This behavior is tricky and hard to catch, so I've just added "bulletproof" solution: running `git lfs pull` every time (if git-lfs is available), just to be safe. Fixes #9
mutantcornholio · Aug 31, 2017 · ee53242 · ee53242
1 parent 8adb6e8
commit ee53242
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 12 deletions.
diff --git a/lib/backends/git-lfs.js b/lib/backends/git-lfs.js
@@ -72,6 +72,13 @@ function pull(hash, options, cacheDir) {
 
                 return gitWrapper.clone(options.repo, repoDir);
             })
+            .then(() => {
+                return new Promise((resolve, reject) => {
+                    gitWrapper.isGitLfsAvailable().then(() => {
+                        gitWrapper.lfsPull(repoDir).then(resolve, reject);
+                    }, resolve);
+                });
+            })
             .then(() => {
                 module.exports._remoteIsFresh = true;
                 return gitWrapper.checkout(repoDir, `veendor-${hash}`)

diff --git a/lib/commandWrappers/gitWrapper.js b/lib/commandWrappers/gitWrapper.js
@@ -56,6 +56,9 @@ module.exports = {
     fetch: (gitDirectory) => {
         return helpers.runInherited('git', ['fetch', '--tags'], {cwd: gitDirectory});
     },
+    lfsPull: (gitDirectory) => {
+        return helpers.runInherited('git', ['lfs', 'pull'], {cwd: gitDirectory});
+    },
     checkout: (gitDirectory, gitId) => {
         return helpers.getOutput('git', ['checkout', gitId], {cwd: gitDirectory});
     },

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "veendor",
-  "version": "1.3.0",
+  "version": "1.3.1",
   "description": "a tool for stroing your npm dependencies in arbitraty storage",
   "main": "bin/veendor.js",
   "bin": {

diff --git a/test/unit/backends/git-lfs.test.js b/test/unit/backends/git-lfs.test.js
@@ -49,6 +49,8 @@ describe('git-lfs', () => {
         sandbox.stub(gitWrapper, 'push').resolves();
         sandbox.stub(gitWrapper, 'isGitRepo').resolves();
         sandbox.stub(gitWrapper, 'resetToRemote').resolves();
+        sandbox.stub(gitWrapper, 'isGitLfsAvailable').resolves();
+        sandbox.stub(gitWrapper, 'lfsPull').resolves();
         sandbox.stub(tarWrapper, 'createArchive').resolves();
         sandbox.stub(tarWrapper, 'extractArchive').resolves();
     });
@@ -109,17 +111,13 @@ describe('git-lfs', () => {
 
         it('should run git fetch only once in a run', done => {
             const checkResult = () => {
-                try {
-                    const calls = gitWrapper.fetch.getCalls();
-                    assert.equal(
-                        calls.length,
-                        1,
-                        `Expected 'gitWrapper.fetch' to be called once`
-                    );
-                    done();
-                } catch (e) {
-                    done(e);
-                }
+                const calls = gitWrapper.fetch.getCalls();
+                notifyAssert(assert.equal.bind(
+                    null,
+                    calls.length,
+                    1,
+                    `Expected 'gitWrapper.fetch' to be called once`
+                ), done);
             };
 
             gitLfs.pull(fakeHash, defaultOptions, '.veendor/git-lfs.0')
@@ -131,6 +129,32 @@ describe('git-lfs', () => {
                 })
                 .then(checkResult, checkResult);
         });
+
+        it('should run git lfs pull if git lfs is available', done => {
+            const checkResult = expectCalls.bind(null, [{
+                spy: gitWrapper.lfsPull,
+                args: [sinon.match('.veendor/git-lfs.0/repo')]
+            }], done);
+
+            gitLfs.pull(fakeHash, defaultOptions, '.veendor/git-lfs.0').then(checkResult, checkResult);
+        });
+
+        it('should not run git lfs pull if git lfs is not available', done => {
+            gitWrapper.isGitLfsAvailable.restore();
+            sandbox.stub(gitWrapper, 'isGitLfsAvailable').rejects(new gitWrapper.GitLfsNotAvailableError);
+
+            const checkResult = () => {
+                const calls = gitWrapper.lfsPull.getCalls();
+                notifyAssert(assert.equal.bind(
+                    null,
+                    calls.length,
+                    0,
+                    `Expected 'gitWrapper.lfsPull' not to be called`
+                ), done);
+            };
+
+            gitLfs.pull(fakeHash, defaultOptions, '.veendor/git-lfs.0').then(checkResult, checkResult);
+        });
     });
 
     describe('.push', () => {
@@ -286,6 +310,7 @@ describe('git-lfs', () => {
             'and `checkLfsAvailability` was set to \'true\'', done => {
             defaultOptions.checkLfsAvailability = true;
 
+            gitWrapper.isGitLfsAvailable.restore();
             sandbox.stub(gitWrapper, 'isGitLfsAvailable').rejects(new gitWrapper.GitLfsNotAvailableError);
 
             assert.isRejected(gitLfs.validateOptions(defaultOptions), gitWrapper.GitLfsNotAvailableError).notify(done);