Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Wolf2019-07-17 11:49:07 -0400
committerMatthias Sohn2019-07-22 18:21:39 -0400
commit667dc0c5175637e32b4b9f41c37006e7f827beeb (patch)
tree682271fe39b60240ec2c65365a5bb7287752e8c4
parent374c142de4634d0888425368b138957c38498482 (diff)
downloadegit-667dc0c5175637e32b4b9f41c37006e7f827beeb.tar.gz
egit-667dc0c5175637e32b4b9f41c37006e7f827beeb.tar.xz
egit-667dc0c5175637e32b4b9f41c37006e7f827beeb.zip
Log an error if the reflog cannot be read
EGit reads the reflog in three places: in the SwitchToMenu, when trying to get a short branch name, and in the reflog view. The latter uses JGit's ReflogCommand and has its own fatal error handling. In the other two places, a failure to read the reflog may cause severe UI functionality loss. Therefore catch RuntimeExceptions, too, and just log them. In SwitchToMenu, also just log IOExceptions when the ref log cannot be read. That way, there's a fair chance that the menu will still show some branches. Bug: 549235 Change-Id: I914536a65cec9d7e5c04199e989926d46e607427 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
-rw-r--r--org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryUtil.java68
-rw-r--r--org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java3
-rw-r--r--org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties1
-rw-r--r--org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/actions/SwitchToMenuTest.java28
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/SwitchToMenu.java27
5 files changed, 92 insertions, 35 deletions
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryUtil.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryUtil.java
index c644cf7a1..a4382e8b7 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryUtil.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryUtil.java
@@ -172,6 +172,33 @@ public class RepositoryUtil {
}
/**
+ * Reads a reflog (in reverse), returning an empty list and logging
+ * exceptions if the reflog cannot be parsed.
+ *
+ * @param repository
+ * to read a reflog from
+ * @param refName
+ * identifying the reflog
+ * @return the entries of the reflog
+ * @throws IOException
+ * if the reflog file itself cannot be read
+ */
+ public static List<ReflogEntry> safeReadReflog(Repository repository,
+ String refName) throws IOException {
+ ReflogReader reflogReader = repository.getReflogReader(refName);
+ if (reflogReader != null) {
+ try {
+ return reflogReader.getReverseEntries();
+ } catch (RuntimeException e) {
+ Activator.logError(MessageFormat.format(
+ CoreText.RepositoryUtil_ReflogCorrupted, refName,
+ repository.getDirectory()), e);
+ }
+ }
+ return Collections.emptyList();
+ }
+
+ /**
* Tries to map a commit to a symbolic reference.
* <p>
* This value will be cached for the given commit ID unless refresh is
@@ -208,28 +235,27 @@ public class RepositoryUtil {
}
try {
- ReflogReader reflogReader = repository.getReflogReader(Constants.HEAD);
- if (reflogReader != null) {
- List<ReflogEntry> lastEntry = reflogReader.getReverseEntries();
- for (ReflogEntry entry : lastEntry) {
- if (entry.getNewId().name().equals(commitId)) {
- CheckoutEntry checkoutEntry = entry.parseCheckout();
- if (checkoutEntry != null) {
- Ref ref = repository
- .findRef(checkoutEntry.getToBranch());
- if (ref != null) {
- ObjectId objectId = ref.getObjectId();
- if (objectId != null && objectId.getName()
- .equals(commitId)) {
- return checkoutEntry.getToBranch();
- }
- ref = repository.getRefDatabase().peel(ref);
+ List<ReflogEntry> lastEntry = safeReadReflog(repository,
+ Constants.HEAD);
+ for (ReflogEntry entry : lastEntry) {
+ if (entry.getNewId().name().equals(commitId)) {
+ CheckoutEntry checkoutEntry = entry.parseCheckout();
+ if (checkoutEntry != null) {
+ Ref ref = repository
+ .findRef(checkoutEntry.getToBranch());
+ if (ref != null) {
+ ObjectId objectId = ref.getObjectId();
+ if (objectId != null && objectId.getName()
+ .equals(commitId)) {
+ return checkoutEntry.getToBranch();
}
- if (ref != null) {
- ObjectId id = ref.getPeeledObjectId();
- if (id != null && id.getName().equals(commitId)) {
- return checkoutEntry.getToBranch();
- }
+ ref = repository.getRefDatabase().peel(ref);
+ }
+ if (ref != null) {
+ ObjectId id = ref.getPeeledObjectId();
+ if (id != null
+ && id.getName().equals(commitId)) {
+ return checkoutEntry.getToBranch();
}
}
}
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java
index d8f2ef3f4..581485b66 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java
@@ -238,6 +238,9 @@ public class CoreText extends NLS {
public static String RepositoryUtil_noHead;
/** */
+ public static String RepositoryUtil_ReflogCorrupted;
+
+ /** */
public static String RemoteRefUpdateCantBeReused;
/** */
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties
index 823af0115..a3bf98dc9 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties
@@ -90,6 +90,7 @@ RepositoryPathChecker_errNotAbsoluteRepoPath={0} is not an absolute directory pa
RepositoryPathChecker_errNoURL=Don't enter a URL but a local absolute path
RepositoryUtil_DirectoryIsNotGitDirectory=Repository directory to configure does not look like a Git directory: {0}
RepositoryUtil_noHead=NO-HEAD
+RepositoryUtil_ReflogCorrupted=Cannot parse the reflog for {0} in repository {1}
ResetOperation_performingReset=Performing {0} reset to {1}
ResourceUtil_SaveLocalHistoryFailed=Saving local history of resource {0} failed
ResourceUtil_mapProjectJob=Mapping Git provider to projects
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/actions/SwitchToMenuTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/actions/SwitchToMenuTest.java
index 3c8594784..3c16f9692 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/actions/SwitchToMenuTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/actions/SwitchToMenuTest.java
@@ -13,10 +13,12 @@
package org.eclipse.egit.ui.internal.actions;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.io.File;
+import java.nio.file.Files;
import java.util.ArrayList;
import java.util.List;
@@ -105,7 +107,31 @@ public class SwitchToMenuTest extends LocalRepositoryTestCase {
selectionWithProj1Common();
// delete reflog again to not confuse other tests
- new File(gitDir, Constants.LOGS + "/" + Constants.HEAD).delete();
+ assertTrue(new File(gitDir, Constants.LOGS + "/" + Constants.HEAD)
+ .delete());
+ }
+
+ @Test
+ public void selectionWithCorruptedReflog() throws Exception {
+ File gitDir = createProjectAndCommitToRepository();
+
+ // create additional reflog entries
+ try (Git git = new Git(lookupRepository(gitDir))) {
+ git.checkout().setName("stable").call();
+ git.checkout().setName("master").call();
+ }
+
+ // Corrupt the reflog
+ File reflog = new File(gitDir, Constants.LOGS + "/" + Constants.HEAD);
+ List<String> lines = Files.readAllLines(reflog.toPath());
+ assertTrue("Expected some lines in the reflog", lines.size() > 1);
+ lines.add(1,
+ "INTENTIONALLY CORRUPTED REFLOG. Just some text that definitely shouldn't be in a reflog.");
+ Files.write(reflog.toPath(), lines);
+
+ selectionWithProj1Common();
+
+ assertTrue(reflog.delete());
}
private void selectionWithProj1Common() {
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/SwitchToMenu.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/SwitchToMenu.java
index 76e7226a2..20675976c 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/SwitchToMenu.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/SwitchToMenu.java
@@ -24,6 +24,7 @@ import java.util.Set;
import java.util.TreeSet;
import java.util.stream.Stream;
+import org.eclipse.egit.core.RepositoryUtil;
import org.eclipse.egit.ui.Activator;
import org.eclipse.egit.ui.internal.CommonUtils;
import org.eclipse.egit.ui.internal.UIIcons;
@@ -41,7 +42,6 @@ import org.eclipse.jgit.lib.ConfigConstants;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.ReflogEntry;
-import org.eclipse.jgit.lib.ReflogReader;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.swt.SWT;
import org.eclipse.swt.events.SelectionAdapter;
@@ -180,10 +180,17 @@ public class SwitchToMenu extends ContributionItem implements
try {
List<Map<String, Ref>> activeBranches = new ArrayList<>();
- for (Repository repository : repositories) {
- Map<String, Ref> branchRefMapping = getMostActiveBranches(
- repository, MAX_NUM_MENU_ENTRIES);
- activeBranches.add(branchRefMapping);
+ try {
+ for (Repository repository : repositories) {
+ Map<String, Ref> branchRefMapping = getMostActiveBranches(
+ repository, MAX_NUM_MENU_ENTRIES);
+ activeBranches.add(branchRefMapping);
+ }
+ } catch (IOException e) {
+ Activator.logError(e.getLocalizedMessage(), e);
+ // The intersection should be empty if we cannot read the reflog
+ // of one repository.
+ activeBranches.clear();
}
Set<String> activeBranchIntersection = getBranchNameIntersection(
@@ -309,14 +316,8 @@ public class SwitchToMenu extends ContributionItem implements
.getRefs(Constants.R_HEADS);
Map<String, Ref> activeRefs = new HashMap<>();
- ReflogReader reflogReader = repository.getReflogReader(Constants.HEAD);
- List<ReflogEntry> reflogEntries;
- if (reflogReader == null) {
- return Collections.emptyMap();
- }
-
- reflogEntries = reflogReader.getReverseEntries();
-
+ List<ReflogEntry> reflogEntries = RepositoryUtil
+ .safeReadReflog(repository, Constants.HEAD);
for (ReflogEntry entry : reflogEntries) {
CheckoutEntry checkout = entry.parseCheckout();
if (checkout != null) {

Back to the top