diff options
Diffstat (limited to 'sonar-scanner-engine')
7 files changed, 132 insertions, 80 deletions
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/CompositeBlameCommand.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/CompositeBlameCommand.java index 83223959ee8..85f0bb26949 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/CompositeBlameCommand.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/CompositeBlameCommand.java @@ -20,14 +20,20 @@ package org.sonar.scm.git; import java.io.File; +import java.io.IOException; import java.nio.file.Files; +import java.util.HashSet; import java.util.List; -import java.util.concurrent.ForkJoinPool; +import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import java.util.stream.Stream; -import java.util.stream.StreamSupport; import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; +import org.eclipse.jgit.treewalk.TreeWalk; import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.scm.BlameCommand; import org.sonar.api.batch.scm.BlameLine; @@ -60,29 +66,60 @@ public class CompositeBlameCommand extends BlameCommand { if (cloneIsInvalid(gitBaseDir)) { return; } - nativeGitEnabled = nativeCmd.isEnabled(basedir.toPath()); - Stream<InputFile> stream = StreamSupport.stream(input.filesToBlame().spliterator(), true); - ForkJoinPool forkJoinPool = new ForkJoinPool(Runtime.getRuntime().availableProcessors(), new GitThreadFactory(), null, false); - // exceptions thrown by the blame method will be ignored - forkJoinPool.submit(() -> stream.forEach(inputFile -> blame(output, git, gitBaseDir, inputFile))); + + Set<String> committedFiles = collectAllCommittedFiles(repo); + nativeGitEnabled = nativeCmd.isEnabled(); + ExecutorService executorService = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors(), new GitThreadFactory()); + + for (InputFile inputFile : input.filesToBlame()) { + String filename = pathResolver.relativePath(gitBaseDir, inputFile.file()); + if (filename == null || !committedFiles.contains(filename)) { + continue; + } + // exceptions thrown by the blame method will be ignored + executorService.submit(() -> blame(output, git, gitBaseDir, inputFile, filename)); + } + + executorService.shutdown(); try { - forkJoinPool.shutdown(); - forkJoinPool.awaitTermination(Long.MAX_VALUE, TimeUnit.SECONDS); + executorService.awaitTermination(Long.MAX_VALUE, TimeUnit.SECONDS); } catch (InterruptedException e) { LOG.info("Git blame interrupted"); } } } - private void blame(BlameOutput output, Git git, File gitBaseDir, InputFile inputFile) { - String filename = pathResolver.relativePath(gitBaseDir, inputFile.file()); + private static Set<String> collectAllCommittedFiles(Repository repo) { + try { + Set<String> files = new HashSet<>(); + + try (RevWalk revWalk = new RevWalk(repo)) { + RevCommit head = revWalk.parseCommit(repo.resolve(Constants.HEAD)); + try (TreeWalk treeWalk = new TreeWalk(repo)) { + treeWalk.addTree(head.getTree()); + treeWalk.setRecursive(true); + + while (treeWalk.next()) { + String path = treeWalk.getPathString(); + files.add(path); + } + } + } + return files; + } catch (IOException e) { + throw new IllegalStateException("Failed to find all committed files", e); + } + } + + private void blame(BlameOutput output, Git git, File gitBaseDir, InputFile inputFile, String filename) { LOG.debug("Blame file {}", filename); List<BlameLine> blame = null; if (nativeGitEnabled) { try { blame = nativeCmd.blame(gitBaseDir.toPath(), filename); } catch (Exception e) { - // fallback to jgit + LOG.debug("Native git blame failed. Falling back to jgit: " + filename, e); + nativeGitEnabled = false; } } @@ -92,7 +129,7 @@ public class CompositeBlameCommand extends BlameCommand { if (!blame.isEmpty()) { if (blame.size() == inputFile.lines() - 1) { - // SONARPLUGINS-3097 Git do not report blame on last empty line + // SONARPLUGINS-3097 Git does not report blame on last empty line blame.add(blame.get(blame.size() - 1)); } output.blameResult(inputFile, blame); @@ -106,10 +143,8 @@ public class CompositeBlameCommand extends BlameCommand { } if (Files.isRegularFile(gitBaseDir.toPath().resolve(".git/shallow"))) { - LOG.warn("Shallow clone detected, no blame information will be provided. " - + "You can convert to non-shallow with 'git fetch --unshallow'."); - analysisWarnings.addUnique("Shallow clone detected during the analysis. " - + "Some files will miss SCM information. This will affect features like auto-assignment of issues. " + LOG.warn("Shallow clone detected, no blame information will be provided. " + "You can convert to non-shallow with 'git fetch --unshallow'."); + analysisWarnings.addUnique("Shallow clone detected during the analysis. " + "Some files will miss SCM information. This will affect features like auto-assignment of issues. " + "Please configure your build to disable shallow clone."); return true; } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitBlameCommand.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitBlameCommand.java index 3fffbe0ea83..1f3dbdaf0d6 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitBlameCommand.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitBlameCommand.java @@ -24,12 +24,13 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.nio.file.Path; import java.time.Instant; -import java.util.ArrayList; import java.util.Date; +import java.util.LinkedList; import java.util.List; import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.annotation.Nullable; import org.sonar.api.batch.scm.BlameLine; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; @@ -37,7 +38,6 @@ import org.springframework.beans.factory.annotation.Autowired; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.emptyList; -import static java.util.Collections.unmodifiableList; import static org.sonar.api.utils.Preconditions.checkState; public class GitBlameCommand { @@ -62,10 +62,10 @@ public class GitBlameCommand { this.gitCommand = gitCommand; } - public boolean isEnabled(Path baseDir) { + public boolean isEnabled() { try { MutableString stdOut = new MutableString(); - executeCommand(baseDir, l -> stdOut.string = l, gitCommand, "--version"); + executeCommand(null, l -> stdOut.string = l, gitCommand, "--version"); return stdOut.string != null && stdOut.string.startsWith("git version"); } catch (Exception e) { LOG.debug("Failed to find git native client", e); @@ -73,21 +73,21 @@ public class GitBlameCommand { } } - public List<BlameLine> blame(Path baseDir, String fileName) { + public List<BlameLine> blame(Path baseDir, String fileName) throws Exception { BlameOutputProcessor outputProcessor = new BlameOutputProcessor(); try { executeCommand(baseDir, outputProcessor::process, gitCommand, BLAME_COMMAND, BLAME_LINE_PORCELAIN_FLAG, IGNORE_WHITESPACES, fileName); - return outputProcessor.getBlameLines(); - } catch (Exception e) { - LOG.debug("Blame failed for " + fileName, e); + } catch (UncommittedLineException e) { + LOG.debug("Unable to blame file '{}' - it has uncommitted changes", fileName); return emptyList(); } + return outputProcessor.getBlameLines(); } - private void executeCommand(Path baseDir, Consumer<String> stdOutLineConsumer, String... command) throws Exception { + private static void executeCommand(@Nullable Path baseDir, Consumer<String> stdOutLineConsumer, String... command) throws Exception { ProcessBuilder pb = new ProcessBuilder() .command(command) - .directory(baseDir.toFile()); + .directory(baseDir != null ? baseDir.toFile() : null); Process p = pb.start(); try { @@ -103,20 +103,19 @@ public class GitBlameCommand { if (exit != 0) { throw new IllegalStateException(String.format("Command execution exited with code: %d", exit)); } - } finally { p.destroy(); } } private static class BlameOutputProcessor { - private final List<BlameLine> blameLines = new ArrayList<>(); - private String sha1; - private String committerTime; - private String committerMail; + private final List<BlameLine> blameLines = new LinkedList<>(); + private String sha1 = null; + private String committerTime = null; + private String committerMail = null; public List<BlameLine> getBlameLines() { - return unmodifiableList(blameLines); + return blameLines; } public void process(String line) { @@ -133,7 +132,7 @@ public class GitBlameCommand { } committerMail = matcher.group(1); if (committerMail.equals("not.committed.yet")) { - throw new IllegalStateException("Uncommitted line found"); + throw new UncommittedLineException(); } } } @@ -159,4 +158,8 @@ public class GitBlameCommand { private static class MutableString { String string; } + + private static class UncommittedLineException extends RuntimeException { + + } } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitThreadFactory.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitThreadFactory.java index 6579c82564f..5316c86e7b6 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitThreadFactory.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitThreadFactory.java @@ -22,15 +22,17 @@ package org.sonar.scm.git; import java.util.concurrent.ForkJoinPool; import java.util.concurrent.ForkJoinPool.ForkJoinWorkerThreadFactory; import java.util.concurrent.ForkJoinWorkerThread; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicInteger; -public class GitThreadFactory implements ForkJoinWorkerThreadFactory { +public class GitThreadFactory implements ThreadFactory { private static final String NAME_PREFIX = "git-scm-"; - private int i = 0; + private final AtomicInteger count = new AtomicInteger(0); @Override - public ForkJoinWorkerThread newThread(ForkJoinPool pool) { - ForkJoinWorkerThread thread = ForkJoinPool.defaultForkJoinWorkerThreadFactory.newThread(pool); - thread.setName(NAME_PREFIX + i++); - return thread; + public Thread newThread(Runnable r) { + Thread t = new Thread(r); + t.setName(NAME_PREFIX + count.getAndIncrement()); + return t; } } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/JGitBlameCommand.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/JGitBlameCommand.java index e9868a03c52..97d797785e9 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/JGitBlameCommand.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/JGitBlameCommand.java @@ -34,7 +34,6 @@ public class JGitBlameCommand { private static final Logger LOG = Loggers.get(JGitBlameCommand.class); public List<BlameLine> blame(Git git, String filename) { - LOG.debug("Blame file {}", filename); BlameResult blameResult; try { blameResult = git.blame() diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/CompositeBlameCommandTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/CompositeBlameCommandTest.java index 5b566b72724..0604b456b93 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/CompositeBlameCommandTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/CompositeBlameCommandTest.java @@ -50,6 +50,7 @@ import org.sonar.api.utils.log.LogTester; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assume.assumeTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.startsWith; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -88,30 +89,54 @@ public class CompositeBlameCommandTest { } @Test - public void fallback_to_jgit_if_native_git_fails() throws IOException { + public void fallback_to_jgit_if_native_git_fails() throws Exception { GitBlameCommand gitCmd = mock(GitBlameCommand.class); BlameCommand blameCmd = new CompositeBlameCommand(analysisWarnings, pathResolver, jGitBlameCommand, gitCmd); File projectDir = createNewTempFolder(); javaUnzip("dummy-git.zip", projectDir); File baseDir = new File(projectDir, "dummy-git"); - when(gitCmd.isEnabled(baseDir.toPath())).thenReturn(true); + when(gitCmd.isEnabled()).thenReturn(true); when(gitCmd.blame(baseDir.toPath(), DUMMY_JAVA)).thenThrow(new IllegalStateException()); setUpBlameInputWithFile(baseDir.toPath()); TestBlameOutput output = new TestBlameOutput(); blameCmd.blame(input, output); assertThat(output.blame).hasSize(1); assertThat(output.blame.get(input.filesToBlame().iterator().next())).hasSize(29); + + // only tried once + verify(gitCmd).blame(any(Path.class), any(String.class)); + assertThat(logTester.logs()).contains("Native git blame failed. Falling back to jgit: src/main/java/org/dummy/Dummy.java"); } @Test - public void use_native_git_by_default() throws IOException { + public void skip_files_not_committed() throws Exception { + // skip if git not installed + assumeTrue(gitBlameCommand.isEnabled()); + + JGitBlameCommand jgit = mock(JGitBlameCommand.class); + BlameCommand blameCmd = new CompositeBlameCommand(analysisWarnings, pathResolver, jgit, gitBlameCommand); File projectDir = createNewTempFolder(); javaUnzip("dummy-git.zip", projectDir); + File baseDir = new File(projectDir, "dummy-git"); + setUpBlameInputWithFile(baseDir.toPath()); + TestBlameOutput output = new TestBlameOutput(); + blameCmd.blame(input, output); + assertThat(output.blame).hasSize(1); + assertThat(output.blame.get(input.filesToBlame().iterator().next())).hasSize(29); + // never had to fall back to jgit + verifyNoInteractions(jgit); + } + + @Test + public void use_native_git_by_default() throws IOException { // skip test if git is not installed - assumeTrue(gitBlameCommand.isEnabled(baseDir.toPath())); + assumeTrue(gitBlameCommand.isEnabled()); + File projectDir = createNewTempFolder(); + javaUnzip("dummy-git.zip", projectDir); + File baseDir = new File(projectDir, "dummy-git"); JGitBlameCommand jgit = mock(JGitBlameCommand.class); BlameCommand blameCmd = new CompositeBlameCommand(analysisWarnings, pathResolver, jgit, gitBlameCommand); @@ -188,7 +213,6 @@ public class CompositeBlameCommandTest { blameCommand.blame(input, output); } - @Test public void return_early_when_clone_with_reference_detected() throws IOException { File projectDir = createNewTempFolder(); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitBlameCommandTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitBlameCommandTest.java index 0de3252668c..4961048e7e2 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitBlameCommandTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitBlameCommandTest.java @@ -30,6 +30,7 @@ import java.util.List; import org.apache.commons.io.FileUtils; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -39,6 +40,7 @@ import org.sonar.api.utils.System2; import org.sonar.api.utils.log.LogTester; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assume.assumeTrue; import static org.sonar.scm.git.GitUtils.createFile; import static org.sonar.scm.git.GitUtils.createRepository; @@ -53,8 +55,13 @@ public class GitBlameCommandTest { public LogTester logTester = new LogTester(); private final GitBlameCommand blameCommand = new GitBlameCommand(); + @Before + public void skipTestsIfNoGitFound() { + assumeTrue(blameCommand.isEnabled()); + } + @Test - public void blame_collects_all_lines() throws IOException { + public void blame_collects_all_lines() throws Exception { File projectDir = createNewTempFolder(); javaUnzip("dummy-git.zip", projectDir); File baseDir = new File(projectDir, "dummy-git"); @@ -85,7 +92,7 @@ public class GitBlameCommandTest { } @Test - public void modified_file_returns_no_blame() throws IOException { + public void modified_file_returns_no_blame() throws Exception { File projectDir = createNewTempFolder(); javaUnzip("dummy-git.zip", projectDir); @@ -98,22 +105,7 @@ public class GitBlameCommandTest { } @Test - public void new_file_returns_no_blame() throws IOException { - File projectDir = createNewTempFolder(); - javaUnzip("dummy-git.zip", projectDir); - - File baseDir = new File(projectDir, "dummy-git"); - String relativePath2 = "src/main/java/org/dummy/Dummy2.java"; - - // Emulate a new file - FileUtils.copyFile(new File(baseDir, DUMMY_JAVA), new File(baseDir, relativePath2)); - - assertThat(blameCommand.blame(baseDir.toPath(), DUMMY_JAVA)).hasSize(29); - assertThat(blameCommand.blame(baseDir.toPath(), relativePath2)).isEmpty(); - } - - @Test - public void symlink_doesnt_fail() throws IOException { + public void throw_exception_if_symlink_found() throws Exception { assumeTrue(!System2.INSTANCE.isOsWindows()); File projectDir = temp.newFolder(); javaUnzip("dummy-git.zip", projectDir); @@ -125,32 +117,30 @@ public class GitBlameCommandTest { Files.createSymbolicLink(baseDir.resolve(relativePath2), baseDir.resolve(DUMMY_JAVA)); blameCommand.blame(baseDir, DUMMY_JAVA); - blameCommand.blame(baseDir, relativePath2); + assertThatThrownBy(() -> blameCommand.blame(baseDir, relativePath2)).isInstanceOf(IllegalStateException.class); } @Test - public void git_should_be_detected() throws IOException { - Path baseDir = temp.newFolder().toPath(); + public void git_should_be_detected() { GitBlameCommand blameCommand = new GitBlameCommand(); - assertThat(blameCommand.isEnabled(baseDir)).isTrue(); + assertThat(blameCommand.isEnabled()).isTrue(); } @Test - public void git_should_not_be_detected() throws IOException { - Path baseDir = temp.newFolder().toPath(); + public void git_should_not_be_detected() { GitBlameCommand blameCommand = new GitBlameCommand("randomcmdthatwillneverbefound"); - assertThat(blameCommand.isEnabled(baseDir)).isFalse(); + assertThat(blameCommand.isEnabled()).isFalse(); } @Test - public void return_empty_if_command_fails() throws IOException { + public void throw_exception_if_command_fails() throws Exception { Path baseDir = temp.newFolder().toPath(); GitBlameCommand blameCommand = new GitBlameCommand("randomcmdthatwillneverbefound"); - assertThat(blameCommand.blame(baseDir, "file")).isEmpty(); + assertThatThrownBy(() -> blameCommand.blame(baseDir, "file")).isInstanceOf(IOException.class); } @Test - public void blame_without_email_doesnt_fail() throws IOException, GitAPIException { + public void blame_without_email_doesnt_fail() throws Exception { Path baseDir = temp.newFolder().toPath(); Git git = createRepository(baseDir); String filePath = "file.txt"; diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitThreadFactoryTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitThreadFactoryTest.java index b1793cae2ac..b14d97bf58d 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitThreadFactoryTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitThreadFactoryTest.java @@ -19,7 +19,6 @@ */ package org.sonar.scm.git; -import java.util.concurrent.ForkJoinPool; import org.junit.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -28,9 +27,9 @@ public class GitThreadFactoryTest { @Test public void testName() { GitThreadFactory factory = new GitThreadFactory(); - ForkJoinPool pool = new ForkJoinPool(); - assertThat(factory.newThread(pool).getName()).isEqualTo("git-scm-0"); - assertThat(factory.newThread(pool).getName()).isEqualTo("git-scm-1"); - + assertThat(factory.newThread(() -> { + }).getName()).isEqualTo("git-scm-0"); + assertThat(factory.newThread(() -> { + }).getName()).isEqualTo("git-scm-1"); } } |