From: Julien HENRY Date: Tue, 16 Oct 2018 12:49:12 +0000 (+0200) Subject: SONAR-11365 Fix PR on PR X-Git-Tag: 7.5~276 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=e481af9138fa10ce01b0dfe9f7b0bd961f8408d7;p=sonarqube.git SONAR-11365 Fix PR on PR --- diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java index 5a60eeb5290..c70841cd4e8 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java @@ -56,7 +56,8 @@ public class ChangedLinesPublisher implements ReportPublisherStep { } @Override public void publish(ScannerReportWriter writer) { - if (scmConfiguration.isDisabled() || !branchConfiguration.isShortOrPullRequest() || branchConfiguration.branchTarget() == null) { + String targetScmBranch = branchConfiguration.targetScmBranch(); + if (scmConfiguration.isDisabled() || !branchConfiguration.isShortOrPullRequest() || targetScmBranch == null) { return; } @@ -66,22 +67,18 @@ public class ChangedLinesPublisher implements ReportPublisherStep { } Profiler profiler = Profiler.create(LOG).startInfo(LOG_MSG); - int count = writeChangedLines(provider, writer); + int count = writeChangedLines(provider, writer, targetScmBranch); LOG.debug("SCM reported changed lines for {} {} in the branch", count, ScannerUtils.pluralize("file", count)); profiler.stopInfo(); } - private int writeChangedLines(ScmProvider provider, ScannerReportWriter writer) { - String targetBranchName = branchConfiguration.branchTarget(); - if (targetBranchName == null) { - return 0; - } + private int writeChangedLines(ScmProvider provider, ScannerReportWriter writer, String targetScmBranch) { Path rootBaseDir = inputModuleHierarchy.root().getBaseDir(); Map changedFiles = StreamSupport.stream(inputComponentStore.allChangedFilesToPublish().spliterator(), false) .collect(Collectors.toMap(DefaultInputFile::path, f -> f)); - Map> pathSetMap = provider.branchChangedLines(targetBranchName, rootBaseDir, changedFiles.keySet()); + Map> pathSetMap = provider.branchChangedLines(targetScmBranch, rootBaseDir, changedFiles.keySet()); int count = 0; if (pathSetMap == null) { diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/MetadataPublisher.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/MetadataPublisher.java index 174ea26d78a..43c23eabfe4 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/MetadataPublisher.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/MetadataPublisher.java @@ -134,9 +134,9 @@ public class MetadataPublisher implements ReportPublisherStep { builder.setBranchName(branchConfiguration.branchName()); BranchType branchType = toProtobufBranchType(branchConfiguration.branchType()); builder.setBranchType(branchType); - String branchTarget = branchConfiguration.branchTarget(); - if (branchTarget != null) { - builder.setMergeBranchName(branchTarget); + String referenceBranch = branchConfiguration.longLivingSonarReferenceBranch(); + if (referenceBranch != null) { + builder.setMergeBranchName(referenceBranch); } if (branchType == BranchType.PULL_REQUEST) { builder.setPullRequestKey(branchConfiguration.pullRequestKey()); diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositoriesProvider.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositoriesProvider.java index 4d5cc21c467..fb993d4b344 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositoriesProvider.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositoriesProvider.java @@ -36,7 +36,7 @@ public class ProjectRepositoriesProvider extends ProviderAdapter { if (project == null) { boolean isIssuesMode = mode.isIssues(); Profiler profiler = Profiler.create(LOG).startInfo(LOG_MSG); - project = loader.load(projectKey.get(), isIssuesMode, branchConfig.branchBase()); + project = loader.load(projectKey.get(), isIssuesMode, branchConfig.longLivingSonarReferenceBranch()); checkProject(isIssuesMode); profiler.stopInfo(); } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectScanContainer.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectScanContainer.java index e3e313974fc..d3eecc84024 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectScanContainer.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectScanContainer.java @@ -279,7 +279,7 @@ public class ProjectScanContainer extends ComponentContainer { BranchConfiguration branchConfig = getComponentByType(BranchConfiguration.class); if (branchConfig.branchType() == BranchType.PULL_REQUEST) { - LOG.info("Pull request {} for merge into {} from {}", branchConfig.pullRequestKey(), pullRequestBaseToDisplayName(branchConfig.branchBase()), branchConfig.branchName()); + LOG.info("Pull request {} for merge into {} from {}", branchConfig.pullRequestKey(), pullRequestBaseToDisplayName(branchConfig.targetScmBranch()), branchConfig.branchName()); } else if (branchConfig.branchName() != null) { LOG.info("Branch name: {}, type: {}", branchConfig.branchName(), branchTypeToDisplayName(branchConfig.branchType())); } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/BranchConfiguration.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/BranchConfiguration.java index 695c3c6d62d..783b9abb21c 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/BranchConfiguration.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/BranchConfiguration.java @@ -41,27 +41,32 @@ public interface BranchConfiguration { } /** - * The name of the branch. + * For long/short living branches, this is the value of sonar.branch.name, and fallback on the default branch name configured in SQ + * For PR: the name of the branch containing PR changes (sonar.pullrequest.branch) + * Only @null if the branch feature is not available. */ @CheckForNull String branchName(); /** - * The name of the target branch to merge into. + * The long living server branch from which we should load project settings/quality profiles/compare changed files/... + * For long living branches, this is the sonar.branch.target (default to default branch) in case of first analysis, + * otherwise it's the branch itself. + * For short living branches, we look at sonar.branch.target (default to default branch). If it exists but is a short living branch or PR, we will + * transitively use its own target. + * For PR, we look at sonar.pullrequest.base (default to default branch). If it exists but is a short living branch or PR, we will + * transitively use its own target. + * Only @null if the branch feature is not available. */ @CheckForNull - String branchTarget(); + String longLivingSonarReferenceBranch(); /** - * The name of the base branch to determine project repository and changed files. - * - * Note: this is important for the scanner during the analysis of long living branches. - * For short living branches, branchBase is always the same as branchTarget. - * For long living branches, branchBase is the target in case of first analysis, - * otherwise it's the branch itself. + * Raw value of sonar.branch.target or sonar.pullrequest.base (fallback to the default branch), will be used by the SCM to compute changed files and changed lines. + * @null for long living branches and if the branch feature is not available */ @CheckForNull - String branchBase(); + String targetScmBranch(); /** * The key of the pull request. @@ -69,4 +74,5 @@ public interface BranchConfiguration { * @throws IllegalStateException if this branch configuration is not a pull request. */ String pullRequestKey(); + } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/DefaultBranchConfiguration.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/DefaultBranchConfiguration.java index 2e048e6c689..93dd2652b32 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/DefaultBranchConfiguration.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/DefaultBranchConfiguration.java @@ -37,13 +37,13 @@ public class DefaultBranchConfiguration implements BranchConfiguration { @CheckForNull @Override - public String branchTarget() { + public String targetScmBranch() { return null; } @CheckForNull @Override - public String branchBase() { + public String longLivingSonarReferenceBranch() { return null; } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scm/ScmChangedFilesProvider.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scm/ScmChangedFilesProvider.java index af13e28575c..df8710e4973 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scm/ScmChangedFilesProvider.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scm/ScmChangedFilesProvider.java @@ -63,11 +63,11 @@ public class ScmChangedFilesProvider extends ProviderAdapter { @CheckForNull private static Collection loadChangedFilesIfNeeded(ScmConfiguration scmConfiguration, BranchConfiguration branchConfiguration, Path rootBaseDir) { - if (branchConfiguration.isShortOrPullRequest() && branchConfiguration.branchTarget() != null) { + if (branchConfiguration.isShortOrPullRequest() && branchConfiguration.targetScmBranch() != null) { ScmProvider scmProvider = scmConfiguration.provider(); if (scmProvider != null) { Profiler profiler = Profiler.create(LOG).startInfo(LOG_MSG); - Collection changedFiles = scmProvider.branchChangedFiles(branchConfiguration.branchTarget(), rootBaseDir); + Collection changedFiles = scmProvider.branchChangedFiles(branchConfiguration.targetScmBranch(), rootBaseDir); profiler.stopInfo(); if (changedFiles != null) { LOG.debug("SCM reported {} {} changed in the branch", changedFiles.size(), ScannerUtils.pluralize("file", changedFiles.size())); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/ScannerMediumTester.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/ScannerMediumTester.java index bae2b4a81f7..db8b4fec76f 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/ScannerMediumTester.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/ScannerMediumTester.java @@ -52,11 +52,9 @@ import org.sonar.api.utils.DateUtils; import org.sonar.batch.bootstrapper.Batch; import org.sonar.batch.bootstrapper.EnvironmentInformation; import org.sonar.batch.bootstrapper.LogOutput; -import org.sonar.core.config.ScannerProperties; import org.sonar.scanner.bootstrap.GlobalAnalysisMode; import org.sonar.scanner.issue.tracking.ServerLineHashesLoader; import org.sonar.scanner.protocol.input.ScannerInput.ServerIssue; -import org.sonar.scanner.report.ReportPublisher; import org.sonar.scanner.repository.FileData; import org.sonar.scanner.repository.MetricsRepository; import org.sonar.scanner.repository.MetricsRepositoryLoader; @@ -397,7 +395,7 @@ public class ScannerMediumTester extends ExternalResource { private BranchType branchType = BranchType.LONG; private String branchName = null; private String branchTarget = null; - private String branchBase = null; + private String longLivingSonarReferenceBranch = null; @Override public BranchType branchType() { @@ -412,14 +410,14 @@ public class ScannerMediumTester extends ExternalResource { @CheckForNull @Override - public String branchTarget() { + public String targetScmBranch() { return branchTarget; } @CheckForNull @Override - public String branchBase() { - return branchBase; + public String longLivingSonarReferenceBranch() { + return longLivingSonarReferenceBranch; } @Override @@ -443,6 +441,11 @@ public class ScannerMediumTester extends ExternalResource { return this; } + public ScannerMediumTester setLongLivingSonarReferenceBranch(String longLivingSonarReferenceBranch) { + this.branchConfiguration.longLivingSonarReferenceBranch = longLivingSonarReferenceBranch; + return this; + } + private class FakeBranchConfigurationLoader implements BranchConfigurationLoader { @Override public BranchConfiguration load(Map localSettings, Supplier> settingsSupplier, ProjectBranches branches, ProjectPullRequests pullRequests) { diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/branch/BranchMediumTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/branch/BranchMediumTest.java index efc99a743f8..9a6f06a1f67 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/branch/BranchMediumTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/branch/BranchMediumTest.java @@ -95,6 +95,7 @@ public class BranchMediumTest { TaskResult result = getResult(tester .setBranchName(branchName) .setBranchTarget(branchTarget) + .setLongLivingSonarReferenceBranch(branchTarget) .setBranchType(BranchType.SHORT)); ScannerReport.Metadata metadata = result.getReportReader().readMetadata(); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java index 6bb328a0222..05015007559 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java @@ -68,7 +68,7 @@ public class ChangedLinesPublisherTest { when(branchConfiguration.isShortOrPullRequest()).thenReturn(true); when(scmConfiguration.isDisabled()).thenReturn(false); when(scmConfiguration.provider()).thenReturn(provider); - when(branchConfiguration.branchTarget()).thenReturn(TARGET_BRANCH); + when(branchConfiguration.targetScmBranch()).thenReturn(TARGET_BRANCH); DefaultInputModule root = mock(DefaultInputModule.class); when(root.getBaseDir()).thenReturn(BASE_DIR); when(inputModuleHierarchy.root()).thenReturn(root); @@ -92,7 +92,7 @@ public class ChangedLinesPublisherTest { @Test public void skip_if_target_branch_is_null() { - when(branchConfiguration.branchTarget()).thenReturn(null); + when(branchConfiguration.targetScmBranch()).thenReturn(null); publisher.publish(writer); verifyZeroInteractions(inputComponentStore, inputModuleHierarchy, provider); assertNotPublished(); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/MetadataPublisherTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/MetadataPublisherTest.java index 46290e6a4bf..e0d73485656 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/MetadataPublisherTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/MetadataPublisherTest.java @@ -36,8 +36,6 @@ import org.sonar.api.batch.fs.internal.DefaultInputModule; import org.sonar.api.batch.fs.internal.InputModuleHierarchy; import org.sonar.api.batch.fs.internal.TestInputFileBuilder; import org.sonar.api.batch.scm.ScmProvider; -import org.sonar.api.config.internal.MapSettings; -import org.sonar.core.config.ScannerProperties; import org.sonar.scanner.ProjectAnalysisInfo; import org.sonar.scanner.bootstrap.ScannerPlugin; import org.sonar.scanner.bootstrap.ScannerPluginRepository; @@ -187,7 +185,7 @@ public class MetadataPublisherTest { String branchName = "feature"; String branchTarget = "short-lived"; when(branches.branchName()).thenReturn(branchName); - when(branches.branchTarget()).thenReturn(branchTarget); + when(branches.longLivingSonarReferenceBranch()).thenReturn(branchTarget); File outputDir = temp.newFolder(); underTest.publish(new ScannerReportWriter(outputDir)); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/branch/BranchConfigurationProviderTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/branch/BranchConfigurationProviderTest.java index ba55c21b7c6..a3f1db882b3 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/branch/BranchConfigurationProviderTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/branch/BranchConfigurationProviderTest.java @@ -98,7 +98,7 @@ public class BranchConfigurationProviderTest { public void should_return_default_if_no_loader() { BranchConfiguration result = provider.provide(null, globalConfiguration, reactor, settingsLoader, branches, pullRequests); - assertThat(result.branchTarget()).isNull(); + assertThat(result.targetScmBranch()).isNull(); assertThat(result.branchType()).isEqualTo(BranchType.LONG); } } diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scm/ScmChangedFilesProviderTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scm/ScmChangedFilesProviderTest.java index bc212aa9489..170a4fd5b78 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scm/ScmChangedFilesProviderTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scm/ScmChangedFilesProviderTest.java @@ -67,7 +67,7 @@ public class ScmChangedFilesProviderTest { @Test public void testNoScmProvider() { when(branchConfiguration.isShortOrPullRequest()).thenReturn(true); - when(branchConfiguration.branchTarget()).thenReturn("target"); + when(branchConfiguration.targetScmBranch()).thenReturn("target"); ScmChangedFiles scmChangedFiles = provider.provide(scmConfiguration, branchConfiguration, inputModuleHierarchy); @@ -77,7 +77,7 @@ public class ScmChangedFilesProviderTest { @Test public void testFailIfRelativePath() { - when(branchConfiguration.branchTarget()).thenReturn("target"); + when(branchConfiguration.targetScmBranch()).thenReturn("target"); when(branchConfiguration.isShortOrPullRequest()).thenReturn(true); when(scmConfiguration.provider()).thenReturn(scmProvider); when(scmProvider.branchChangedFiles("target", rootBaseDir)).thenReturn(Collections.singleton(Paths.get("changedFile"))); @@ -89,7 +89,7 @@ public class ScmChangedFilesProviderTest { @Test public void testProviderDoesntSupport() { - when(branchConfiguration.branchTarget()).thenReturn("target"); + when(branchConfiguration.targetScmBranch()).thenReturn("target"); when(branchConfiguration.isShortOrPullRequest()).thenReturn(true); when(scmConfiguration.provider()).thenReturn(scmProvider); when(scmProvider.branchChangedFiles("target", rootBaseDir)).thenReturn(null); @@ -119,7 +119,7 @@ public class ScmChangedFilesProviderTest { when(scmConfiguration.provider()).thenReturn(legacy); when(branchConfiguration.isShortOrPullRequest()).thenReturn(true); - when(branchConfiguration.branchTarget()).thenReturn("target"); + when(branchConfiguration.targetScmBranch()).thenReturn("target"); ScmChangedFiles scmChangedFiles = provider.provide(scmConfiguration, branchConfiguration, inputModuleHierarchy); @@ -129,7 +129,7 @@ public class ScmChangedFilesProviderTest { @Test public void testReturnChangedFiles() { - when(branchConfiguration.branchTarget()).thenReturn("target"); + when(branchConfiguration.targetScmBranch()).thenReturn("target"); when(branchConfiguration.isShortOrPullRequest()).thenReturn(true); when(scmConfiguration.provider()).thenReturn(scmProvider); when(scmProvider.branchChangedFiles("target", rootBaseDir)).thenReturn(Collections.singleton(Paths.get("changedFile").toAbsolutePath()));