From c0103bc59d5b4a539f01bc9c98610ab3f11be114 Mon Sep 17 00:00:00 2001 From: Markus Duft Date: Tue, 5 Dec 2017 11:16:30 +0100 Subject: [PATCH] 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 --- .../META-INF/MANIFEST.MF | 1 + .../jgit/lfs/server/fs/CheckoutTest.java | 22 +++++++++++++++++-- .../jgit/lfs/internal/LfsText.properties | 1 + .../org/eclipse/jgit/lfs/SmudgeFilter.java | 5 +++++ .../eclipse/jgit/lfs/internal/LfsText.java | 1 + .../src/org/eclipse/jgit/pgm/Main.java | 3 +++ .../jgit/dircache/DirCacheCheckout.java | 15 ++++++++++--- 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 1589f4228..5ee3511b4 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 df43ccf41..67c8bd267 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.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 void setup() throws Exception { .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 void testUnknownContent() throws Exception { 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 29cee3a8e..0e00f146a 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 5c0d3b47a..e89b7b086 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 static Collection downloadLfsResource(Lfs lfs, Repository db, 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 d82ade2f7..d7d0fe186 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 static LfsText get() { /***/ 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 189a5acbf..a376bc098 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 Thread newThread(Runnable taskBody) { * @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 31500f517..5c85afd8d 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.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 @@ private static void runExternalFilterCommand(Repository repo, 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 @@ private static void runBuiltinFilterCommand(Repository repo, 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) {