diff options
7 files changed, 229 insertions, 63 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 762e20b65d0..a39b6401767 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 @@ -71,7 +71,7 @@ public class CompositeBlameCommand extends BlameCommand { profiler.startDebug("Collecting committed files"); Set<String> committedFiles = collectAllCommittedFiles(repo); profiler.stopDebug(); - nativeGitEnabled = nativeCmd.isEnabled(); + nativeGitEnabled = nativeCmd.checkIfEnabled(); ExecutorService executorService = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors(), new GitThreadFactory()); for (InputFile inputFile : input.filesToBlame()) { 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 06a216d51ab..10733386574 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 @@ -19,53 +19,60 @@ */ package org.sonar.scm.git; -import java.io.InputStream; -import java.io.InputStreamReader; +import java.io.IOException; import java.nio.file.Path; import java.time.Instant; import java.util.Date; import java.util.LinkedList; import java.util.List; -import java.util.Scanner; -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.System2; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.springframework.beans.factory.annotation.Autowired; -import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.emptyList; import static org.sonar.api.utils.Preconditions.checkState; public class GitBlameCommand { private static final Logger LOG = Loggers.get(GitBlameCommand.class); private static final Pattern EMAIL_PATTERN = Pattern.compile("<(\\S*?)>"); - private static final String COMITTER_TIME = "committer-time "; - private static final String COMITTER_MAIL = "committer-mail "; + private static final String COMMITTER_TIME = "committer-time "; + private static final String COMMITTER_MAIL = "committer-mail "; - private static final String GIT_COMMAND = "git"; + private static final String DEFAULT_GIT_COMMAND = "git"; private static final String BLAME_COMMAND = "blame"; private static final String BLAME_LINE_PORCELAIN_FLAG = "--line-porcelain"; private static final String IGNORE_WHITESPACES = "-w"; - private final String gitCommand; + private final System2 system; + private final ProcessWrapperFactory processWrapperFactory; + private String gitCommand; @Autowired - public GitBlameCommand() { - this(GIT_COMMAND); + public GitBlameCommand(System2 system, ProcessWrapperFactory processWrapperFactory) { + this.system = system; + this.processWrapperFactory = processWrapperFactory; } - public GitBlameCommand(String gitCommand) { + GitBlameCommand(String gitCommand, System2 system, ProcessWrapperFactory processWrapperFactory) { this.gitCommand = gitCommand; + this.system = system; + this.processWrapperFactory = processWrapperFactory; } - public boolean isEnabled() { + /** + * This method must be executed before org.sonar.scm.git.GitBlameCommand#blame + * + * @return true, if native git is installed + */ + public boolean checkIfEnabled() { try { + this.gitCommand = locateDefaultGit(); MutableString stdOut = new MutableString(); - executeCommand(null, l -> stdOut.string = l, gitCommand, "--version"); + this.processWrapperFactory.create(null, l -> stdOut.string = l, gitCommand, "--version").execute(); return stdOut.string != null && stdOut.string.startsWith("git version"); } catch (Exception e) { LOG.debug("Failed to find git native client", e); @@ -73,10 +80,38 @@ public class GitBlameCommand { } } + private String locateDefaultGit() throws IOException { + if (this.gitCommand != null) { + return this.gitCommand; + } + // if not set fall back to defaults + if (system.isOsWindows()) { + return locateGitOnWindows(); + } + return DEFAULT_GIT_COMMAND; + } + + private String locateGitOnWindows() throws IOException { + // Windows will search current directory in addition to the PATH variable, which is unsecure. + // To avoid it we use where.exe to find git binary only in PATH. + LOG.debug("Looking for git command in the PATH using where.exe (Windows)"); + List<String> whereCommandResult = new LinkedList<>(); + this.processWrapperFactory.create(null, whereCommandResult::add, "C:\\Windows\\System32\\where.exe", "$PATH:git.exe") + .execute(); + + if (!whereCommandResult.isEmpty()) { + String out = whereCommandResult.get(0).trim(); + LOG.debug("Found git.exe at {}", out); + return out; + } + throw new IllegalStateException("git.exe not found in PATH. PATH value was: " + system.property("PATH")); + } + 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); + this.processWrapperFactory.create(baseDir, outputProcessor::process, gitCommand, BLAME_COMMAND, BLAME_LINE_PORCELAIN_FLAG, IGNORE_WHITESPACES, fileName) + .execute(); } catch (UncommittedLineException e) { LOG.debug("Unable to blame file '{}' - it has uncommitted changes", fileName); return emptyList(); @@ -84,31 +119,6 @@ public class GitBlameCommand { return outputProcessor.getBlameLines(); } - private static void executeCommand(@Nullable Path baseDir, Consumer<String> stdOutLineConsumer, String... command) throws Exception { - ProcessBuilder pb = new ProcessBuilder() - .command(command) - .directory(baseDir != null ? baseDir.toFile() : null); - - Process p = pb.start(); - try { - InputStream processStdOutput = p.getInputStream(); - // don't use BufferedReader#readLine because it will also parse CR, which may be part of the actual source code line - try (Scanner scanner = new Scanner(new InputStreamReader(processStdOutput, UTF_8))) { - scanner.useDelimiter("\n"); - while (scanner.hasNext()) { - stdOutLineConsumer.accept(scanner.next()); - } - } - - int exit = p.waitFor(); - 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 LinkedList<>(); private String sha1 = null; @@ -124,11 +134,11 @@ public class GitBlameCommand { sha1 = line.split(" ")[0]; } else if (line.startsWith("\t")) { saveEntry(); - } else if (line.startsWith(COMITTER_TIME)) { - committerTime = line.substring(COMITTER_TIME.length()); - } else if (line.startsWith(COMITTER_MAIL)) { + } else if (line.startsWith(COMMITTER_TIME)) { + committerTime = line.substring(COMMITTER_TIME.length()); + } else if (line.startsWith(COMMITTER_MAIL)) { Matcher matcher = EMAIL_PATTERN.matcher(line); - if (!matcher.find(COMITTER_MAIL.length()) || matcher.groupCount() != 1) { + if (!matcher.find(COMMITTER_MAIL.length()) || matcher.groupCount() != 1) { throw new IllegalStateException("Couldn't parse committer email from: " + line); } committerMail = matcher.group(1); diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitScmSupport.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitScmSupport.java index a67bd56320d..65a8044dfbe 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitScmSupport.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitScmSupport.java @@ -33,6 +33,7 @@ public final class GitScmSupport { return Arrays.asList( JGitBlameCommand.class, CompositeBlameCommand.class, + ProcessWrapperFactory.class, GitBlameCommand.class, GitScmProvider.class, GitIgnoreCommand.class); diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/ProcessWrapperFactory.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/ProcessWrapperFactory.java new file mode 100644 index 00000000000..6d9c602d1c2 --- /dev/null +++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/ProcessWrapperFactory.java @@ -0,0 +1,88 @@ +/* + * SonarQube + * Copyright (C) 2009-2022 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.scm.git; + +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.file.Path; +import java.util.Scanner; +import java.util.function.Consumer; +import javax.annotation.Nullable; +import org.sonar.api.utils.log.Logger; +import org.sonar.api.utils.log.Loggers; + +import static java.lang.String.format; +import static java.lang.String.join; +import static java.nio.charset.StandardCharsets.UTF_8; + +public class ProcessWrapperFactory { + private static final Logger LOG = Loggers.get(ProcessWrapperFactory.class); + + public ProcessWrapperFactory() { + // nothing to do + } + + public ProcessWrapper create(@Nullable Path baseDir, Consumer<String> stdOutLineConsumer, String... command) { + return new ProcessWrapper(baseDir, stdOutLineConsumer, command); + } + + static class ProcessWrapper { + + private final Path baseDir; + private final Consumer<String> stdOutLineConsumer; + private final String[] command; + + ProcessWrapper(@Nullable Path baseDir, Consumer<String> stdOutLineConsumer, String... command) { + this.baseDir = baseDir; + this.stdOutLineConsumer = stdOutLineConsumer; + this.command = command; + } + + public void execute() throws IOException { + ProcessBuilder pb = new ProcessBuilder() + .command(command) + .directory(baseDir != null ? baseDir.toFile() : null); + + Process p = pb.start(); + try { + InputStream processStdOutput = p.getInputStream(); + // don't use BufferedReader#readLine because it will also parse CR, which may be part of the actual source code line + try (Scanner scanner = new Scanner(new InputStreamReader(processStdOutput, UTF_8))) { + scanner.useDelimiter("\n"); + while (scanner.hasNext()) { + stdOutLineConsumer.accept(scanner.next()); + } + } + + int exit = p.waitFor(); + if (exit != 0) { + throw new IllegalStateException(format("Command execution exited with code: %d", exit)); + } + } catch (InterruptedException e) { + LOG.warn(format("Command [%s] interrupted", join(" ", command)), e); + Thread.currentThread().interrupt(); + } finally { + p.destroy(); + } + } + } + +} 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 0604b456b93..5ed16c963de 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 @@ -61,8 +61,9 @@ import static org.sonar.scm.git.Utils.javaUnzip; public class CompositeBlameCommandTest { private static final String DUMMY_JAVA = "src/main/java/org/dummy/Dummy.java"; private final PathResolver pathResolver = new PathResolver(); + private final ProcessWrapperFactory processWrapperFactory = new ProcessWrapperFactory(); private final JGitBlameCommand jGitBlameCommand = new JGitBlameCommand(); - private final GitBlameCommand gitBlameCommand = new GitBlameCommand(); + private final GitBlameCommand gitBlameCommand = new GitBlameCommand(System2.INSTANCE, processWrapperFactory); private final AnalysisWarnings analysisWarnings = mock(AnalysisWarnings.class); private final CompositeBlameCommand blameCommand = new CompositeBlameCommand(analysisWarnings, pathResolver, jGitBlameCommand, gitBlameCommand); @@ -75,7 +76,7 @@ public class CompositeBlameCommandTest { @Test public void use_jgit_if_native_git_disabled() throws IOException { - GitBlameCommand gitCmd = new GitBlameCommand("invalidcommandnotfound"); + GitBlameCommand gitCmd = new GitBlameCommand("invalidcommandnotfound", System2.INSTANCE, processWrapperFactory); BlameCommand blameCmd = new CompositeBlameCommand(analysisWarnings, pathResolver, jGitBlameCommand, gitCmd); File projectDir = createNewTempFolder(); javaUnzip("dummy-git.zip", projectDir); @@ -96,7 +97,7 @@ public class CompositeBlameCommandTest { javaUnzip("dummy-git.zip", projectDir); File baseDir = new File(projectDir, "dummy-git"); - when(gitCmd.isEnabled()).thenReturn(true); + when(gitCmd.checkIfEnabled()).thenReturn(true); when(gitCmd.blame(baseDir.toPath(), DUMMY_JAVA)).thenThrow(new IllegalStateException()); setUpBlameInputWithFile(baseDir.toPath()); TestBlameOutput output = new TestBlameOutput(); @@ -112,7 +113,7 @@ public class CompositeBlameCommandTest { @Test public void skip_files_not_committed() throws Exception { // skip if git not installed - assumeTrue(gitBlameCommand.isEnabled()); + assumeTrue(gitBlameCommand.checkIfEnabled()); JGitBlameCommand jgit = mock(JGitBlameCommand.class); BlameCommand blameCmd = new CompositeBlameCommand(analysisWarnings, pathResolver, jgit, gitBlameCommand); @@ -133,7 +134,7 @@ public class CompositeBlameCommandTest { @Test public void use_native_git_by_default() throws IOException { // skip test if git is not installed - assumeTrue(gitBlameCommand.isEnabled()); + assumeTrue(gitBlameCommand.checkIfEnabled()); File projectDir = createNewTempFolder(); javaUnzip("dummy-git.zip", projectDir); File baseDir = new File(projectDir, "dummy-git"); @@ -299,14 +300,15 @@ public class CompositeBlameCommandTest { } private File createNewTempFolder() throws IOException { - //This is needed for Windows, otherwise the created File point to invalid (shortened by Windows) temp folder path + // This is needed for Windows, otherwise the created File point to invalid (shortened by Windows) temp folder path return temp.newFolder().toPath().toRealPath(LinkOption.NOFOLLOW_LINKS).toFile(); } private static class TestBlameOutput implements BlameCommand.BlameOutput { private final Map<InputFile, List<BlameLine>> blame = new LinkedHashMap<>(); - @Override public void blameResult(InputFile inputFile, List<BlameLine> list) { + @Override + public void blameResult(InputFile inputFile, List<BlameLine> list) { blame.put(inputFile, list); } } 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 1ea15883137..328383c235c 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 @@ -27,6 +27,7 @@ import java.nio.file.Path; import java.util.Date; import java.util.LinkedList; import java.util.List; +import java.util.function.Consumer; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.junit.Before; @@ -37,10 +38,16 @@ import org.sonar.api.batch.scm.BlameLine; import org.sonar.api.utils.DateUtils; import org.sonar.api.utils.System2; import org.sonar.api.utils.log.LogTester; +import org.sonar.scm.git.ProcessWrapperFactory.ProcessWrapper; 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.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static org.sonar.scm.git.GitUtils.createFile; import static org.sonar.scm.git.GitUtils.createRepository; import static org.sonar.scm.git.Utils.javaUnzip; @@ -52,11 +59,12 @@ public class GitBlameCommandTest { public TemporaryFolder temp = new TemporaryFolder(); @Rule public LogTester logTester = new LogTester(); - private final GitBlameCommand blameCommand = new GitBlameCommand(); + private final ProcessWrapperFactory processWrapperFactory = new ProcessWrapperFactory(); + private final GitBlameCommand blameCommand = new GitBlameCommand(System2.INSTANCE, processWrapperFactory); @Before public void skipTestsIfNoGitFound() { - assumeTrue(blameCommand.isEnabled()); + assumeTrue(blameCommand.checkIfEnabled()); } @Test @@ -134,20 +142,20 @@ public class GitBlameCommandTest { @Test public void git_should_be_detected() { - GitBlameCommand blameCommand = new GitBlameCommand(); - assertThat(blameCommand.isEnabled()).isTrue(); + GitBlameCommand blameCommand = new GitBlameCommand(System2.INSTANCE, processWrapperFactory); + assertThat(blameCommand.checkIfEnabled()).isTrue(); } @Test public void git_should_not_be_detected() { - GitBlameCommand blameCommand = new GitBlameCommand("randomcmdthatwillneverbefound"); - assertThat(blameCommand.isEnabled()).isFalse(); + GitBlameCommand blameCommand = new GitBlameCommand("randomcmdthatwillneverbefound", System2.INSTANCE, processWrapperFactory); + assertThat(blameCommand.checkIfEnabled()).isFalse(); } @Test public void throw_exception_if_command_fails() throws Exception { Path baseDir = temp.newFolder().toPath(); - GitBlameCommand blameCommand = new GitBlameCommand("randomcmdthatwillneverbefound"); + GitBlameCommand blameCommand = new GitBlameCommand("randomcmdthatwillneverbefound", System2.INSTANCE, processWrapperFactory); assertThatThrownBy(() -> blameCommand.blame(baseDir, "file")).isInstanceOf(IOException.class); } @@ -159,7 +167,8 @@ public class GitBlameCommandTest { createFile(filePath, "line", baseDir); commitWithNoEmail(git, filePath); - GitBlameCommand blameCommand = new GitBlameCommand(); + GitBlameCommand blameCommand = new GitBlameCommand(System2.INSTANCE, processWrapperFactory); + assertThat(blameCommand.checkIfEnabled()).isTrue(); List<BlameLine> blame = blameCommand.blame(baseDir, filePath); assertThat(blame).hasSize(1); BlameLine blameLine = blame.get(0); @@ -168,6 +177,62 @@ public class GitBlameCommandTest { assertThat(blameLine.date()).isNotNull(); } + @Test + public void do_not_execute() throws Exception { + Path baseDir = temp.newFolder().toPath(); + Git git = createRepository(baseDir); + String filePath = "file.txt"; + createFile(filePath, "line", baseDir); + commitWithNoEmail(git, filePath); + + GitBlameCommand blameCommand = new GitBlameCommand(System2.INSTANCE, processWrapperFactory); + assertThat(blameCommand.checkIfEnabled()).isTrue(); + List<BlameLine> blame = blameCommand.blame(baseDir, filePath); + assertThat(blame).hasSize(1); + BlameLine blameLine = blame.get(0); + assertThat(blameLine.author()).isNull(); + assertThat(blameLine.revision()).isNotNull(); + assertThat(blameLine.date()).isNotNull(); + } + + @Test + public void execution_on_windows_should_fallback_to_full_path() { + System2 system2 = mock(System2.class); + when(system2.isOsWindows()).thenReturn(true); + + ProcessWrapperFactory mockFactory = mock(ProcessWrapperFactory.class); + ProcessWrapper mockProcess = mock(ProcessWrapper.class); + when(mockFactory.create(isNull(), any(), eq("C:\\Windows\\System32\\where.exe"), eq("$PATH:git.exe"))).then(invocation -> { + var argument = (Consumer<String>) invocation.getArgument(1); + argument.accept("C:\\mockGit.exe"); + return mockProcess; + }); + + when(mockFactory.create(isNull(), any(), eq("C:\\mockGit.exe"), eq("--version"))).then(invocation -> { + var argument = (Consumer<String>) invocation.getArgument(1); + argument.accept("git version 2.30.1"); + return mockProcess; + }); + + GitBlameCommand blameCommand = new GitBlameCommand(system2, mockFactory); + assertThat(blameCommand.checkIfEnabled()).isTrue(); + assertThat(logTester.logs()).contains("Found git.exe at C:\\mockGit.exe"); + } + + @Test + public void execution_on_windows_is_disabled_if_git_not_on_path() { + System2 system2 = mock(System2.class); + when(system2.isOsWindows()).thenReturn(true); + when(system2.property("PATH")).thenReturn("C:\\some-path;C:\\some-another-path"); + + ProcessWrapperFactory mockFactory = mock(ProcessWrapperFactory.class); + ProcessWrapper mockProcess = mock(ProcessWrapper.class); + when(mockFactory.create(isNull(), any(), eq("C:\\Windows\\System32\\where.exe"), eq("$PATH:git.exe"))).thenReturn(mockProcess); + + GitBlameCommand blameCommand = new GitBlameCommand(system2, mockFactory); + assertThat(blameCommand.checkIfEnabled()).isFalse(); + } + private void commitWithNoEmail(Git git, String path) throws GitAPIException { git.add().addFilepattern(path).call(); git.commit().setCommitter("joe", "").setMessage("msg").call(); @@ -179,7 +244,7 @@ public class GitBlameCommandTest { } private File createNewTempFolder() throws IOException { - //This is needed for Windows, otherwise the created File point to invalid (shortened by Windows) temp folder path + // This is needed for Windows, otherwise the created File point to invalid (shortened by Windows) temp folder path return temp.newFolder().toPath().toRealPath(LinkOption.NOFOLLOW_LINKS).toFile(); } } diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitScmProviderTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitScmProviderTest.java index af58e703ef6..a41f2a508b9 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitScmProviderTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitScmProviderTest.java @@ -138,7 +138,7 @@ public class GitScmProviderTest { @Test public void returnImplem() { JGitBlameCommand jblameCommand = new JGitBlameCommand(); - GitBlameCommand nativeBlameCommand = new GitBlameCommand(); + GitBlameCommand nativeBlameCommand = new GitBlameCommand(System2.INSTANCE, new ProcessWrapperFactory()); CompositeBlameCommand compositeBlameCommand = new CompositeBlameCommand(analysisWarnings, new PathResolver(), jblameCommand, nativeBlameCommand); GitScmProvider gitScmProvider = new GitScmProvider(compositeBlameCommand, analysisWarnings, gitIgnoreCommand, system2); |