From ad02a7cbada59560fde64f9ecb361aabbd08b235 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 9 Aug 2019 12:11:41 +0900 Subject: [PATCH 01/24] Stop using deprecated Constants.CHARACTER_ENCODING Change-Id: I105b8a05bc64f249879a0795a059958553cc60c6 Signed-off-by: David Pursehouse --- .../tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java | 2 +- .../src/org/eclipse/jgit/merge/ResolveMerger.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java index b13180831..61ab04289 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java @@ -302,7 +302,7 @@ public class MergeAlgorithmTest { MergeResult r = new MergeAlgorithm().merge(RawTextComparator.DEFAULT, T(commonBase), T(ours), T(theirs)); ByteArrayOutputStream bo=new ByteArrayOutputStream(50); - fmt.formatMerge(bo, r, "B", "O", "T", Constants.CHARACTER_ENCODING); + fmt.formatMerge(bo, r, "B", "O", "T", UTF_8.name()); return new String(bo.toByteArray(), UTF_8); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java index d4282e0b8..e37f20731 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java @@ -46,11 +46,11 @@ */ package org.eclipse.jgit.merge; +import static java.nio.charset.StandardCharsets.UTF_8; import static java.time.Instant.EPOCH; import static org.eclipse.jgit.diff.DiffAlgorithm.SupportedAlgorithm.HISTOGRAM; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_DIFF_SECTION; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_ALGORITHM; -import static org.eclipse.jgit.lib.Constants.CHARACTER_ENCODING; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; import java.io.BufferedOutputStream; @@ -1029,7 +1029,7 @@ public class ResolveMerger extends ThreeWayMerger { db != null ? nonNullRepo().getDirectory() : null, inCoreLimit); try { new MergeFormatter().formatMerge(buf, result, - Arrays.asList(commitNames), CHARACTER_ENCODING); + Arrays.asList(commitNames), UTF_8.name()); buf.close(); } catch (IOException e) { buf.destroy(); From 42000a71e3b73639f8ba2858f8ba8e7941a1ab1c Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 10 Aug 2019 00:39:16 +0200 Subject: [PATCH 02/24] LockFile: log exception if creation of lock file failed Change-Id: I4de75d12ec9e61193494916307289378cdb6220e Signed-off-by: Matthias Sohn --- .../org/eclipse/jgit/internal/JGitText.properties | 5 +++-- .../src/org/eclipse/jgit/internal/JGitText.java | 1 + .../org/eclipse/jgit/internal/storage/file/LockFile.java | 7 ++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 290a0a28e..cba892ef6 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -302,8 +302,9 @@ expectedLessThanGot=expected less than ''{0}'', got ''{1}'' expectedPktLineWithService=expected pkt-line with ''# service=-'', got ''{0}'' expectedReceivedContentType=expected Content-Type {0}; received Content-Type {1} expectedReportForRefNotReceived={0}: expected report for ref {1} not received -failedAtomicFileCreation=Atomic file creation failed, number of hard links to file {0} was not 2 but {1}" -failedToDetermineFilterDefinition=An exception occured while determining filter definitions +failedAtomicFileCreation=Atomic file creation failed, number of hard links to file {0} was not 2 but {1} +failedCreateLockFile=Creating lock file {} failed +failedToDetermineFilterDefinition=An exception occurred while determining filter definitions failedUpdatingRefs=failed updating refs failureDueToOneOfTheFollowing=Failure due to one of the following: failureUpdatingFETCH_HEAD=Failure updating FETCH_HEAD: {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 00aaa42b4..23950100d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -364,6 +364,7 @@ public class JGitText extends TranslationBundle { /***/ public String expectedReceivedContentType; /***/ public String expectedReportForRefNotReceived; /***/ public String failedAtomicFileCreation; + /***/ public String failedCreateLockFile; /***/ public String failedToDetermineFilterDefinition; /***/ public String failedUpdatingRefs; /***/ public String failureDueToOneOfTheFollowing; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java index 420e73754..f7e78b94f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java @@ -165,7 +165,12 @@ public class LockFile { */ public boolean lock() throws IOException { FileUtils.mkdirs(lck.getParentFile(), true); - token = FS.DETECTED.createNewFileAtomic(lck); + try { + token = FS.DETECTED.createNewFileAtomic(lck); + } catch (IOException e) { + LOG.error(JGitText.get().failedCreateLockFile, lck, e); + throw e; + } if (token.isCreated()) { haveLck = true; try { From 06fc6c7cc0219b3ec515eae70eb3858e29f6d485 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 10 Aug 2019 00:51:30 +0200 Subject: [PATCH 03/24] Bazel: enable logging for tests in org.eclipse.jgit.test - use slf4j-simple for logging in test runs - for log configuration see https://www.slf4j.org/api/org/slf4j/impl/SimpleLogger.html Change-Id: I9f0a532644b31162c867cd0d63f083296eaf6be5 Signed-off-by: Matthias Sohn --- org.eclipse.jgit.test/BUILD | 1 + org.eclipse.jgit.test/tst-rsrc/log4j.properties | 6 +++--- org.eclipse.jgit.test/tst-rsrc/simplelogger.properties | 9 +++++++++ 3 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 org.eclipse.jgit.test/tst-rsrc/simplelogger.properties diff --git a/org.eclipse.jgit.test/BUILD b/org.eclipse.jgit.test/BUILD index b06778eed..89bc90f38 100644 --- a/org.eclipse.jgit.test/BUILD +++ b/org.eclipse.jgit.test/BUILD @@ -42,6 +42,7 @@ java_library( resources = DATA, deps = [ "//lib:junit", + "//lib:slf4j-simple", "//org.eclipse.jgit:jgit", "//org.eclipse.jgit.junit:junit", ], diff --git a/org.eclipse.jgit.test/tst-rsrc/log4j.properties b/org.eclipse.jgit.test/tst-rsrc/log4j.properties index ee1ac3515..fad593327 100644 --- a/org.eclipse.jgit.test/tst-rsrc/log4j.properties +++ b/org.eclipse.jgit.test/tst-rsrc/log4j.properties @@ -7,8 +7,8 @@ log4j.appender.stdout=org.apache.log4j.ConsoleAppender log4j.appender.stdout.Target=System.out log4j.appender.stdout.layout=org.apache.log4j.PatternLayout log4j.appender.stdout.layout.ConversionPattern=%d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n -#log4j.appender.fileLogger.bufferedIO = true -#log4j.appender.fileLogger.bufferSize = 4096 +log4j.appender.fileLogger.bufferedIO = true +log4j.appender.fileLogger.bufferSize = 4096 -#log4j.logger.org.eclipse.jgit.util.FS = DEBUG +log4j.logger.org.eclipse.jgit.util.FS = DEBUG #log4j.logger.org.eclipse.jgit.internal.storage.file.FileSnapshot = DEBUG diff --git a/org.eclipse.jgit.test/tst-rsrc/simplelogger.properties b/org.eclipse.jgit.test/tst-rsrc/simplelogger.properties new file mode 100644 index 000000000..011b2f8bb --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/simplelogger.properties @@ -0,0 +1,9 @@ +org.slf4j.simpleLogger.logFile = System.err +org.slf4j.simpleLogger.cacheOutputStream = true +org.slf4j.simpleLogger.defaultLogLevel = info +org.slf4j.simpleLogger.showDateTime = true +org.slf4j.simpleLogger.dateTimeFormat = HH:mm:ss.SSSXXX +org.slf4j.simpleLogger.showThreadName = true + +#org.slf4j.simpleLogger.log.org.eclipse.jgit.util.FS = debug +#org.slf4j.simpleLogger.log.org.eclipse.jgit.internal.storage.file.FileSnapshot = debug From 0bc89fabe279ae3ec5eef28ca551becf90816d26 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 10 Aug 2019 20:32:07 +0200 Subject: [PATCH 04/24] Disable debug log for FS in org.eclipse.jgit.test This was enabled unintentionally in 06fc6c7c and spams the test logs. We can enable this when needed. Change-Id: I9f3042c0e285ff236be65fcc02bdcfdb90efc3af Signed-off-by: Matthias Sohn --- org.eclipse.jgit.test/tst-rsrc/log4j.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit.test/tst-rsrc/log4j.properties b/org.eclipse.jgit.test/tst-rsrc/log4j.properties index fad593327..856a731ab 100644 --- a/org.eclipse.jgit.test/tst-rsrc/log4j.properties +++ b/org.eclipse.jgit.test/tst-rsrc/log4j.properties @@ -10,5 +10,5 @@ log4j.appender.stdout.layout.ConversionPattern=%d{yyyy-MM-dd HH:mm:ss} %-5p %c{1 log4j.appender.fileLogger.bufferedIO = true log4j.appender.fileLogger.bufferSize = 4096 -log4j.logger.org.eclipse.jgit.util.FS = DEBUG +#log4j.logger.org.eclipse.jgit.util.FS = DEBUG #log4j.logger.org.eclipse.jgit.internal.storage.file.FileSnapshot = DEBUG From a7338dd1e5173b081d0ec1a7f31f48ed34b07d70 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 10 Aug 2019 23:28:07 +0200 Subject: [PATCH 05/24] Remove FileBasedConfig.load(boolean) introduced in d45219ba We can't add this method to the super class StoredConfig since that abstracts from filesystem storage. MockSystemReader.MockConfig is a StoredConfig and is also used by tests for dfs based storage. Hence remove this leaky abstraction. This implies we always use the fallback FileStoreAttributes which means a config file modification is considered racy within the first 2 seconds. This should not be an issue since typically configs change rarely and re-reading a config within the racy period is relatively cheap since configs are small. Change-Id: Ia2615addc24a7cadf3c566ee842c6f4f07e159a5 Signed-off-by: Matthias Sohn --- org.eclipse.jgit/.settings/.api_filters | 8 ------ .../jgit/storage/file/FileBasedConfig.java | 28 ++----------------- .../src/org/eclipse/jgit/util/FS.java | 22 +++++++-------- 3 files changed, 12 insertions(+), 46 deletions(-) diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 7aa7301b0..5874cd51f 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -78,14 +78,6 @@ - - - - - - - - diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java index 633632dc0..e7b0941f2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java @@ -149,37 +149,13 @@ public class FileBasedConfig extends StoredConfig { */ @Override public void load() throws IOException, ConfigInvalidException { - load(true); - } - - /** - * Load the configuration as a Git text style configuration file. - *

- * If the file does not exist, this configuration is cleared, and thus - * behaves the same as though the file exists, but is empty. - * - * @param useFileSnapshotWithConfig - * if {@code true} use the FileSnapshot with config, otherwise - * use it without config - * @throws IOException - * if IO failed - * @throws ConfigInvalidException - * if config is invalid - * @since 5.1.9 - */ - public void load(boolean useFileSnapshotWithConfig) - throws IOException, ConfigInvalidException { final int maxStaleRetries = 5; int retries = 0; while (true) { final FileSnapshot oldSnapshot = snapshot; final FileSnapshot newSnapshot; - if (useFileSnapshotWithConfig) { - newSnapshot = FileSnapshot.save(getFile()); - } else { - // don't use config in this snapshot to avoid endless recursion - newSnapshot = FileSnapshot.saveNoConfig(getFile()); - } + // don't use config in this snapshot to avoid endless recursion + newSnapshot = FileSnapshot.saveNoConfig(getFile()); try { final byte[] in = IO.readFully(getFile()); final ObjectId newHash = hash(in); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java index 19c04619d..509e6d228 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java @@ -100,7 +100,7 @@ import org.eclipse.jgit.internal.storage.file.FileSnapshot; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.treewalk.FileTreeIterator.FileEntry; import org.eclipse.jgit.treewalk.FileTreeIterator.FileModeStrategy; import org.eclipse.jgit.treewalk.WorkingTreeIterator.Entry; @@ -510,17 +510,17 @@ public abstract class FS { private static Optional readFromConfig( FileStore s) { - FileBasedConfig userConfig = SystemReader.getInstance() + StoredConfig userConfig = SystemReader.getInstance() .openUserConfig(null, FS.DETECTED); try { - userConfig.load(false); + userConfig.load(); } catch (IOException e) { LOG.error(MessageFormat.format(JGitText.get().readConfigFailed, - userConfig.getFile().getAbsolutePath()), e); + userConfig), e); } catch (ConfigInvalidException e) { LOG.error(MessageFormat.format( JGitText.get().repositoryConfigFileInvalid, - userConfig.getFile().getAbsolutePath(), + userConfig, e.getMessage())); } String key = getConfigKey(s); @@ -544,7 +544,7 @@ public abstract class FS { private static void saveToConfig(FileStore s, FileStoreAttributes c) { - FileBasedConfig userConfig = SystemReader.getInstance() + StoredConfig userConfig = SystemReader.getInstance() .openUserConfig(null, FS.DETECTED); long resolution = c.getFsTimestampResolution().toNanos(); TimeUnit resolutionUnit = getUnit(resolution); @@ -562,7 +562,7 @@ public abstract class FS { String key = getConfigKey(s); while (!succeeded && retries < max_retries) { try { - userConfig.load(false); + userConfig.load(); userConfig.setString( ConfigConstants.CONFIG_FILESYSTEM_SECTION, key, ConfigConstants.CONFIG_KEY_TIMESTAMP_RESOLUTION, @@ -581,7 +581,7 @@ public abstract class FS { // race with another thread, wait a bit and try again try { LOG.warn(MessageFormat.format(JGitText.get().cannotLock, - userConfig.getFile().getAbsolutePath())); + userConfig)); retries++; Thread.sleep(20); } catch (InterruptedException e1) { @@ -589,13 +589,11 @@ public abstract class FS { } } catch (IOException e) { LOG.error(MessageFormat.format( - JGitText.get().cannotSaveConfig, - userConfig.getFile().getAbsolutePath()), e); + JGitText.get().cannotSaveConfig, userConfig), e); } catch (ConfigInvalidException e) { LOG.error(MessageFormat.format( JGitText.get().repositoryConfigFileInvalid, - userConfig.getFile().getAbsolutePath(), - e.getMessage())); + userConfig, e.getMessage())); } } } From 65cee6ccbf0526e94f1cdb7fd06597c6ffa7c01a Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 10 Aug 2019 23:48:42 +0200 Subject: [PATCH 06/24] Override FileBasedConfig's save method in MockConfig This ensures we don't try to persist MockConfig using its superclasses save() method which fails with an NPE since MockConfig has no backing file. Change-Id: Ifba2d24c9438bb30d3828ed31a4c131f940b45eb Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/junit/MockSystemReader.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java index 8de386e34..abbbf3296 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java @@ -80,6 +80,11 @@ public class MockSystemReader extends SystemReader { // Do nothing } + @Override + public void save() throws IOException { + // Do nothing + } + @Override public boolean isOutdated() { return false; From 12d03a97d8c2796a0728afb4e00d4d1c32b93f8b Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 10 Aug 2019 23:34:58 +0200 Subject: [PATCH 07/24] Ensure we use MockSystemReader in tests If we use the default system reader FileStoreAttributes cannot persist attributes in userConfig when tests run in Bazel due to sandboxing. Hence we need to ensure that all tests use MockSystemReader (and especially a mocked userConfig). Change-Id: Ic1ad8e2ec5a150c5433434a5f6667d6c4674c87d Signed-off-by: Matthias Sohn --- .../tst/org/eclipse/jgit/lfs/server/fs/LfsServerTest.java | 3 +++ .../eclipse/jgit/internal/storage/file/FileSnapshotTest.java | 2 ++ .../tst/org/eclipse/jgit/storage/file/FileBasedConfigTest.java | 3 +++ 3 files changed, 8 insertions(+) diff --git a/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/LfsServerTest.java b/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/LfsServerTest.java index ec44da4ca..e55c4c643 100644 --- a/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/LfsServerTest.java +++ b/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/LfsServerTest.java @@ -74,6 +74,7 @@ import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jgit.junit.MockSystemReader; import org.eclipse.jgit.junit.http.AppServer; import org.eclipse.jgit.lfs.errors.LfsException; import org.eclipse.jgit.lfs.lib.AnyLongObjectId; @@ -85,6 +86,7 @@ import org.eclipse.jgit.lfs.test.LongObjectIdTestUtils; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.FileUtils; import org.eclipse.jgit.util.IO; +import org.eclipse.jgit.util.SystemReader; import org.junit.After; import org.junit.Before; @@ -119,6 +121,7 @@ public abstract class LfsServerTest { @Before public void setup() throws Exception { + SystemReader.setInstance(new MockSystemReader()); tmp = Files.createTempDirectory("jgit_test_"); // measure timer resolution before the test to avoid time critical tests diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileSnapshotTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileSnapshotTest.java index 6fa35d64b..35f4fcba9 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileSnapshotTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileSnapshotTest.java @@ -62,6 +62,7 @@ import java.time.Instant; import java.util.ArrayList; import java.util.concurrent.TimeUnit; +import org.eclipse.jgit.junit.MockSystemReader; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.FS.FileStoreAttributes; import org.eclipse.jgit.util.FileUtils; @@ -84,6 +85,7 @@ public class FileSnapshotTest { @Before public void setUp() throws Exception { + SystemReader.setInstance(new MockSystemReader()); trash = Files.createTempDirectory("tmp_"); // measure timer resolution before the test to avoid time critical tests // are affected by time needed for measurement diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/FileBasedConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/FileBasedConfigTest.java index 1adddb5ea..f3cd61da6 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/FileBasedConfigTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/FileBasedConfigTest.java @@ -55,9 +55,11 @@ import java.nio.file.Path; import java.nio.file.StandardOpenOption; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.junit.MockSystemReader; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.FileUtils; import org.eclipse.jgit.util.IO; +import org.eclipse.jgit.util.SystemReader; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -82,6 +84,7 @@ public class FileBasedConfigTest { @Before public void setUp() throws Exception { + SystemReader.setInstance(new MockSystemReader()); trash = Files.createTempDirectory("tmp_"); FS.getFileStoreAttributes(trash.getParent()); } From c28167e1036fe132f8a66db7943d54ee8d869d34 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 10 Aug 2019 23:44:39 +0200 Subject: [PATCH 08/24] Ensure LocalDiskRepositoryTestCase#setup fully uses MockSystemReader FS#getFileStoreAttributes used the real userConfig and not the mocked one. This led to test errors when running tests with Bazel since it sandboxes tests which prevents they can write to ~/.gitconfig. Fix this by first preparing the MockedSystemReader and the mocked config before calling FS#getFileStoreAttributes. Also fix ConfigTest which broke due to this change since it inherits from LocalDiskRepositoryTestCase and calls its setup method which was changed here. We can no longer assert by comparing plain text since FS adds FileStoreAttributes to the mocked userConfig. Also the default options seen by this test changed since we now use a mocked config. Change-Id: I76bc7c94953fe979266147d3b309a68dda9d4dfe Signed-off-by: Matthias Sohn --- .../junit/LocalDiskRepositoryTestCase.java | 12 ++--- .../tst/org/eclipse/jgit/pgm/ConfigTest.java | 45 ++++++++++++------- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java index af23ad1e3..cce83c00b 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java @@ -128,11 +128,8 @@ public abstract class LocalDiskRepositoryTestCase { if (!tmp.delete() || !tmp.mkdir()) throw new IOException("Cannot create " + tmp); - // measure timer resolution before the test to avoid time critical tests - // are affected by time needed for measurement - FS.getFileStoreAttributes(tmp.toPath().getParent()); - mockSystemReader = new MockSystemReader(); + SystemReader.setInstance(mockSystemReader); mockSystemReader.userGitConfig = new FileBasedConfig(new File(tmp, "usergitconfig"), FS.DETECTED); // We have to set autoDetach to false for tests, because tests expect to be able @@ -142,7 +139,12 @@ public abstract class LocalDiskRepositoryTestCase { null, ConfigConstants.CONFIG_KEY_AUTODETACH, false); mockSystemReader.userGitConfig.save(); ceilTestDirectories(getCeilings()); - SystemReader.setInstance(mockSystemReader); + + // Measure timer resolution before the test to avoid time critical tests + // are affected by time needed for measurement. + // The MockSystemReader must be configured first since we need to use + // the same one here + FS.getFileStoreAttributes(tmp.toPath().getParent()); author = new PersonIdent("J. Author", "jauthor@example.com"); committer = new PersonIdent("J. Committer", "jcommitter@example.com"); diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ConfigTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ConfigTest.java index 0ce645139..1ce86d15e 100644 --- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ConfigTest.java +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ConfigTest.java @@ -43,14 +43,14 @@ package org.eclipse.jgit.pgm; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; -import java.util.ArrayList; import java.util.Arrays; -import java.util.List; +import java.util.HashMap; +import java.util.Map; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.lib.CLIRepositoryTestCase; -import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.SystemReader; import org.junit.Before; import org.junit.Test; @@ -65,6 +65,7 @@ public class ConfigTest extends CLIRepositoryTestCase { } } + @SuppressWarnings("boxing") @Test public void testListConfig() throws Exception { boolean isWindows = SystemReader.getInstance().getProperty("os.name") @@ -73,19 +74,31 @@ public class ConfigTest extends CLIRepositoryTestCase { .equals("Mac OS X"); String[] output = execute("git config --list"); - List expect = new ArrayList<>(); - expect.add("gc.autoDetach=false"); - expect.add("core.filemode=" + !isWindows); - expect.add("core.logallrefupdates=true"); - if (isMac) - expect.add("core.precomposeunicode=true"); - expect.add("core.repositoryformatversion=0"); - if (!FS.DETECTED.supportsSymlinks()) - expect.add("core.symlinks=false"); - expect.add(""); // ends with LF (last line empty) - assertEquals("expected default configuration", - Arrays.asList(expect.toArray()).toString(), - Arrays.asList(output).toString()); + + Map options = parseOptions(output); + + assertEquals(!isWindows, Boolean.valueOf(options.get("core.filemode"))); + assertTrue((Boolean.valueOf(options.get("core.logallrefupdates")))); + if (isMac) { + assertTrue( + (Boolean.valueOf(options.get("core.precomposeunicode")))); + } + assertEquals(Integer.valueOf(0), + Integer.valueOf(options.get("core.repositoryformatversion"))); + } + + private Map parseOptions(String[] output) { + Map options = new HashMap<>(); + Arrays.stream(output).forEachOrdered(s -> { + int p = s.indexOf('='); + if (p == -1) { + return; + } + String key = s.substring(0, p); + String value = s.substring(p + 1); + options.put(key, value); + }); + return options; } } From 118ee616400f66bdb6da5de9fa1479d14d5cb122 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 10 Aug 2019 23:45:43 +0200 Subject: [PATCH 09/24] LocalDiskRefTreeDatabaseTest shall use MockSystemReader It missed to call the setup() method of its super class which prepares the MockSystemReader Change-Id: I39858749f8d0115fc6ac7edc8847ffb2bbc85c33 Signed-off-by: Matthias Sohn --- .../internal/storage/reftree/LocalDiskRefTreeDatabaseTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftree/LocalDiskRefTreeDatabaseTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftree/LocalDiskRefTreeDatabaseTest.java index d5a07e02f..cafec04f7 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftree/LocalDiskRefTreeDatabaseTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftree/LocalDiskRefTreeDatabaseTest.java @@ -80,6 +80,7 @@ public class LocalDiskRefTreeDatabaseTest extends LocalDiskRepositoryTestCase { @Override @Before public void setUp() throws Exception { + super.setUp(); FileRepository init = createWorkRepository(); FileBasedConfig cfg = init.getConfig(); cfg.setInt("core", null, "repositoryformatversion", 1); From 9add6d3e24278ec8c8075a99490ea397c387d4ce Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 10 Aug 2019 23:46:50 +0200 Subject: [PATCH 10/24] Implement toString in MockSystemReader and MockConfig This helps when debugging. Change-Id: I3d72e1ea207ba60be77a7a9a840abb71ade1271c Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/junit/MockSystemReader.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java index abbbf3296..92b531d19 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java @@ -89,6 +89,11 @@ public class MockSystemReader extends SystemReader { public boolean isOutdated() { return false; } + + @Override + public String toString() { + return "MockConfig"; + } } long now = 1250379778668L; // Sat Aug 15 20:12:58 GMT-03:30 2009 @@ -285,4 +290,10 @@ public class MockSystemReader extends SystemReader { e.printStackTrace(); } } + + @Override + public String toString() { + return "MockSystemReader"; + } + } From cc29da8e0f228867205331ec918e3cc17dcfecb8 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 12 Aug 2019 07:14:54 +0200 Subject: [PATCH 11/24] Ensure root cause of lock creation failures is logged Change-Id: I91cdf1e085a29c0aabd6d22c6ebe848b2d75f42c Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/util/FS_POSIX.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java index 8795329bf..03b94ee11 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java @@ -48,6 +48,7 @@ import java.io.IOException; import java.io.InputStreamReader; import java.io.PrintStream; import java.nio.charset.Charset; +import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -422,7 +423,7 @@ public class FS_POSIX extends FS { * An implementation of the File#createNewFile() semantics which can create * a unique file atomically also on NFS. If the config option * {@code core.supportsAtomicCreateNewFile = true} (which is the default) - * then simply File#createNewFile() is called. + * then simply Files#createFile() is called. * * But if {@code core.supportsAtomicCreateNewFile = false} then after * successful creation of the lock file a hard link to that lock file is @@ -443,14 +444,17 @@ public class FS_POSIX extends FS { */ @Override public LockToken createNewFileAtomic(File file) throws IOException { - if (!file.createNewFile()) { + Path path; + try { + path = file.toPath(); + Files.createFile(path); + } catch (FileAlreadyExistsException e) { return token(false, null); } if (supportsAtomicCreateNewFile() || !supportsUnixNLink) { return token(true, null); } Path link = null; - Path path = file.toPath(); try { link = Files.createLink(Paths.get(uniqueLinkPath(file)), path); Integer nlink = (Integer) (Files.getAttribute(path, From 6ef225c5191f0e0c7ffc621843528568455d72a1 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 12 Aug 2019 12:18:16 +0200 Subject: [PATCH 12/24] Handle InvalidPathException in FS_POSIX#createNewFileAtomic Bug: 547400 Change-Id: Ic3541e360a2968ba3532a3d3fa4828b0d0463c02 Signed-off-by: Matthias Sohn --- org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java index 03b94ee11..8561f6f3a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java @@ -50,6 +50,7 @@ import java.io.PrintStream; import java.nio.charset.Charset; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.attribute.PosixFilePermission; @@ -448,7 +449,7 @@ public class FS_POSIX extends FS { try { path = file.toPath(); Files.createFile(path); - } catch (FileAlreadyExistsException e) { + } catch (FileAlreadyExistsException | InvalidPathException e) { return token(false, null); } if (supportsAtomicCreateNewFile() || !supportsUnixNLink) { From 4d46a68abee7a59bba7c5d85b5ad90206bc1d757 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 12 Aug 2019 12:02:59 +0200 Subject: [PATCH 13/24] Update orbit to R20190602212107-2019-06 to enable backports from master update - org.apache.httpcomponents.httpclient to 4.5.6.v20190503-0009 - org.apache.httpcomponents.httpcore to 4.4.10.v20190123-2214 - com.jcraft.jsch" version to 0.1.55.v20190404-1902 - org.mockito to 2.23.0.v20190527-1420 add its dependencies - net.bytebuddy.byte-buddy 1.9.0.v20181107-1410 - net.bytebuddy.byte-buddy-agent 1.9.0.v20181106-1534 - org.objenesis to 2.6.0.v20180420-1519 CQ: 18303 (objenesis) CQ: 18703 (httpclient) CQ: 18704 (httpcore) CQ: 19588 (jsch) CQ: 20214 (mockito) CQ: 20215 (butebuddy) CQ: 20216 (bytebuddy-agent) Change-Id: I8d76375de867e32b7818449f843b7c053a717b5e Signed-off-by: Matthias Sohn --- WORKSPACE | 36 +++++++++++++++---- lib/BUILD | 4 +++ .../org.eclipse.jgit.target/jgit-4.5.target | 30 +++++++++------- .../org.eclipse.jgit.target/jgit-4.5.tpd | 2 +- .../org.eclipse.jgit.target/jgit-4.6.target | 30 +++++++++------- .../org.eclipse.jgit.target/jgit-4.6.tpd | 2 +- .../org.eclipse.jgit.target/jgit-4.7.target | 30 +++++++++------- .../org.eclipse.jgit.target/jgit-4.7.tpd | 2 +- .../org.eclipse.jgit.target/jgit-4.8.target | 30 +++++++++------- .../org.eclipse.jgit.target/jgit-4.8.tpd | 2 +- .../jgit-4.9-staging.target | 30 +++++++++------- .../jgit-4.9-staging.tpd | 2 +- ...018-09.tpd => R20190602212107-2019-06.tpd} | 28 ++++++++------- org.eclipse.jgit.test/pom.xml | 6 ++++ pom.xml | 6 ++-- 15 files changed, 149 insertions(+), 91 deletions(-) rename org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/{R20180905201904-2018-09.tpd => R20190602212107-2019-06.tpd} (67%) diff --git a/WORKSPACE b/WORKSPACE index 649f2b2e9..8aa52d4aa 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -11,8 +11,8 @@ load( maven_jar( name = "jsch", - artifact = "com.jcraft:jsch:0.1.54", - sha1 = "da3584329a263616e277e15462b387addd1b208d", + artifact = "com.jcraft:jsch:0.1.55", + sha1 = "bbd40e5aa7aa3cfad5db34965456cee738a42a50", ) maven_jar( @@ -29,14 +29,14 @@ maven_jar( maven_jar( name = "httpclient", - artifact = "org.apache.httpcomponents:httpclient:4.5.5", - sha1 = "1603dfd56ebcd583ccdf337b6c3984ac55d89e58", + artifact = "org.apache.httpcomponents:httpclient:4.5.6", + sha1 = "1afe5621985efe90a92d0fbc9be86271efbe796f", ) maven_jar( name = "httpcore", - artifact = "org.apache.httpcomponents:httpcore:4.4.9", - sha1 = "a86ce739e5a7175b4b234c290a00a5fdb80957a0", + artifact = "org.apache.httpcomponents:httpcore:4.4.10", + sha1 = "acc54d9b28bdffe4bbde89ed2e4a1e86b5285e2b", ) maven_jar( @@ -105,6 +105,30 @@ maven_jar( sha1 = "42a25dc3219429f0e5d060061f71acb49bf010a0", ) +maven_jar( + name = "mockito-core", + artifact = "org.mockito:mockito-core:2.23.0", + sha1 = "497ddb32fd5d01f9dbe99a2ec790aeb931dff1b1", +) + +maven_jar( + name = "bytebuddy", + artifact = "net.bytebuddy:byte-buddy:1.9.0", + sha1 = "8cb0d5baae526c9df46ae17693bbba302640538b", +) + +maven_jar( + name = "bytebuddy-agent", + artifact = "net.bytebuddy:byte-buddy-agent:1.9.0", + sha1 = "37b5703b4a6290be3fffc63ae9c6bcaaee0ff856", +) + +maven_jar( + name = "objenesis", + artifact = "org.objenesis:objenesis:2.6", + sha1 = "639033469776fd37c08358c6b92a4761feb2af4b", +) + maven_jar( name = "gson", artifact = "com.google.code.gson:gson:2.8.2", diff --git a/lib/BUILD b/lib/BUILD index b188265a8..1b6f1d772 100644 --- a/lib/BUILD +++ b/lib/BUILD @@ -132,9 +132,13 @@ java_library( testonly = 1, visibility = ["//visibility:public"], exports = [ + "@bytebuddy//jar", + "@bytebuddy-agent//jar", "@hamcrest-core//jar", "@hamcrest-library//jar", "@junit//jar", + "@mockito-core//jar", + "@objenesis//jar" ], ) diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target index 28ebdbf70..dba5c72c8 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target @@ -1,7 +1,7 @@ - - + + @@ -31,10 +31,10 @@ - - - - + + + + @@ -46,14 +46,18 @@ - - - - + + + + + + + + - - + + @@ -66,7 +70,7 @@ - + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.tpd index 8e65c9820..7a5feb073 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.tpd @@ -1,7 +1,7 @@ target "jgit-4.5" with source configurePhase include "projects/jetty-9.4.11.tpd" -include "orbit/R20180905201904-2018-09.tpd" +include "orbit/R20190602212107-2019-06.tpd" location "http://download.eclipse.org/releases/mars/" { org.eclipse.osgi lazy diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.target index 8490e55b2..6c7de213b 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.target @@ -1,7 +1,7 @@ - - + + @@ -31,10 +31,10 @@ - - - - + + + + @@ -46,14 +46,18 @@ - - - - + + + + + + + + - - + + @@ -66,7 +70,7 @@ - + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.tpd index 44c846e0b..8b3dd0b47 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.tpd @@ -1,7 +1,7 @@ target "jgit-4.6" with source configurePhase include "projects/jetty-9.4.11.tpd" -include "orbit/R20180905201904-2018-09.tpd" +include "orbit/R20190602212107-2019-06.tpd" location "http://download.eclipse.org/releases/neon/" { org.eclipse.osgi lazy diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.target index a6651b837..0fb8d4ced 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.target @@ -1,7 +1,7 @@ - - + + @@ -31,10 +31,10 @@ - - - - + + + + @@ -46,14 +46,18 @@ - - - - + + + + + + + + - - + + @@ -66,7 +70,7 @@ - + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.tpd index b63955f2a..e93fc46b5 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.tpd @@ -1,7 +1,7 @@ target "jgit-4.7" with source configurePhase include "projects/jetty-9.4.11.tpd" -include "orbit/R20180905201904-2018-09.tpd" +include "orbit/R20190602212107-2019-06.tpd" location "http://download.eclipse.org/releases/oxygen/" { org.eclipse.osgi lazy diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.8.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.8.target index 7adb7dafd..34f3863b9 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.8.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.8.target @@ -1,7 +1,7 @@ - - + + @@ -31,10 +31,10 @@ - - - - + + + + @@ -46,14 +46,18 @@ - - - - + + + + + + + + - - + + @@ -66,7 +70,7 @@ - + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.8.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.8.tpd index 4e41ad9a0..ba6337dbf 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.8.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.8.tpd @@ -1,7 +1,7 @@ target "jgit-4.8" with source configurePhase include "projects/jetty-9.4.11.tpd" -include "orbit/R20180905201904-2018-09.tpd" +include "orbit/R20190602212107-2019-06.tpd" location "http://download.eclipse.org/releases/photon/" { org.eclipse.osgi lazy diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.9-staging.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.9-staging.target index bdabdffa2..66b8b3ac8 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.9-staging.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.9-staging.target @@ -1,7 +1,7 @@ - - + + @@ -31,10 +31,10 @@ - - - - + + + + @@ -46,14 +46,18 @@ - - - - + + + + + + + + - - + + @@ -66,7 +70,7 @@ - + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.9-staging.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.9-staging.tpd index 6f963c7ce..fc0179937 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.9-staging.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.9-staging.tpd @@ -1,7 +1,7 @@ target "jgit-4.9-staging" with source configurePhase include "projects/jetty-9.4.11.tpd" -include "orbit/R20180905201904-2018-09.tpd" +include "orbit/R20190602212107-2019-06.tpd" location "http://download.eclipse.org/staging/2018-09/" { org.eclipse.osgi lazy diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20180905201904-2018-09.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20190602212107-2019-06.tpd similarity index 67% rename from org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20180905201904-2018-09.tpd rename to org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20190602212107-2019-06.tpd index eaae99cbe..f49571ec7 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20180905201904-2018-09.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20190602212107-2019-06.tpd @@ -1,7 +1,7 @@ -target "R20180905201904-2018-09" with source configurePhase +target "R20190602212107-2019-06" with source configurePhase // see http://download.eclipse.org/tools/orbit/downloads/ -location "http://download.eclipse.org/tools/orbit/downloads/drops/R20180905201904/repository" { +location "http://download.eclipse.org/tools/orbit/downloads/drops/R20190602212107/repository" { org.apache.ant [1.9.6.v201510161327,1.9.6.v201510161327] org.apache.ant.source [1.9.6.v201510161327,1.9.6.v201510161327] org.apache.commons.codec [1.10.0.v20180409-1845,1.10.0.v20180409-1845] @@ -10,10 +10,10 @@ location "http://download.eclipse.org/tools/orbit/downloads/drops/R2018090520190 org.apache.commons.compress.source [1.15.0.v20180119-1613,1.15.0.v20180119-1613s] org.apache.commons.logging [1.2.0.v20180409-1502,1.2.0.v20180409-1502] org.apache.commons.logging.source [1.2.0.v20180409-1502,1.2.0.v20180409-1502] - org.apache.httpcomponents.httpclient [4.5.5.v20180409-1525,4.5.5.v20180409-1525] - org.apache.httpcomponents.httpclient.source [4.5.5.v20180409-1525,4.5.5.v20180409-1525] - org.apache.httpcomponents.httpcore [4.4.9.v20180409-1525,4.4.9.v20180409-1525] - org.apache.httpcomponents.httpcore.source [4.4.9.v20180409-1525,4.4.9.v20180409-1525] + org.apache.httpcomponents.httpclient [4.5.6.v20190503-0009,4.5.6.v20190503-0009] + org.apache.httpcomponents.httpclient.source [4.5.6.v20190503-0009,4.5.6.v20190503-0009] + org.apache.httpcomponents.httpcore [4.4.10.v20190123-2214,4.4.10.v20190123-2214] + org.apache.httpcomponents.httpcore.source [4.4.10.v20190123-2214,4.4.10.v20190123-2214] org.apache.log4j [1.2.15.v201012070815,1.2.15.v201012070815] org.apache.log4j.source [1.2.15.v201012070815,1.2.15.v201012070815] org.kohsuke.args4j [2.33.0.v20160323-2218,2.33.0.v20160323-2218] @@ -25,14 +25,18 @@ location "http://download.eclipse.org/tools/orbit/downloads/drops/R2018090520190 org.hamcrest.library.source [1.3.0.v20180524-2246,1.3.0.v20180524-2246] javaewah [1.1.6.v20160919-1400,1.1.6.v20160919-1400] javaewah.source [1.1.6.v20160919-1400,1.1.6.v20160919-1400] - org.objenesis [1.0.0.v201505121915,1.0.0.v201505121915] - org.objenesis.source [1.0.0.v201505121915,1.0.0.v201505121915] - org.mockito [1.8.4.v201303031500,1.8.4.v201303031500] - org.mockito.source [1.8.4.v201303031500,1.8.4.v201303031500] + org.objenesis [2.6.0.v20180420-1519,2.6.0.v20180420-1519] + org.objenesis.source [2.6.0.v20180420-1519,2.6.0.v20180420-1519] + org.mockito [2.23.0.v20190527-1420,2.23.0.v20190527-1420] + org.mockito.source [2.23.0.v20190527-1420,2.23.0.v20190527-1420] + net.bytebuddy.byte-buddy [1.9.0.v20181107-1410,1.9.0.v20181107-1410] + net.bytebuddy.byte-buddy.source [1.9.0.v20181107-1410,1.9.0.v20181107-1410] + net.bytebuddy.byte-buddy-agent [1.9.0.v20181106-1534,1.9.0.v20181106-1534] + net.bytebuddy.byte-buddy-agent.source [1.9.0.v20181106-1534,1.9.0.v20181106-1534] com.google.gson [2.8.2.v20180104-1110,2.8.2.v20180104-1110] com.google.gson.source [2.8.2.v20180104-1110,2.8.2.v20180104-1110] - com.jcraft.jsch [0.1.54.v20170116-1932,0.1.54.v20170116-1932] - com.jcraft.jsch.source [0.1.54.v20170116-1932,0.1.54.v20170116-1932] + com.jcraft.jsch [0.1.55.v20190404-1902,0.1.55.v20190404-1902] + com.jcraft.jsch.source [0.1.55.v20190404-1902,0.1.55.v20190404-1902] org.junit [4.12.0.v201504281640,4.12.0.v201504281640] org.junit.source [4.12.0.v201504281640,4.12.0.v201504281640] javax.servlet [3.1.0.v201410161800,3.1.0.v201410161800] diff --git a/org.eclipse.jgit.test/pom.xml b/org.eclipse.jgit.test/pom.xml index 3a887efb8..00b74fc27 100644 --- a/org.eclipse.jgit.test/pom.xml +++ b/org.eclipse.jgit.test/pom.xml @@ -89,6 +89,12 @@ [1.1.0,2.0.0) + + org.mockito + mockito-core + 2.23.0 + + org.eclipse.jgit org.eclipse.jgit diff --git a/pom.xml b/pom.xml index b7e33305c..77cb12891 100644 --- a/pom.xml +++ b/pom.xml @@ -198,7 +198,7 @@ ${project.build.directory}/META-INF/MANIFEST.MF 4.11.0.201803080745-r - 0.1.54 + 0.1.55 1.1.1 1.1.6 4.12 @@ -209,8 +209,8 @@ 3.1.0 9.4.11.v20180605 0.14.1 - 4.5.5 - 4.4.9 + 4.5.6 + 4.4.10 1.7.2 1.2.15 3.1.0 From 9b4a55c0b65074e6abec4b9aff1b602ee6a976b9 Mon Sep 17 00:00:00 2001 From: Vishal Devgire Date: Tue, 9 Apr 2019 13:01:36 +0530 Subject: [PATCH 14/24] Make supportsAtomicCreateNewFile return true as default The method org.eclipse.jgit.util.FS.supportsAtomicCreateNewFile() should default to true as mentioned in docs [1] org.eclipse.jgit.util.FS_POSIX.supportsAtomicCreateNewFile() method will set the value to false if the git config core.supportsatomiccreatenewfile is not set. It should default to true if the configuration is undefined. [1] https://github.com/eclipse/jgit/blob/4169a95a65683e39e7a6a8f2b11b543e2bc754db/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java#L372 Bug: 544164 Change-Id: I16ccf989a89da2cf4975c200b3228b25ba4c0d55 Signed-off-by: Vishal Devgire Signed-off-by: Matthias Sohn --- org.eclipse.jgit.test/META-INF/MANIFEST.MF | 2 + .../org/eclipse/jgit/util/FS_POSIXTest.java | 125 ++++++++++++++++++ .../src/org/eclipse/jgit/util/FS_POSIX.java | 7 +- 3 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FS_POSIXTest.java diff --git a/org.eclipse.jgit.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.test/META-INF/MANIFEST.MF index 89ce34dbf..48ae7cf5e 100644 --- a/org.eclipse.jgit.test/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.test/META-INF/MANIFEST.MF @@ -68,6 +68,8 @@ Import-Package: com.googlecode.javaewah;version="[1.1.6,2.0.0)", org.junit.rules;version="[4.12,5.0.0)", org.junit.runner;version="[4.12,5.0.0)", org.junit.runners;version="[4.12,5.0.0)", + org.mockito;version="[2.23.0,3.0.0)", + org.mockito.stubbing;version="2.23.0", org.slf4j;version="[1.7.0,2.0.0)", org.tukaani.xz;version="[1.6.0,2.0)" Require-Bundle: org.hamcrest.core;bundle-version="[1.1.0,2.0.0)", diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FS_POSIXTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FS_POSIXTest.java new file mode 100644 index 000000000..9683f93e6 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FS_POSIXTest.java @@ -0,0 +1,125 @@ +/* + * Copyright (C) 2019, Vishal Devgire + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.util; + +import org.eclipse.jgit.lib.ConfigConstants; +import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class FS_POSIXTest { + private SystemReader originalSystemReaderInstance; + + private FileBasedConfig mockSystemConfig; + + private FileBasedConfig mockUserConfig; + + @Before + public void setUp() throws Exception { + SystemReader systemReader = Mockito.mock(SystemReader.class); + + originalSystemReaderInstance = SystemReader.getInstance(); + SystemReader.setInstance(systemReader); + + mockSystemConfig = mock(FileBasedConfig.class); + mockUserConfig = mock(FileBasedConfig.class); + when(systemReader.openSystemConfig(any(), any())) + .thenReturn(mockSystemConfig); + when(systemReader.openUserConfig(any(), any())) + .thenReturn(mockUserConfig); + + when(mockSystemConfig.getString(ConfigConstants.CONFIG_CORE_SECTION, + null, ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION)) + .thenReturn(null); + } + + @After + public void tearDown() { + SystemReader.setInstance(originalSystemReaderInstance); + } + + @Test + public void supportsAtomicCreateNewFile_shouldReturnSupportedAsDefault() { + assertTrue(new FS_POSIX().supportsAtomicCreateNewFile()); + } + + @Test + public void supportsAtomicCreateNewFile_shouldReturnTrueIfFlagIsSetInUserConfig() { + setAtomicCreateCreationFlag(mockUserConfig, "true"); + assertTrue(new FS_POSIX().supportsAtomicCreateNewFile()); + } + + @Test + public void supportsAtomicCreateNewFile_shouldReturnTrueIfFlagIsSetInSystemConfig() { + setAtomicCreateCreationFlag(mockSystemConfig, "true"); + assertTrue(new FS_POSIX().supportsAtomicCreateNewFile()); + } + + @Test + public void supportsAtomicCreateNewFile_shouldReturnFalseIfFlagUnsetInUserConfig() { + setAtomicCreateCreationFlag(mockUserConfig, "false"); + assertFalse(new FS_POSIX().supportsAtomicCreateNewFile()); + } + + @Test + public void supportsAtomicCreateNewFile_shouldReturnFalseIfFlagUnsetInSystemConfig() { + setAtomicCreateCreationFlag(mockSystemConfig, "false"); + assertFalse(new FS_POSIX().supportsAtomicCreateNewFile()); + } + + private void setAtomicCreateCreationFlag(FileBasedConfig config, + String value) { + when(config.getString(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION)) + .thenReturn(value); + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java index 8561f6f3a..1c42f7857 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java @@ -122,6 +122,10 @@ public class FS_POSIX extends FS { ret = getAtomicFileCreationSupportOption( SystemReader.getInstance().openSystemConfig(null, this)); } + + if (ret == AtomicFileCreation.UNDEFINED) { + ret = AtomicFileCreation.SUPPORTED; + } supportsAtomicCreateNewFile = ret; } @@ -139,7 +143,8 @@ public class FS_POSIX extends FS { ? AtomicFileCreation.SUPPORTED : AtomicFileCreation.NOT_SUPPORTED; } catch (IOException | ConfigInvalidException e) { - return AtomicFileCreation.SUPPORTED; + LOG.error(e.getMessage(), e); + return AtomicFileCreation.UNDEFINED; } } From 69ce7fc7fb18ef82200e111344eabd205d45b5cf Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Tue, 13 Aug 2019 00:26:25 +0200 Subject: [PATCH 15/24] Ensure FSTest uses MockSystemReader Tests shall not modify ~/.gitconfig. When running tests with bazel this test failed since bazel isolates tests in a sandbox. Change-Id: I7dd092afd14972da58a95eb7c200d353f0959fa1 Signed-off-by: Matthias Sohn --- org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSTest.java index 2054e1efa..3e1202cde 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSTest.java @@ -65,6 +65,7 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import org.eclipse.jgit.errors.CommandFailedException; +import org.eclipse.jgit.junit.MockSystemReader; import org.eclipse.jgit.junit.RepositoryTestCase; import org.eclipse.jgit.lib.RepositoryCache; import org.junit.After; @@ -77,6 +78,7 @@ public class FSTest { @Before public void setUp() throws Exception { + SystemReader.setInstance(new MockSystemReader()); trash = File.createTempFile("tmp_", ""); trash.delete(); assertTrue("mkdir " + trash, trash.mkdir()); From 2d84bb43410469b44bca145c3d06a0386c0e2175 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 12 Aug 2019 17:40:39 +0200 Subject: [PATCH 16/24] Improve retry handling when saving FileStoreAttributes fails - fix handling of interrupts in FileStoreAttributes#saveToConfig - increase retry wait time to 100ms - don't wait after last retry - dont retry if failure is caused by another exception than LockFailedException Change-Id: I108c012717d2bcce71f2c6cb9cf0879de704ebc2 Signed-off-by: Matthias Sohn --- .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../src/org/eclipse/jgit/util/FS.java | 18 ++++++++++++++---- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index cba892ef6..1579dc7a7 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -430,6 +430,7 @@ localRefIsMissingObjects=Local ref {0} is missing object(s). localRepository=local repository lockCountMustBeGreaterOrEqual1=lockCount must be >= 1 lockError=lock error: {0} +lockFailedRetry=locking {0} failed after {1} retries lockOnNotClosed=Lock on {0} not closed. lockOnNotHeld=Lock on {0} not held. malformedpersonIdentString=Malformed PersonIdent string (no < was found): {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 23950100d..a6110e57d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -491,6 +491,7 @@ public class JGitText extends TranslationBundle { /***/ public String localRepository; /***/ public String lockCountMustBeGreaterOrEqual1; /***/ public String lockError; + /***/ public String lockFailedRetry; /***/ public String lockOnNotClosed; /***/ public String lockOnNotHeld; /***/ public String malformedpersonIdentString; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java index 509e6d228..6efd02f47 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java @@ -580,20 +580,30 @@ public abstract class FS { } catch (LockFailedException e) { // race with another thread, wait a bit and try again try { - LOG.warn(MessageFormat.format(JGitText.get().cannotLock, - userConfig)); retries++; - Thread.sleep(20); + if (retries < max_retries) { + Thread.sleep(100); + LOG.debug("locking {} failed, retries {}/{}", //$NON-NLS-1$ + userConfig, Integer.valueOf(retries), + Integer.valueOf(max_retries)); + } else { + LOG.warn(MessageFormat.format( + JGitText.get().lockFailedRetry, userConfig, + Integer.valueOf(retries))); + } } catch (InterruptedException e1) { - Thread.interrupted(); + Thread.currentThread().interrupt(); + break; } } catch (IOException e) { LOG.error(MessageFormat.format( JGitText.get().cannotSaveConfig, userConfig), e); + break; } catch (ConfigInvalidException e) { LOG.error(MessageFormat.format( JGitText.get().repositoryConfigFileInvalid, userConfig, e.getMessage())); + break; } } } From b51bb4f7bd305287fed885161fbef1cf3bd16613 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 15 Aug 2019 01:36:41 +0200 Subject: [PATCH 17/24] Fix javadoc for SystemReader#getInstance The existing javadoc was copied from another method and not adapted. Change-Id: I39a7e5d719b2c379de9bd1a4710a55a73700c6f0 Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/util/SystemReader.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java index 953c9768f..b61a6f191 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java @@ -152,16 +152,16 @@ public abstract class SystemReader { private static SystemReader INSTANCE = DEFAULT; /** - * Get time since epoch, with up to millisecond resolution. + * Get the current SystemReader instance * - * @return time since epoch, with up to millisecond resolution. + * @return the current SystemReader instance. */ public static SystemReader getInstance() { return INSTANCE; } /** - * Set the new instance to use when accessing properties. + * Set a new SystemReader instance to use when accessing properties. * * @param newReader * the new instance to use when accessing properties, or null for From fed2a8a4f9b905b6d11f90bf25f023d4ca736365 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 14 Aug 2019 13:25:16 +0200 Subject: [PATCH 18/24] Add missing dependencies for running FS_POSIXTest in Eclipse Change-Id: I35fcdfab55ff02943f6185b232e01ce9cfe151a9 Signed-off-by: Matthias Sohn --- org.eclipse.jgit.test/META-INF/MANIFEST.MF | 2 ++ 1 file changed, 2 insertions(+) diff --git a/org.eclipse.jgit.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.test/META-INF/MANIFEST.MF index 48ae7cf5e..cd3c9689c 100644 --- a/org.eclipse.jgit.test/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.test/META-INF/MANIFEST.MF @@ -10,6 +10,7 @@ Bundle-ActivationPolicy: lazy Bundle-RequiredExecutionEnvironment: JavaSE-1.8 Import-Package: com.googlecode.javaewah;version="[1.1.6,2.0.0)", com.jcraft.jsch;version="[0.1.54,0.2.0)", + net.bytebuddy.dynamic.loading;version="[1.9.0,2.0.0)", org.apache.commons.compress.archivers;version="[1.15.0,2.0)", org.apache.commons.compress.archivers.tar;version="[1.15.0,2.0)", org.apache.commons.compress.archivers.zip;version="[1.15.0,2.0)", @@ -70,6 +71,7 @@ Import-Package: com.googlecode.javaewah;version="[1.1.6,2.0.0)", org.junit.runners;version="[4.12,5.0.0)", org.mockito;version="[2.23.0,3.0.0)", org.mockito.stubbing;version="2.23.0", + org.objenesis;version="2.6.0", org.slf4j;version="[1.7.0,2.0.0)", org.tukaani.xz;version="[1.6.0,2.0)" Require-Bundle: org.hamcrest.core;bundle-version="[1.1.0,2.0.0)", From 7f92a70fb3a5b7e91e5d370098fa65f6c8f35efa Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sun, 11 Aug 2019 02:43:02 +0200 Subject: [PATCH 19/24] Avoid setup and saving FileStoreAttributes compete for ~/.gitconfig lock FS determines FileStore attributes in a background thread and tries to save the results to the global git configuration. This competed with LocalDiskRepositoryTestCase#setup trying to save changes to the same file requiring the same lock. This frequently led to one of the threads failing to acquire the lock. Fix this by first initiating determination of FileStore attributes which then uses a MockSystemReader not using a userConfig stored to disk which avoids this race for the lock. Change-Id: I30fcd96bc15100f8ef9b2a9eb3320bb5ace97c67 Signed-off-by: Matthias Sohn --- .../jgit/junit/LocalDiskRepositoryTestCase.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java index cce83c00b..f8f0c18cb 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java @@ -130,6 +130,13 @@ public abstract class LocalDiskRepositoryTestCase { mockSystemReader = new MockSystemReader(); SystemReader.setInstance(mockSystemReader); + + // Measure timer resolution before the test to avoid time critical tests + // are affected by time needed for measurement. + // The MockSystemReader must be configured first since we need to use + // the same one here + FS.getFileStoreAttributes(tmp.toPath().getParent()); + mockSystemReader.userGitConfig = new FileBasedConfig(new File(tmp, "usergitconfig"), FS.DETECTED); // We have to set autoDetach to false for tests, because tests expect to be able @@ -140,12 +147,6 @@ public abstract class LocalDiskRepositoryTestCase { mockSystemReader.userGitConfig.save(); ceilTestDirectories(getCeilings()); - // Measure timer resolution before the test to avoid time critical tests - // are affected by time needed for measurement. - // The MockSystemReader must be configured first since we need to use - // the same one here - FS.getFileStoreAttributes(tmp.toPath().getParent()); - author = new PersonIdent("J. Author", "jauthor@example.com"); committer = new PersonIdent("J. Committer", "jcommitter@example.com"); From f383206ace187ec92672b806e18a54e8d398a27d Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 15 Aug 2019 01:25:28 +0200 Subject: [PATCH 20/24] Cache user global and system-wide git configurations So far the git configuration and the system wide git configuration were always reloaded when jgit accessed these global configuration files to access global configuration options which are not in the context of a single git repository. Cache these configurations in SystemReader and only reload them if their file metadata observed using FileSnapshot indicates a modification. Change-Id: I092fe11a5d95f1c5799273cacfc7a415d0b7786c Signed-off-by: Matthias Sohn Signed-off-by: Thomas Wolf --- .../http/test/SmartClientSmartServerTest.java | 18 ++- .../junit/LocalDiskRepositoryTestCase.java | 9 +- .../eclipse/jgit/junit/MockSystemReader.java | 41 +++++- org.eclipse.jgit.lfs/.settings/.api_filters | 11 ++ .../jgit/lfs/InstallBuiltinLfsCommand.java | 41 +++--- .../eclipse/jgit/api/CloneCommandTest.java | 6 +- .../org/eclipse/jgit/util/FS_POSIXTest.java | 74 ++++++---- org.eclipse.jgit/.settings/.api_filters | 26 ++++ .../eclipse/jgit/internal/JGitText.properties | 5 +- .../org/eclipse/jgit/internal/JGitText.java | 5 +- .../internal/storage/file/FileRepository.java | 85 +++-------- .../eclipse/jgit/transport/HttpConfig.java | 11 +- .../eclipse/jgit/transport/TransportHttp.java | 10 +- .../src/org/eclipse/jgit/util/FS.java | 25 ++-- .../src/org/eclipse/jgit/util/FS_POSIX.java | 64 +++------ .../org/eclipse/jgit/util/SystemReader.java | 133 +++++++++++++++--- 16 files changed, 337 insertions(+), 227 deletions(-) create mode 100644 org.eclipse.jgit.lfs/.settings/.api_filters diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java index b26324d4f..78bf778cc 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java @@ -107,7 +107,6 @@ import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; -import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.transport.AbstractAdvertiseRefsHook; import org.eclipse.jgit.transport.AdvertiseRefsHook; import org.eclipse.jgit.transport.CredentialItem; @@ -127,7 +126,6 @@ import org.eclipse.jgit.transport.http.apache.HttpClientConnectionFactory; import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; import org.eclipse.jgit.transport.resolver.UploadPackFactory; -import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.HttpSupport; import org.eclipse.jgit.util.SystemReader; import org.hamcrest.Matchers; @@ -652,8 +650,8 @@ public class SmartClientSmartServerTest extends HttpTestCase { @Test public void testInitialClone_RedirectMax() throws Exception { - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig = SystemReader.getInstance() + .getUserConfig(); userConfig.setInt("http", null, "maxRedirects", 4); userConfig.save(); initialClone_Redirect(4, 302); @@ -661,8 +659,8 @@ public class SmartClientSmartServerTest extends HttpTestCase { @Test public void testInitialClone_RedirectTooOften() throws Exception { - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig = SystemReader.getInstance() + .getUserConfig(); userConfig.setInt("http", null, "maxRedirects", 3); userConfig.save(); Repository dst = createBareRepository(); @@ -701,8 +699,8 @@ public class SmartClientSmartServerTest extends HttpTestCase { @Test public void testInitialClone_RedirectOnPostAllowed() throws Exception { - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig = SystemReader.getInstance() + .getUserConfig(); userConfig.setString("http", null, "followRedirects", "true"); userConfig.save(); Repository dst = createBareRepository(); @@ -764,8 +762,8 @@ public class SmartClientSmartServerTest extends HttpTestCase { @Test public void testInitialClone_RedirectForbidden() throws Exception { - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig = SystemReader.getInstance() + .getUserConfig(); userConfig.setString("http", null, "followRedirects", "false"); userConfig.save(); diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java index f8f0c18cb..29579d007 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java @@ -137,14 +137,15 @@ public abstract class LocalDiskRepositoryTestCase { // the same one here FS.getFileStoreAttributes(tmp.toPath().getParent()); - mockSystemReader.userGitConfig = new FileBasedConfig(new File(tmp, - "usergitconfig"), FS.DETECTED); + FileBasedConfig userConfig = new FileBasedConfig( + new File(tmp, "usergitconfig"), FS.DETECTED); // We have to set autoDetach to false for tests, because tests expect to be able // to clean up by recursively removing the repository, and background GC might be // in the middle of writing or deleting files, which would disrupt this. - mockSystemReader.userGitConfig.setBoolean(ConfigConstants.CONFIG_GC_SECTION, + userConfig.setBoolean(ConfigConstants.CONFIG_GC_SECTION, null, ConfigConstants.CONFIG_KEY_AUTODETACH, false); - mockSystemReader.userGitConfig.save(); + userConfig.save(); + mockSystemReader.setUserGitConfig(userConfig); ceilTestDirectories(getCeilings()); author = new PersonIdent("J. Author", "jauthor@example.com"); diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java index 92b531d19..123fdb305 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java @@ -60,6 +60,7 @@ import java.util.concurrent.TimeUnit; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.SystemReader; @@ -100,10 +101,36 @@ public class MockSystemReader extends SystemReader { final Map values = new HashMap<>(); - FileBasedConfig userGitConfig; + private FileBasedConfig userGitConfig; FileBasedConfig systemGitConfig; + /** + * Set the user-level git config + * + * @param userGitConfig + * set another user-level git config + * @return the old user-level git config + */ + public FileBasedConfig setUserGitConfig(FileBasedConfig userGitConfig) { + FileBasedConfig old = this.userGitConfig; + this.userGitConfig = userGitConfig; + return old; + } + + /** + * Set the system-level git config + * + * @param systemGitConfig + * the new system-level git config + * @return the old system-level config + */ + public FileBasedConfig setSystemGitConfig(FileBasedConfig systemGitConfig) { + FileBasedConfig old = this.systemGitConfig; + this.systemGitConfig = systemGitConfig; + return old; + } + /** * Constructor for MockSystemReader */ @@ -166,6 +193,18 @@ public class MockSystemReader extends SystemReader { return systemGitConfig; } + @Override + public StoredConfig getUserConfig() + throws IOException, ConfigInvalidException { + return userGitConfig; + } + + @Override + public StoredConfig getSystemConfig() + throws IOException, ConfigInvalidException { + return systemGitConfig; + } + /** {@inheritDoc} */ @Override public String getHostname() { diff --git a/org.eclipse.jgit.lfs/.settings/.api_filters b/org.eclipse.jgit.lfs/.settings/.api_filters new file mode 100644 index 000000000..db45ade67 --- /dev/null +++ b/org.eclipse.jgit.lfs/.settings/.api_filters @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/InstallBuiltinLfsCommand.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/InstallBuiltinLfsCommand.java index 028b19b2a..b7b0535ea 100644 --- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/InstallBuiltinLfsCommand.java +++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/InstallBuiltinLfsCommand.java @@ -43,14 +43,12 @@ package org.eclipse.jgit.lfs; import java.io.IOException; -import java.text.MessageFormat; +import org.eclipse.jgit.api.errors.InvalidConfigurationException; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.lfs.internal.LfsText; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.StoredConfig; -import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.LfsFactory.LfsInstallCommand; import org.eclipse.jgit.util.SystemReader; @@ -70,12 +68,28 @@ public class InstallBuiltinLfsCommand implements LfsInstallCommand { private Repository repository; - /** {@inheritDoc} */ + /** + * {@inheritDoc} + * + * @throws IOException + * if an I/O error occurs while accessing a git config or + * executing {@code git lfs install} in an external process + * @throws InvalidConfigurationException + * if a git configuration is invalid + * @throws InterruptedException + * if the current thread is interrupted while waiting for the + * {@code git lfs install} executed in an external process + */ @Override - public Void call() throws Exception { + public Void call() throws IOException, InvalidConfigurationException, + InterruptedException { StoredConfig cfg = null; if (repository == null) { - cfg = loadUserConfig(); + try { + cfg = SystemReader.getInstance().getUserConfig(); + } catch (ConfigInvalidException e) { + throw new InvalidConfigurationException(e.getMessage(), e); + } } else { cfg = repository.getConfig(); } @@ -116,19 +130,4 @@ public class InstallBuiltinLfsCommand implements LfsInstallCommand { return this; } - private StoredConfig loadUserConfig() throws IOException { - FileBasedConfig c = SystemReader.getInstance().openUserConfig(null, - FS.DETECTED); - try { - c.load(); - } catch (ConfigInvalidException e1) { - throw new IOException(MessageFormat - .format(LfsText.get().userConfigInvalid, c.getFile() - .getAbsolutePath(), e1), - e1); - } - - return c; - } - } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java index 0d7009dca..613ca5ce9 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java @@ -68,9 +68,9 @@ import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.submodule.SubmoduleStatus; import org.eclipse.jgit.submodule.SubmoduleStatusType; import org.eclipse.jgit.submodule.SubmoduleWalk; @@ -633,8 +633,8 @@ public class CloneCommandTest extends RepositoryTestCase { ConfigConstants.CONFIG_BRANCH_SECTION, "test", ConfigConstants.CONFIG_KEY_REBASE, null)); - FileBasedConfig userConfig = SystemReader.getInstance().openUserConfig( - null, git.getRepository().getFS()); + StoredConfig userConfig = SystemReader.getInstance() + .getUserConfig(); userConfig.setString(ConfigConstants.CONFIG_BRANCH_SECTION, null, ConfigConstants.CONFIG_KEY_AUTOSETUPREBASE, ConfigConstants.CONFIG_KEY_ALWAYS); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FS_POSIXTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FS_POSIXTest.java index 9683f93e6..87349a25a 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FS_POSIXTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FS_POSIXTest.java @@ -43,48 +43,63 @@ package org.eclipse.jgit.util; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.eclipse.jgit.junit.MockSystemReader; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.mockito.Mockito; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class FS_POSIXTest { private SystemReader originalSystemReaderInstance; - private FileBasedConfig mockSystemConfig; + private FileBasedConfig systemConfig; + + private FileBasedConfig userConfig; - private FileBasedConfig mockUserConfig; + private Path tmp; @Before public void setUp() throws Exception { - SystemReader systemReader = Mockito.mock(SystemReader.class); + tmp = Files.createTempDirectory("jgit_test_"); + MockSystemReader mockSystemReader = new MockSystemReader(); + SystemReader.setInstance(mockSystemReader); + + // Measure timer resolution before the test to avoid time critical tests + // are affected by time needed for measurement. + // The MockSystemReader must be configured first since we need to use + // the same one here + FS.getFileStoreAttributes(tmp.getParent()); + systemConfig = new FileBasedConfig( + new File(tmp.toFile(), "systemgitconfig"), FS.DETECTED); + userConfig = new FileBasedConfig(systemConfig, + new File(tmp.toFile(), "usergitconfig"), FS.DETECTED); + // We have to set autoDetach to false for tests, because tests expect to + // be able to clean up by recursively removing the repository, and + // background GC might be in the middle of writing or deleting files, + // which would disrupt this. + userConfig.setBoolean(ConfigConstants.CONFIG_GC_SECTION, null, + ConfigConstants.CONFIG_KEY_AUTODETACH, false); + userConfig.save(); + mockSystemReader.setSystemGitConfig(systemConfig); + mockSystemReader.setUserGitConfig(userConfig); originalSystemReaderInstance = SystemReader.getInstance(); - SystemReader.setInstance(systemReader); - - mockSystemConfig = mock(FileBasedConfig.class); - mockUserConfig = mock(FileBasedConfig.class); - when(systemReader.openSystemConfig(any(), any())) - .thenReturn(mockSystemConfig); - when(systemReader.openUserConfig(any(), any())) - .thenReturn(mockUserConfig); - - when(mockSystemConfig.getString(ConfigConstants.CONFIG_CORE_SECTION, - null, ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION)) - .thenReturn(null); + SystemReader.setInstance(mockSystemReader); } @After - public void tearDown() { + public void tearDown() throws IOException { SystemReader.setInstance(originalSystemReaderInstance); + FileUtils.delete(tmp.toFile(), FileUtils.RECURSIVE | FileUtils.RETRY); } @Test @@ -94,32 +109,31 @@ public class FS_POSIXTest { @Test public void supportsAtomicCreateNewFile_shouldReturnTrueIfFlagIsSetInUserConfig() { - setAtomicCreateCreationFlag(mockUserConfig, "true"); + setAtomicCreateCreationFlag(userConfig, "true"); assertTrue(new FS_POSIX().supportsAtomicCreateNewFile()); } @Test public void supportsAtomicCreateNewFile_shouldReturnTrueIfFlagIsSetInSystemConfig() { - setAtomicCreateCreationFlag(mockSystemConfig, "true"); + setAtomicCreateCreationFlag(systemConfig, "true"); assertTrue(new FS_POSIX().supportsAtomicCreateNewFile()); } @Test public void supportsAtomicCreateNewFile_shouldReturnFalseIfFlagUnsetInUserConfig() { - setAtomicCreateCreationFlag(mockUserConfig, "false"); + setAtomicCreateCreationFlag(userConfig, "false"); assertFalse(new FS_POSIX().supportsAtomicCreateNewFile()); } @Test public void supportsAtomicCreateNewFile_shouldReturnFalseIfFlagUnsetInSystemConfig() { - setAtomicCreateCreationFlag(mockSystemConfig, "false"); + setAtomicCreateCreationFlag(systemConfig, "false"); assertFalse(new FS_POSIX().supportsAtomicCreateNewFile()); } private void setAtomicCreateCreationFlag(FileBasedConfig config, String value) { - when(config.getString(ConfigConstants.CONFIG_CORE_SECTION, null, - ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION)) - .thenReturn(value); + config.setString(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION, value); } } diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 5874cd51f..f05afaf53 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -248,4 +248,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 1579dc7a7..dc1db8140 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -17,6 +17,7 @@ applyingCommit=Applying {0} archiveFormatAlreadyAbsent=Archive format already absent: {0} archiveFormatAlreadyRegistered=Archive format already registered with different implementation: {0} argumentIsNotAValidCommentString=Invalid comment: {0} +assumeAtomicCreateNewFile=Reading option "core.supportsAtomicFileCreation" failed, fallback to default assuming atomic file creation is supported atLeastOnePathIsRequired=At least one path is required. atLeastOnePatternIsRequired=At least one pattern is required. atLeastTwoFiltersNeeded=At least two filters needed. @@ -562,6 +563,7 @@ pushNotPermitted=push not permitted pushOptionsNotSupported=Push options not supported; received {0} rawLogMessageDoesNotParseAsLogEntry=Raw log message does not parse as log entry readConfigFailed=Reading config file ''{0}'' failed +readFileStoreAttributesFailed=Reading FileStore attributes from user config failed readerIsRequired=Reader is required readingObjectsFromLocalRepositoryFailed=reading objects from local repository failed: {0} readLastModifiedFailed=Reading lastModified of {0} failed @@ -618,6 +620,7 @@ rewinding=Rewinding to commit {0} s3ActionDeletion=Deletion s3ActionReading=Reading s3ActionWriting=Writing +saveFileStoreAttributesFailed=Saving measured FileStore attributes to user config failed searchForReuse=Finding sources searchForSizes=Getting sizes secondsAgo={0} seconds ago @@ -777,7 +780,7 @@ uriNotConfigured=Submodule URI not configured uriNotFound={0} not found uriNotFoundWithMessage={0} not found: {1} URINotSupported=URI not supported: {0} -userConfigFileInvalid=User config file {0} invalid {1} +userConfigInvalid=Git config in the user's home directory {0} is invalid {1} walkFailure=Walk failure. wantNotValid=want {0} not valid weeksAgo={0} weeks ago diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index a6110e57d..4c6026624 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -78,6 +78,7 @@ public class JGitText extends TranslationBundle { /***/ public String archiveFormatAlreadyAbsent; /***/ public String archiveFormatAlreadyRegistered; /***/ public String argumentIsNotAValidCommentString; + /***/ public String assumeAtomicCreateNewFile; /***/ public String atLeastOnePathIsRequired; /***/ public String atLeastOnePatternIsRequired; /***/ public String atLeastTwoFiltersNeeded; @@ -623,6 +624,7 @@ public class JGitText extends TranslationBundle { /***/ public String pushOptionsNotSupported; /***/ public String rawLogMessageDoesNotParseAsLogEntry; /***/ public String readConfigFailed; + /***/ public String readFileStoreAttributesFailed; /***/ public String readerIsRequired; /***/ public String readingObjectsFromLocalRepositoryFailed; /***/ public String readLastModifiedFailed; @@ -679,6 +681,7 @@ public class JGitText extends TranslationBundle { /***/ public String s3ActionDeletion; /***/ public String s3ActionReading; /***/ public String s3ActionWriting; + /***/ public String saveFileStoreAttributesFailed; /***/ public String searchForReuse; /***/ public String searchForSizes; /***/ public String secondsAgo; @@ -838,7 +841,7 @@ public class JGitText extends TranslationBundle { /***/ public String uriNotFound; /***/ public String uriNotFoundWithMessage; /***/ public String URINotSupported; - /***/ public String userConfigFileInvalid; + /***/ public String userConfigInvalid; /***/ public String walkFailure; /***/ public String wantNotValid; /***/ public String weeksAgo; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java index 5ced68646..356d64b56 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java @@ -81,15 +81,17 @@ import org.eclipse.jgit.lib.RefDatabase; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.ReflogReader; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.storage.file.FileRepositoryBuilder; import org.eclipse.jgit.storage.pack.PackConfig; -import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.FileUtils; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.RawParseUtils; import org.eclipse.jgit.util.StringUtils; import org.eclipse.jgit.util.SystemReader; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Represents a Git repository. A repository holds all objects and refs used for @@ -116,10 +118,10 @@ import org.eclipse.jgit.util.SystemReader; * This implementation only handles a subtly undocumented subset of git features. */ public class FileRepository extends Repository { + private static final Logger LOG = LoggerFactory + .getLogger(FileRepository.class); private static final String UNNAMED = "Unnamed repository; edit this file to name it for gitweb."; //$NON-NLS-1$ - private final FileBasedConfig systemConfig; - private final FileBasedConfig userConfig; private final FileBasedConfig repoConfig; private final RefDatabase refs; private final ObjectDirectory objectDatabase; @@ -178,32 +180,16 @@ public class FileRepository extends Repository { */ public FileRepository(BaseRepositoryBuilder options) throws IOException { super(options); - - if (StringUtils.isEmptyOrNull(SystemReader.getInstance().getenv( - Constants.GIT_CONFIG_NOSYSTEM_KEY))) - systemConfig = SystemReader.getInstance().openSystemConfig(null, - getFS()); - else - systemConfig = new FileBasedConfig(null, FS.DETECTED) { - @Override - public void load() { - // empty, do not load - } - - @Override - public boolean isOutdated() { - // regular class would bomb here - return false; - } - }; - userConfig = SystemReader.getInstance().openUserConfig(systemConfig, - getFS()); + StoredConfig userConfig = null; + try { + userConfig = SystemReader.getInstance().getUserConfig(); + } catch (ConfigInvalidException e) { + LOG.error(e.getMessage(), e); + throw new IOException(e.getMessage(), e); + } repoConfig = new FileBasedConfig(userConfig, getFS().resolve( getDirectory(), Constants.CONFIG), getFS()); - - loadSystemConfig(); - loadUserConfig(); loadRepoConfig(); repoConfig.addChangeListener(new ConfigChangedListener() { @@ -247,28 +233,6 @@ public class FileRepository extends Repository { } } - private void loadSystemConfig() throws IOException { - try { - systemConfig.load(); - } catch (ConfigInvalidException e) { - throw new IOException(MessageFormat.format(JGitText - .get().systemConfigFileInvalid, systemConfig.getFile() - .getAbsolutePath(), - e), e); - } - } - - private void loadUserConfig() throws IOException { - try { - userConfig.load(); - } catch (ConfigInvalidException e) { - throw new IOException(MessageFormat.format(JGitText - .get().userConfigFileInvalid, userConfig.getFile() - .getAbsolutePath(), - e), e); - } - } - private void loadRepoConfig() throws IOException { try { repoConfig.load(); @@ -398,26 +362,13 @@ public class FileRepository extends Repository { /** {@inheritDoc} */ @Override public FileBasedConfig getConfig() { - if (systemConfig.isOutdated()) { - try { - loadSystemConfig(); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - if (userConfig.isOutdated()) { - try { - loadUserConfig(); - } catch (IOException e) { - throw new RuntimeException(e); + try { + SystemReader.getInstance().getUserConfig(); + if (repoConfig.isOutdated()) { + loadRepoConfig(); } - } - if (repoConfig.isOutdated()) { - try { - loadRepoConfig(); - } catch (IOException e) { - throw new RuntimeException(e); - } + } catch (IOException | ConfigInvalidException e) { + throw new RuntimeException(e); } return repoConfig; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java index 101ce3568..ce9e1b3de 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java @@ -53,8 +53,7 @@ import java.util.function.Supplier; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.Config; -import org.eclipse.jgit.storage.file.FileBasedConfig; -import org.eclipse.jgit.util.FS; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.util.StringUtils; import org.eclipse.jgit.util.SystemReader; import org.slf4j.Logger; @@ -210,14 +209,12 @@ public class HttpConfig { * to get the configuration values for */ public HttpConfig(URIish uri) { - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig = null; try { - userConfig.load(); + userConfig = SystemReader.getInstance().getUserConfig(); } catch (IOException | ConfigInvalidException e) { // Log it and then work with default values. - LOG.error(MessageFormat.format(JGitText.get().userConfigFileInvalid, - userConfig.getFile().getAbsolutePath(), e)); + LOG.error(e.getMessage(), e); init(new Config(), uri); return; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java index 8b41ab046..0df1b70cb 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java @@ -108,11 +108,9 @@ import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.lib.SymbolicRef; -import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.transport.HttpAuthMethod.Type; import org.eclipse.jgit.transport.HttpConfig.HttpRedirectMode; import org.eclipse.jgit.transport.http.HttpConnection; -import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.HttpSupport; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.RawParseUtils; @@ -715,15 +713,13 @@ public class TransportHttp extends HttpTransport implements WalkTransport, } private void updateSslVerifyUser(boolean value) { - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig = null; try { - userConfig.load(); + userConfig = SystemReader.getInstance().getUserConfig(); updateSslVerify(userConfig, value); } catch (IOException | ConfigInvalidException e) { // Log it, but otherwise ignore here. - LOG.error(MessageFormat.format(JGitText.get().userConfigFileInvalid, - userConfig.getFile().getAbsolutePath(), e)); + LOG.error(e.getMessage(), e); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java index 6efd02f47..7d37cfa65 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java @@ -510,18 +510,12 @@ public abstract class FS { private static Optional readFromConfig( FileStore s) { - StoredConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig; try { - userConfig.load(); - } catch (IOException e) { - LOG.error(MessageFormat.format(JGitText.get().readConfigFailed, - userConfig), e); - } catch (ConfigInvalidException e) { - LOG.error(MessageFormat.format( - JGitText.get().repositoryConfigFileInvalid, - userConfig, - e.getMessage())); + userConfig = SystemReader.getInstance().getUserConfig(); + } catch (IOException | ConfigInvalidException e) { + LOG.error(JGitText.get().readFileStoreAttributesFailed, e); + return Optional.empty(); } String key = getConfigKey(s); Duration resolution = Duration.ofNanos(userConfig.getTimeUnit( @@ -544,8 +538,13 @@ public abstract class FS { private static void saveToConfig(FileStore s, FileStoreAttributes c) { - StoredConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig; + try { + userConfig = SystemReader.getInstance().getUserConfig(); + } catch (IOException | ConfigInvalidException e) { + LOG.error(JGitText.get().saveFileStoreAttributesFailed, e); + return; + } long resolution = c.getFsTimestampResolution().toNanos(); TimeUnit resolutionUnit = getUnit(resolution); long resolutionValue = resolutionUnit.convert(resolution, diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java index 1c42f7857..eda8afb10 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java @@ -42,6 +42,9 @@ */ package org.eclipse.jgit.util; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_CORE_SECTION; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION; + import java.io.BufferedReader; import java.io.File; import java.io.IOException; @@ -67,10 +70,9 @@ import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.errors.CommandFailedException; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.internal.JGitText; -import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.lib.StoredConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -87,7 +89,7 @@ public class FS_POSIX extends FS { private volatile boolean supportsUnixNLink = true; - private volatile AtomicFileCreation supportsAtomicCreateNewFile = AtomicFileCreation.UNDEFINED; + private volatile AtomicFileCreation supportsAtomicFileCreation = AtomicFileCreation.UNDEFINED; private enum AtomicFileCreation { SUPPORTED, NOT_SUPPORTED, UNDEFINED @@ -112,42 +114,6 @@ public class FS_POSIX extends FS { } } - private void determineAtomicFileCreationSupport() { - // @TODO: enhance SystemReader to support this without copying code - AtomicFileCreation ret = getAtomicFileCreationSupportOption( - SystemReader.getInstance().openUserConfig(null, this)); - if (ret == AtomicFileCreation.UNDEFINED - && StringUtils.isEmptyOrNull(SystemReader.getInstance() - .getenv(Constants.GIT_CONFIG_NOSYSTEM_KEY))) { - ret = getAtomicFileCreationSupportOption( - SystemReader.getInstance().openSystemConfig(null, this)); - } - - if (ret == AtomicFileCreation.UNDEFINED) { - ret = AtomicFileCreation.SUPPORTED; - } - supportsAtomicCreateNewFile = ret; - } - - private AtomicFileCreation getAtomicFileCreationSupportOption( - FileBasedConfig config) { - try { - config.load(); - String value = config.getString(ConfigConstants.CONFIG_CORE_SECTION, - null, - ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION); - if (value == null) { - return AtomicFileCreation.UNDEFINED; - } - return StringUtils.toBoolean(value) - ? AtomicFileCreation.SUPPORTED - : AtomicFileCreation.NOT_SUPPORTED; - } catch (IOException | ConfigInvalidException e) { - LOG.error(e.getMessage(), e); - return AtomicFileCreation.UNDEFINED; - } - } - /** {@inheritDoc} */ @Override public FS newInstance() { @@ -362,10 +328,24 @@ public class FS_POSIX extends FS { /** {@inheritDoc} */ @Override public boolean supportsAtomicCreateNewFile() { - if (supportsAtomicCreateNewFile == AtomicFileCreation.UNDEFINED) { - determineAtomicFileCreationSupport(); + if (supportsAtomicFileCreation == AtomicFileCreation.UNDEFINED) { + try { + StoredConfig config = SystemReader.getInstance().getUserConfig(); + String value = config.getString(CONFIG_CORE_SECTION, null, + CONFIG_KEY_SUPPORTSATOMICFILECREATION); + if (value != null) { + supportsAtomicFileCreation = StringUtils.toBoolean(value) + ? AtomicFileCreation.SUPPORTED + : AtomicFileCreation.NOT_SUPPORTED; + } else { + supportsAtomicFileCreation = AtomicFileCreation.SUPPORTED; + } + } catch (IOException | ConfigInvalidException e) { + LOG.warn(JGitText.get().assumeAtomicCreateNewFile, e); + supportsAtomicFileCreation = AtomicFileCreation.SUPPORTED; + } } - return supportsAtomicCreateNewFile == AtomicFileCreation.SUPPORTED; + return supportsAtomicFileCreation == AtomicFileCreation.SUPPORTED; } @Override diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java index b61a6f191..d554562a7 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java @@ -47,6 +47,7 @@ package org.eclipse.jgit.util; import java.io.File; +import java.io.IOException; import java.net.InetAddress; import java.net.UnknownHostException; import java.security.AccessController; @@ -56,12 +57,17 @@ import java.text.SimpleDateFormat; import java.util.Locale; import java.util.TimeZone; +import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectChecker; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.util.time.MonotonicClock; import org.eclipse.jgit.util.time.MonotonicSystemClock; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Interface to read values from the system. @@ -72,6 +78,10 @@ import org.eclipse.jgit.util.time.MonotonicSystemClock; *

*/ public abstract class SystemReader { + + private final static Logger LOG = LoggerFactory + .getLogger(SystemReader.class); + private static final SystemReader DEFAULT; private static Boolean isMacOS; @@ -87,6 +97,10 @@ public abstract class SystemReader { private static class Default extends SystemReader { private volatile String hostname; + private volatile FileBasedConfig systemConfig; + + private volatile FileBasedConfig userConfig; + @Override public String getenv(String variable) { return System.getenv(variable); @@ -99,28 +113,69 @@ public abstract class SystemReader { @Override public FileBasedConfig openSystemConfig(Config parent, FS fs) { - File configFile = fs.getGitSystemConfig(); - if (configFile == null) { - return new FileBasedConfig(null, fs) { - @Override - public void load() { - // empty, do not load - } - - @Override - public boolean isOutdated() { - // regular class would bomb here - return false; - } - }; + if (systemConfig == null) { + systemConfig = createSystemConfig(parent, fs); } - return new FileBasedConfig(parent, configFile, fs); + return systemConfig; + } + + protected FileBasedConfig createSystemConfig(Config parent, FS fs) { + if (StringUtils.isEmptyOrNull(getenv(Constants.GIT_CONFIG_NOSYSTEM_KEY))) { + File configFile = fs.getGitSystemConfig(); + if (configFile != null) { + return new FileBasedConfig(parent, configFile, fs); + } + } + return new FileBasedConfig(null, fs) { + @Override + public void load() { + // empty, do not load + } + + @Override + public boolean isOutdated() { + // regular class would bomb here + return false; + } + }; } @Override public FileBasedConfig openUserConfig(Config parent, FS fs) { - final File home = fs.userHome(); - return new FileBasedConfig(parent, new File(home, ".gitconfig"), fs); //$NON-NLS-1$ + if (userConfig == null) { + File home = fs.userHome(); + userConfig = new FileBasedConfig(parent, + new File(home, ".gitconfig"), fs); //$NON-NLS-1$ + } + return userConfig; + } + + @Override + public StoredConfig getSystemConfig() + throws IOException, ConfigInvalidException { + if (systemConfig == null) { + systemConfig = createSystemConfig(null, FS.DETECTED); + } + if (systemConfig.isOutdated()) { + LOG.debug("loading system config {}", systemConfig); //$NON-NLS-1$ + systemConfig.load(); + } + return systemConfig; + } + + @Override + public StoredConfig getUserConfig() + throws IOException, ConfigInvalidException { + if (userConfig == null) { + userConfig = openUserConfig(getSystemConfig(), FS.DETECTED); + } else { + getSystemConfig(); + } + if (userConfig.isOutdated()) { + LOG.debug("loading user config {}", userConfig); //$NON-NLS-1$ + userConfig.load(); + } + return userConfig; } @Override @@ -149,7 +204,7 @@ public abstract class SystemReader { } } - private static SystemReader INSTANCE = DEFAULT; + private static volatile SystemReader INSTANCE = DEFAULT; /** * Get the current SystemReader instance @@ -225,7 +280,10 @@ public abstract class SystemReader { public abstract String getProperty(String key); /** - * Open the git configuration found in the user home + * Open the git configuration found in the user home. Use + * {@link #getUserConfig()} to get the current git configuration in the user + * home since it manages automatic reloading when the gitconfig file was + * modified and avoids unnecessary reloads. * * @param parent * a config with values not found directly in the returned config @@ -237,7 +295,10 @@ public abstract class SystemReader { public abstract FileBasedConfig openUserConfig(Config parent, FS fs); /** - * Open the gitconfig configuration found in the system-wide "etc" directory + * Open the gitconfig configuration found in the system-wide "etc" + * directory. Use {@link #getSystemConfig()} to get the current system-wide + * git configuration since it manages automatic reloading when the gitconfig + * file was modified and avoids unnecessary reloads. * * @param parent * a config with values not found directly in the returned @@ -250,6 +311,38 @@ public abstract class SystemReader { */ public abstract FileBasedConfig openSystemConfig(Config parent, FS fs); + /** + * Get the git configuration found in the user home. The configuration will + * be reloaded automatically if the configuration file was modified. Also + * reloads the system config if the system config file was modified. If the + * configuration file wasn't modified returns the cached configuration. + * + * @return the git configuration found in the user home + * @throws ConfigInvalidException + * if configuration is invalid + * @throws IOException + * if something went wrong when reading files + * @since 5.1.9 + */ + public abstract StoredConfig getUserConfig() + throws IOException, ConfigInvalidException; + + /** + * Get the gitconfig configuration found in the system-wide "etc" directory. + * The configuration will be reloaded automatically if the configuration + * file was modified otherwise returns the cached system level config. + * + * @return the gitconfig configuration found in the system-wide "etc" + * directory + * @throws ConfigInvalidException + * if configuration is invalid + * @throws IOException + * if something went wrong when reading files + * @since 5.1.9 + */ + public abstract StoredConfig getSystemConfig() + throws IOException, ConfigInvalidException; + /** * Get the current system time * From 31356f5d181ea1a1c43b44d9dfbe9e8e81aed984 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 17 Aug 2019 00:16:32 +0200 Subject: [PATCH 21/24] FileUtils#lastModifiedInstant should not log error if path doesn't exist Change-Id: Id8447735beb24becb41612d3d29d5351f8273d22 Signed-off-by: Matthias Sohn --- org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java index 80f188cb2..9f7d9a236 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java @@ -55,6 +55,7 @@ import java.nio.file.CopyOption; import java.nio.file.Files; import java.nio.file.InvalidPathException; import java.nio.file.LinkOption; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; @@ -677,9 +678,14 @@ public class FileUtils { try { return Files.getLastModifiedTime(path, LinkOption.NOFOLLOW_LINKS) .toInstant(); + } catch (NoSuchFileException e) { + LOG.debug( + "Cannot read lastModifiedInstant since path {} does not exist", //$NON-NLS-1$ + path); + return Instant.EPOCH; } catch (IOException e) { LOG.error(MessageFormat - .format(JGitText.get().readLastModifiedFailed, path)); + .format(JGitText.get().readLastModifiedFailed, path), e); return Instant.ofEpochMilli(path.toFile().lastModified()); } } From 41da8de001794dcd6a3e782c313cc65abe7d4392 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 17 Aug 2019 00:17:51 +0200 Subject: [PATCH 22/24] Fix NPE in CommitOnlyTest#getHead Change-Id: I05abd00d151cf9834d1a097dd16dc280b62a7edd Signed-off-by: Matthias Sohn --- .../tst/org/eclipse/jgit/api/CommitOnlyTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CommitOnlyTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CommitOnlyTest.java index bbd6ec0bd..43c00518a 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CommitOnlyTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CommitOnlyTest.java @@ -1294,9 +1294,15 @@ public class CommitOnlyTest extends RepositoryTestCase { try { final Repository repo = git.getRepository(); final ObjectId headId = repo.resolve(Constants.HEAD + "^{commit}"); + if (headId == null) { + return ""; + } try (RevWalk rw = new RevWalk(repo)) { final TreeWalk tw = TreeWalk.forPath(repo, path, rw.parseTree(headId)); + if (tw == null) { + return ""; + } return new String(tw.getObjectReader().open(tw.getObjectId(0)) .getBytes()); } From a4216e5382026ff9956af82322c7c5af24e0f245 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 17 Aug 2019 00:18:50 +0200 Subject: [PATCH 23/24] Fix NPE in ObjectIdOwnerMap#get Change-Id: I3812961a27ac410d610ef50c73a28f21bb05ae79 Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/lib/ObjectIdOwnerMap.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdOwnerMap.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdOwnerMap.java index 9df593375..74c712c46 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdOwnerMap.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdOwnerMap.java @@ -135,6 +135,9 @@ public class ObjectIdOwnerMap */ @SuppressWarnings("unchecked") public V get(AnyObjectId toFind) { + if (toFind == null) { + return null; + } int h = toFind.w1; V obj = directory[h & mask][h >>> SEGMENT_SHIFT]; for (; obj != null; obj = (V) obj.next) From 86a567f6152315f5d55309c7119885d0fa3476ce Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 17 Aug 2019 00:20:01 +0200 Subject: [PATCH 24/24] Fix NPE in RebaseTodoFile#parseComments Change-Id: I5487f3c2609eaf2a0ddf71ebb2f6c9701fb7600c Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/lib/RebaseTodoFile.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RebaseTodoFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RebaseTodoFile.java index 06b4b227c..0d3185183 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RebaseTodoFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RebaseTodoFile.java @@ -137,11 +137,13 @@ public class RebaseTodoFile { if (skip != -1) { // try to parse the line as non-comment line = parseLine(buf, skip, lineEnd); - // successfully parsed as non-comment line - // mark this line as a comment explicitly - line.setAction(Action.COMMENT); - // use the read line as comment string - line.setComment(commentString); + if (line != null) { + // successfully parsed as non-comment line + // mark this line as a comment explicitly + line.setAction(Action.COMMENT); + // use the read line as comment string + line.setComment(commentString); + } } } catch (Exception e) { // parsing as non-comment line failed