[git/hg] While reading .git/refs don't put fake hashes in case of error

* Skip them and log error.
* Make RepoStateException checked, to make sure the logic is handled in
  all necessary cases.
* Keep unchecked exception (use IllegalStateException) if a critical
  error happens during repository initialization
  (e.g. .git/HEAD is missing).
* Remove GitTestRepositoryManager since overriding update() is not
  needed anymore (it doesn't throw the exception anymore).
  It automatically fixes IDEA-132298.

platform/dvcs-impl/src/com/intellij/dvcs/DvcsUtil.java

@@ -52,6 +52,7 @@ import com.intellij.util.text.DateFormatUtil;
 import com.intellij.vcs.log.TimedVcsCommit;
 import com.intellij.vcsUtil.VcsUtil;
 import org.intellij.images.editor.ImageFileEditor;
+import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -60,11 +61,10 @@ import java.io.IOException;
 import java.util.*;
 import java.util.concurrent.Callable;
 
-/**
- * @author Kirill Likhodedov
- */
 public class DvcsUtil {
 
+  private static final Logger LOG = Logger.getInstance(DvcsUtil.class);
+
   private static final Logger LOGGER = Logger.getInstance(DvcsUtil.class);
   private static final int IO_RETRIES = 3; // number of retries before fail if an IOException happens during file read.
   private static final int SHORT_HASH_LENGTH = 8;
@@ -219,9 +219,9 @@ public class DvcsUtil {
     }
   };
 
-  public static void assertFileExists(File file, String message) {
+  public static void assertFileExists(File file, String message) throws IllegalStateException {
     if (!file.exists()) {
-      throw new RepoStateException(message);
+      throw new IllegalStateException(message);
     }
   }
 
@@ -234,7 +234,7 @@ public class DvcsUtil {
    * @return file content.
    */
   @NotNull
-  public static String tryLoadFile(@NotNull final File file) {
+  public static String tryLoadFile(@NotNull final File file) throws RepoStateException {
     return tryOrThrow(new Callable<String>() {
       @Override
       public String call() throws Exception {
@@ -243,20 +243,32 @@ public class DvcsUtil {
     }, file);
   }
 
+  @Nullable
+  @Contract("_ , !null -> !null")
+  public static String tryLoadFileOrReturn(@NotNull final File file, @Nullable String defaultValue) {
+    try {
+      return tryLoadFile(file);
+    }
+    catch (RepoStateException e) {
+      LOG.error(e);
+      return defaultValue;
+    }
+  }
+
   /**
    * Tries to execute the given action.
    * If an IOException happens, tries again up to 3 times, and then throws a {@link RepoStateException}.
    * If an other exception happens, rethrows it as a {@link RepoStateException}.
    * In the case of success returns the result of the task execution.
    */
-  public static <T> T tryOrThrow(Callable<T> actionToTry, File fileToLoad) {
+  public static <T> T tryOrThrow(Callable<T> actionToTry, File fileToLoad) throws RepoStateException {
     IOException cause = null;
     for (int i = 0; i < IO_RETRIES; i++) {
       try {
         return actionToTry.call();
       }
       catch (IOException e) {
-        LOGGER.info("IOException while loading " + fileToLoad, e);
+        LOG.info("IOException while loading " + fileToLoad, e);
         cause = e;
       }
       catch (Exception e) {    // this shouldn't happen since only IOExceptions are thrown in clients.

platform/dvcs-impl/src/com/intellij/dvcs/repo/AbstractRepositoryManager.java

@@ -216,7 +216,7 @@ public abstract class AbstractRepositoryManager<T extends Repository> extends Ab
             T repository = createRepository(root);
             repositories.put(root, repository);
           }
-          catch (RepoStateException e) {
+          catch (IllegalStateException e) {
             LOG.error("Couldn't initialize Repository in " + root.getPresentableUrl(), e);
           }
         }

platform/dvcs-impl/src/com/intellij/dvcs/repo/RepoStateException.java

@@ -15,10 +15,7 @@
  */
 package com.intellij.dvcs.repo;
 
-/**
- * @author Nadya Zabrodina
- */
-public class RepoStateException extends RuntimeException {
+public class RepoStateException extends Exception {
 
   public RepoStateException(String message) {
     super(message);

plugins/git4idea/src/META-INF/plugin.xml

@@ -126,7 +126,6 @@
     <component>
       <interface-class>git4idea.repo.GitRepositoryManager</interface-class>
       <implementation-class>git4idea.repo.GitRepositoryManager</implementation-class>
-      <headless-implementation-class>git4idea.test.GitTestRepositoryManager</headless-implementation-class>
     </component>
   </project-components>
 

plugins/git4idea/src/git4idea/repo/GitRepositoryReader.java

@@ -78,7 +78,7 @@ class GitRepositoryReader {
   @Nullable
   private static Hash createHash(@Nullable String hash) {
     try {
-      return hash == null ? GitBranch.DUMMY_HASH : HashImpl.build(hash);
+      return hash == null ? null : HashImpl.build(hash);
     }
     catch (Throwable t) {
       LOG.info(t);
@@ -139,7 +139,7 @@ class GitRepositoryReader {
       String branchName = head.ref;
       String hash = readCurrentRevision();  // TODO we know the branch name, so no need to read head twice
       Hash h = createHash(hash);
-      if (h == null) {
+      if (branchName == null || h == null) {
         return null;
       }
       return new GitLocalBranch(branchName, h);
@@ -168,7 +168,16 @@ class GitRepositoryReader {
     if (!headName.exists()) {
       return null;
     }
-    String branchName = DvcsUtil.tryLoadFile(headName);
+
+    String branchName;
+    try {
+      branchName = DvcsUtil.tryLoadFile(headName);
+    }
+    catch (RepoStateException e) {
+      LOG.error(e);
+      return null;
+    }
+
     File branchFile = findBranchFile(branchName);
     if (!branchFile.exists()) { // can happen when rebasing from detached HEAD: IDEA-93806
       return null;
@@ -213,14 +222,20 @@ class GitRepositoryReader {
       return null;
     }
 
-    List<HashAndName> hashAndNames = readPackedRefsFile(new Condition<HashAndName>() {
-      @Override
-      public boolean value(HashAndName hashAndName) {
-        return hashAndName.name.endsWith(ref);
-      }
-    });
-    HashAndName item = ContainerUtil.getFirstItem(hashAndNames);
-    return item == null ? null : item.hash;
+    try {
+      List<HashAndName> hashAndNames = readPackedRefsFile(new Condition<HashAndName>() {
+        @Override
+        public boolean value(HashAndName hashAndName) {
+          return hashAndName.name.endsWith(ref);
+        }
+      });
+      HashAndName item = ContainerUtil.getFirstItem(hashAndNames);
+      return item == null ? null : item.hash;
+    }
+    catch (RepoStateException e) {
+      LOG.error(e);
+      return null;
+    }
   }
 
   /**
@@ -228,7 +243,7 @@ class GitRepositoryReader {
    *                            and return a singleton list of this entry.
    *                            If null, the whole file is read, and all valid entries are returned.
    */
-  private List<HashAndName> readPackedRefsFile(@Nullable final Condition<HashAndName> firstMatchCondition) {
+  private List<HashAndName> readPackedRefsFile(@Nullable final Condition<HashAndName> firstMatchCondition) throws RepoStateException {
     return DvcsUtil.tryOrThrow(new Callable<List<HashAndName>>() {
       @Override
       public List<HashAndName> call() throws Exception {
@@ -292,9 +307,14 @@ class GitRepositoryReader {
   GitBranchesCollection readBranches(@NotNull Collection<GitRemote> remotes) {
     Set<GitLocalBranch> localBranches = readUnpackedLocalBranches();
     Set<GitRemoteBranch> remoteBranches = readUnpackedRemoteBranches(remotes);
-    GitBranchesCollection packedBranches = readPackedBranches(remotes);
-    localBranches.addAll(packedBranches.getLocalBranches());
-    remoteBranches.addAll(packedBranches.getRemoteBranches());
+    try {
+      GitBranchesCollection packedBranches = readPackedBranches(remotes);
+      localBranches.addAll(packedBranches.getLocalBranches());
+      remoteBranches.addAll(packedBranches.getRemoteBranches());
+    }
+    catch (RepoStateException e) {
+      LOG.error(e);
+    }
     return new GitBranchesCollection(localBranches, remoteBranches);
   }
 
@@ -318,13 +338,7 @@ class GitRepositoryReader {
 
   @Nullable
   private static String loadHashFromBranchFile(@NotNull File branchFile) {
-    try {
-      return DvcsUtil.tryLoadFile(branchFile);
-    }
-    catch (RepoStateException e) {  // notify about error but don't break the process
-      LOG.error("Couldn't read " + branchFile, e);
-    }
-    return null;
+    return DvcsUtil.tryLoadFileOrReturn(branchFile, null);
   }
 
   /**
@@ -365,7 +379,7 @@ class GitRepositoryReader {
    * @param remotes
    */
   @NotNull
-  private GitBranchesCollection readPackedBranches(@NotNull final Collection<GitRemote> remotes) {
+  private GitBranchesCollection readPackedBranches(@NotNull final Collection<GitRemote> remotes) throws RepoStateException {
     final Set<GitLocalBranch> localBranches = new HashSet<GitLocalBranch>();
     final Set<GitRemoteBranch> remoteBranches = new HashSet<GitRemoteBranch>();
     if (!myPackedRefsFile.exists()) {
@@ -417,15 +431,22 @@ class GitRepositoryReader {
       return new GitStandardRemoteBranch(remote, branchName, hash);
     }
   }
-
-  @NotNull
+  
+  @Nullable
   private static String readBranchFile(@NotNull File branchFile) {
-    return DvcsUtil.tryLoadFile(branchFile);
+    return DvcsUtil.tryLoadFileOrReturn(branchFile, null);
   }
 
   @NotNull
   private Head readHead() {
-    String headContent = DvcsUtil.tryLoadFile(myHeadFile);
+    String headContent;
+    try {
+      headContent = DvcsUtil.tryLoadFile(myHeadFile);
+    }
+    catch (RepoStateException e) {
+      LOG.error(e);
+      return new Head(false, null);
+    }
     Matcher matcher = BRANCH_PATTERN.matcher(headContent);
     if (matcher.matches()) {
       return new Head(true, matcher.group(1));
@@ -439,7 +460,8 @@ class GitRepositoryReader {
       LOG.info(".git/HEAD has not standard format: [" + headContent + "]. We've parsed branch [" + matcher.group(1) + "]");
       return new Head(true, matcher.group(1));
     }
-    throw new RepoStateException("Invalid format of the .git/HEAD file: [" + headContent + "]");
+    LOG.error(new RepoStateException("Invalid format of the .git/HEAD file: [" + headContent + "]"));
+    return new Head(false, null);
   }
 
   /**
@@ -514,10 +536,10 @@ class GitRepositoryReader {
    * Container to hold two information items: current .git/HEAD value and is Git on branch.
    */
   private static class Head {
-    @NotNull private final String ref;
+    @Nullable private final String ref;
     private final boolean isBranch;
 
-    Head(boolean branch, @NotNull String ref) {
+    Head(boolean branch, @Nullable String ref) {
       isBranch = branch;
       this.ref = ref;
     }

plugins/git4idea/tests/git4idea/test/GitTestRepositoryManager.java

@@ -1,60 +0,0 @@
-/*
- * Copyright 2000-2014 JetBrains s.r.o.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package git4idea.test;
-
-import com.intellij.dvcs.repo.RepoStateException;
-import com.intellij.openapi.project.Project;
-import com.intellij.openapi.util.Disposer;
-import com.intellij.openapi.vcs.ProjectLevelVcsManager;
-import com.intellij.openapi.vfs.VirtualFile;
-import git4idea.GitPlatformFacade;
-import git4idea.repo.GitRepository;
-import git4idea.repo.GitRepositoryImpl;
-import git4idea.repo.GitRepositoryManager;
-import org.jetbrains.annotations.NotNull;
-
-public class GitTestRepositoryManager extends GitRepositoryManager {
-
-  @NotNull private final Project myProject;
-  @NotNull private final GitPlatformFacade myFacade;
-
-  public GitTestRepositoryManager(@NotNull Project project,
-                                  @NotNull GitPlatformFacade platformFacade,
-                                  @NotNull ProjectLevelVcsManager vcsManager) {
-    super(project, platformFacade, vcsManager);
-    myProject = project;
-    myFacade = platformFacade;
-  }
-
-  @NotNull
-  @Override
-  protected GitRepository createRepository(@NotNull VirtualFile root) {
-    return new GitRepositoryImpl(root, myFacade, myProject, this, false) {
-      @Override
-      public void update() {
-        try {
-          super.update();
-        }
-        catch (RepoStateException e) {
-          if (!Disposer.isDisposed(this)) { // project dir will simply be removed during dispose
-            throw e;
-          }
-        }
-      }
-    };
-  }
-
-}

plugins/hg4idea/src/org/zmlx/hg4idea/repo/HgRepositoryReader.java

@@ -19,6 +19,7 @@ import com.intellij.dvcs.DvcsUtil;
 import com.intellij.dvcs.repo.RepoStateException;
 import com.intellij.dvcs.repo.Repository;
 import com.intellij.openapi.components.ServiceManager;
+import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.util.io.FileUtil;
 import com.intellij.vcs.log.Hash;
 import com.intellij.vcs.log.VcsLogObjectsFactory;
@@ -46,6 +47,8 @@ import java.util.regex.Pattern;
  */
 public class HgRepositoryReader {
 
+  private static final Logger LOG = Logger.getInstance(HgRepositoryReader.class);
+
   private static Pattern HASH_NAME = Pattern.compile("\\s*([0-9a-fA-F]+)\\s+(.+)");
   private static Pattern HASH_STATUS_NAME = Pattern.compile("\\s*([0-9a-fA-F]+)\\s+\\w\\s+(.+)");
   //hash + name_or_revision_num; hash + status_character +  name_or_revision_num
@@ -114,7 +117,8 @@ public class HgRepositoryReader {
     }
     catch (IOException e) {
       // dirState exists if not fresh,  if we could not load dirState info repository must be corrupted
-      throw new RepoStateException("IOException while trying to read current repository state information.", e);
+      LOG.error("IOException while trying to read current repository state information.", e);
+      return null;
     }
   }
 
@@ -139,7 +143,14 @@ public class HgRepositoryReader {
   @Nullable
   public String readCurrentTipRevision() {
     if (!isBranchInfoAvailable()) return null;
-    String[] branchesWithHeads = DvcsUtil.tryLoadFile(myBranchHeadsFile).split("\n");
+    String[] branchesWithHeads;
+    try {
+      branchesWithHeads = DvcsUtil.tryLoadFile(myBranchHeadsFile).split("\n");
+    }
+    catch (RepoStateException e) {
+      LOG.error(e);
+      return null;
+    }
     String head = branchesWithHeads[0];
     Matcher matcher = HASH_NAME.matcher(head);
     if (matcher.matches()) {
@@ -162,7 +173,7 @@ public class HgRepositoryReader {
    */
   @NotNull
   public String readCurrentBranch() {
-    return branchExist() ? DvcsUtil.tryLoadFile(myCurrentBranch) : HgRepository.DEFAULT_BRANCH;
+    return branchExist() ? DvcsUtil.tryLoadFileOrReturn(myCurrentBranch, HgRepository.DEFAULT_BRANCH) : HgRepository.DEFAULT_BRANCH;
   }
 
   @NotNull
@@ -171,7 +182,7 @@ public class HgRepositoryReader {
     // Set<String> branchNames = new HashSet<String>();
     if (isBranchInfoAvailable()) {
       Pattern activeBranchPattern = myStatusInBranchFile ? HASH_STATUS_NAME : HASH_NAME;
-      String[] branchesWithHeads = DvcsUtil.tryLoadFile(myBranchHeadsFile).split("\n");
+      String[] branchesWithHeads = DvcsUtil.tryLoadFileOrReturn(myBranchHeadsFile, "").split("\n");
       // first one - is a head revision: head hash + head number;
       for (int i = 1; i < branchesWithHeads.length; ++i) {
         Matcher matcher = activeBranchPattern.matcher(branchesWithHeads[i]);
@@ -241,7 +252,7 @@ public class HgRepositoryReader {
     if (!fileWithReferences.exists()) {
       return refs;
     }
-    String[] namesWithHashes = DvcsUtil.tryLoadFile(fileWithReferences).split("\n");
+    String[] namesWithHashes = DvcsUtil.tryLoadFileOrReturn(fileWithReferences, "").split("\n");
     for (String str : namesWithHashes) {
       Matcher matcher = HASH_NAME.matcher(str);
       if (matcher.matches()) {
@@ -253,7 +264,7 @@ public class HgRepositoryReader {
 
   @Nullable
   public String readCurrentBookmark() {
-    return myCurrentBookmark.exists() ? DvcsUtil.tryLoadFile(myCurrentBookmark) : null;
+    return myCurrentBookmark.exists() ? DvcsUtil.tryLoadFileOrReturn(myCurrentBookmark, "") : null;
   }
 
   @NotNull