LFS: Enable LFS support for the CLI, better error handling

Enable LFS support for the CLI by registering the according filters.

Errors during filter creation must be propagated up the call stack, as a
failure to create a filter should be treated as fatal if the filter is
required.

Change-Id: I3833757839bdda97cd01b6c21c1613d199e2692d
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>

7 files changed, 43 insertions, 5 deletions

diff --git a/org.eclipse.jgit.lfs.server.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.lfs.server.test/META-INF/MANIFEST.MF
index 1589f42285..5ee3511b45 100644
--- a/org.eclipse.jgit.lfs.server.test/META-INF/MANIFEST.MF
+++ b/org.eclipse.jgit.lfs.server.test/META-INF/MANIFEST.MF
@@ -29,6 +29,7 @@ Import-Package: javax.servlet;version="[3.1.0,4.0.0)",
  org.eclipse.jetty.util.security;version="[9.4.5,10.0.0)",
  org.eclipse.jetty.util.thread;version="[9.4.5,10.0.0)",
  org.eclipse.jgit.api;version="[4.11.0,4.12.0)",
+ org.eclipse.jgit.api.errors;version="[4.11.0,4.12.0)",
  org.eclipse.jgit.internal.storage.file;version="[4.11.0,4.12.0)",
  org.eclipse.jgit.junit;version="[4.11.0,4.12.0)",
  org.eclipse.jgit.junit.http;version="[4.11.0,4.12.0)",
diff --git a/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/CheckoutTest.java b/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/CheckoutTest.java
index df43ccf41b..67c8bd267c 100644
--- a/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/CheckoutTest.java
+++ b/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/CheckoutTest.java
@@ -49,10 +49,13 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 
 import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.JGitInternalException;
 import org.eclipse.jgit.junit.JGitTestUtil;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lfs.BuiltinLFS;
+import org.eclipse.jgit.lfs.lib.Constants;
 import org.eclipse.jgit.lfs.lib.LongObjectId;
+import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.storage.file.FileRepositoryBuilder;
@@ -78,8 +81,12 @@ public class CheckoutTest extends LfsServerTest {
 				.create(tmp.resolve(".git").toFile());
 		db.create();
 		StoredConfig cfg = db.getConfig();
-		cfg.setString("filter", "lfs", "usejgitbuiltin", "true");
-		cfg.setString("lfs", null, "url", server.getURI().toString() + "/lfs");
+		cfg.setBoolean(ConfigConstants.CONFIG_FILTER_SECTION, Constants.LFS,
+				ConfigConstants.CONFIG_KEY_USEJGITBUILTIN, true);
+		cfg.setBoolean(ConfigConstants.CONFIG_FILTER_SECTION, Constants.LFS,
+				ConfigConstants.CONFIG_KEY_REQUIRED, false);
+		cfg.setString(Constants.LFS, null, "url",
+				server.getURI().toString() + "/lfs");
 		cfg.save();
 
 		tdb = new TestRepository<>(db);
@@ -112,6 +119,17 @@ public class CheckoutTest extends LfsServerTest {
 				server.getRequests().toString());
 	}
 
+	@Test(expected = JGitInternalException.class)
+	public void testUnknownContentRequired() throws Exception {
+		StoredConfig cfg = tdb.getRepository().getConfig();
+		cfg.setBoolean(ConfigConstants.CONFIG_FILTER_SECTION, Constants.LFS,
+				ConfigConstants.CONFIG_KEY_REQUIRED, true);
+		cfg.save();
+
+		// must throw
+		git.checkout().setName("test").call();
+	}
+
 	@Test
 	public void testKnownContent() throws Exception {
 		putContent(
diff --git a/org.eclipse.jgit.lfs/resources/org/eclipse/jgit/lfs/internal/LfsText.properties b/org.eclipse.jgit.lfs/resources/org/eclipse/jgit/lfs/internal/LfsText.properties
index 29cee3a8e0..0e00f146ae 100644
--- a/org.eclipse.jgit.lfs/resources/org/eclipse/jgit/lfs/internal/LfsText.properties
+++ b/org.eclipse.jgit.lfs/resources/org/eclipse/jgit/lfs/internal/LfsText.properties
@@ -5,6 +5,7 @@ inconsistentContentLength=Unexpected content length reported by LFS server ({0})
 invalidLongId=Invalid id: {0}
 invalidLongIdLength=Invalid id length {0}; should be {1}
 lfsUnavailable=LFS is not available for repository {0}
+protocolError=LFS Protocol Error {0}: {1}
 requiredHashFunctionNotAvailable=Required hash function {0} not available.
 repositoryNotFound=Repository {0} not found
 repositoryReadOnly=Repository {0} is read-only
diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/SmudgeFilter.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/SmudgeFilter.java
index 5c0d3b47a7..e89b7b086b 100644
--- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/SmudgeFilter.java
+++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/SmudgeFilter.java
@@ -182,6 +182,11 @@ public class SmudgeFilter extends FilterCommand {
 			Protocol.Response resp = gson.fromJson(reader,
 					Protocol.Response.class);
 			for (Protocol.ObjectInfo o : resp.objects) {
+				if (o.error != null) {
+					throw new IOException(
+							MessageFormat.format(LfsText.get().protocolError,
+									o.error.code, o.error.message));
+				}
 				if (o.actions == null) {
 					continue;
 				}
diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsText.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsText.java
index d82ade2f7f..d7d0fe186a 100644
--- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsText.java
+++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsText.java
@@ -67,6 +67,7 @@ public class LfsText extends TranslationBundle {
 	/***/ public String invalidLongId;
 	/***/ public String invalidLongIdLength;
 	/***/ public String lfsUnavailable;
+	/***/ public String protocolError;
 	/***/ public String requiredHashFunctionNotAvailable;
 	/***/ public String repositoryNotFound;
 	/***/ public String repositoryReadOnly;
diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Main.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Main.java
index 189a5acbf3..a376bc098d 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Main.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Main.java
@@ -132,6 +132,9 @@ public class Main {
 	 * @throws java.lang.Exception
 	 */
 	public static void main(final String[] argv) throws Exception {
+		// make sure built-in filters are registered
+		BuiltinLFS.register();
+
 		new Main().run(argv);
 	}
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
index 31500f5171..5c85afd8df 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
@@ -67,6 +67,7 @@ import org.eclipse.jgit.errors.IndexWriteException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.events.WorkingTreeModifiedEvent;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.CoreConfig.AutoCRLF;
 import org.eclipse.jgit.lib.CoreConfig.EolStreamType;
@@ -1539,6 +1540,9 @@ public class DirCacheCheckout {
 	private static void runBuiltinFilterCommand(Repository repo,
 			CheckoutMetadata checkoutMetadata, ObjectLoader ol,
 			OutputStream channel) throws MissingObjectException, IOException {
+		boolean isMandatory = repo.getConfig().getBoolean(
+				ConfigConstants.CONFIG_FILTER_SECTION, "lfs",
+				ConfigConstants.CONFIG_KEY_REQUIRED, false);
 		FilterCommand command = null;
 		try {
 			command = FilterCommandRegistry.createFilterCommand(
@@ -1546,9 +1550,14 @@ public class DirCacheCheckout {
 					channel);
 		} catch (IOException e) {
 			LOG.error(JGitText.get().failedToDetermineFilterDefinition, e);
-			// In case an IOException occurred during creating of the command
-			// then proceed as if there would not have been a builtin filter.
-			ol.copyTo(channel);
+			if (!isMandatory) {
+				// In case an IOException occurred during creating of the
+				// command then proceed as if there would not have been a
+				// builtin filter (only if the filter is not mandatory).
+				ol.copyTo(channel);
+			} else {
+				throw e;
+			}
 		}
 		if (command != null) {
 			while (command.run() != -1) {