Skip to content

Commit

Permalink
Merge pull request #31862 from holly-cummins/logging-for-scm-url
Browse files Browse the repository at this point in the history
Log a warning during extension build if two sources of scm information disagree
  • Loading branch information
gsmet authored Mar 16, 2023
2 parents bccd6e5 + 358b38b commit 44a2be0
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ private void computeArtifactCoords(ObjectNode extObject) {
}

private void computeSourceLocation(ObjectNode extObject) {
Map<String, String> repo = ScmInfoProvider.getSourceRepo(null);
Map<String, String> repo = new ScmInfoProvider(null).getSourceRepo();
if (repo != null) {
ObjectNode metadata = getMetadataNode(extObject);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -682,9 +682,11 @@ private void addSource(ObjectNode extObject) throws MojoExecutionException {
Scm scm = getScm();
String scmUrl = scm != null ? scm.getUrl() : null;

Map<String, String> repo = new ScmInfoProvider().getSourceRepo(scmUrl);
ScmInfoProvider scmInfoProvider = new ScmInfoProvider(scmUrl);
Map<String, String> repo = scmInfoProvider.getSourceRepo();
ObjectNode metadata = getMetadataNode(extObject);

if (repo != null) {
ObjectNode metadata = getMetadataNode(extObject);
for (Map.Entry<String, String> e : repo.entrySet()) {
// Ignore if already set
String value = e.getValue();
Expand All @@ -695,6 +697,16 @@ private void addSource(ObjectNode extObject) throws MojoExecutionException {
}

}

String warning = scmInfoProvider.getInconsistencyWarning();
if (warning != null) {
getLog().warn(warning);
}
}
// We had been generic, but go a bit more specific so we can give a sensible message
else if (!metadata.has("scm-url")) {
getLog().debug(
"Could not work out a source control repository from the build environment or build file. Consider adding an scm-url entry in quarkus-extension.yaml");
}
}

Expand All @@ -708,14 +720,15 @@ private Scm getScm() {

if (localProject == null) {
final Log log = getLog();
log.warn("Workspace provider could not resolve local project for " + artifact.getGroupId() + ":"
log.debug("Workspace provider could not resolve local project for " + artifact.getGroupId() + ":"
+ artifact.getArtifactId());
}

while (scm == null && localProject != null) {
scm = localProject.getRawModel().getScm();
localProject = localProject.getLocalParent();
}

return scm;

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,47 @@

public class ScmInfoProvider {

public static Map<String, String> getSourceRepo(String valueFromBuildFile) {
private final String valueFromBuildFile;
private final String valueFromEnvironment;

public ScmInfoProvider(String valueFromBuildFile) {
this.valueFromBuildFile = valueFromBuildFile;
// We could try and parse the .git/config file, but that will be fragile
// We could use JGit, something like https://wiki.eclipse.org/JGit/User_Guide#Repository but it seems a lot for our needs
String repo = System.getenv("GITHUB_REPOSITORY");
Map info = null;
if (repo != null) {
info = new HashMap();
String qualifiedRepo = "https://github.com/" + repo;
// Don't try and guess where slashes will be, just deal with any double slashes by brute force
qualifiedRepo = qualifiedRepo.replace("github.com//", "github.com/");
info.put("url", qualifiedRepo);
this.valueFromEnvironment = qualifiedRepo.replace("github.com//", "github.com/");
} else {
this.valueFromEnvironment = null;
}
}

public Map<String, String> getSourceRepo() {
// We use a map here because we might also add things like github tags
Map<String, String> info = null;
if (valueFromEnvironment != null) {
info = new HashMap<>();
info.put("url", valueFromEnvironment);

} else if (valueFromBuildFile != null) {
info = new HashMap();
info = new HashMap<>();
info.put("url", valueFromBuildFile);
}

return info;
}

public String getInconsistencyWarning() {
if (valueFromEnvironment != null && valueFromBuildFile != null && !valueFromEnvironment.equals(valueFromBuildFile)) {
return "The scm-url coordinates in the build file, " + valueFromBuildFile
+ " did not match the repository configured in the GITHUB_REPOSITORY environment variable, "
+ valueFromEnvironment
+ ". The value which will be used for the extension metadata is "
+ valueFromEnvironment + ".";
} else {
return null;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package io.quarkus.devtools.project.extensions;

import static io.quarkus.devtools.project.extensions.ScmInfoProvider.getSourceRepo;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.Map;

Expand All @@ -28,7 +28,7 @@ public void setUp() {

@Test
public void shouldReturnNullWhenNoEnvironmentOrBuildConfigIsPresent() {
Map scm = getSourceRepo(null);
Map<String, String> scm = new ScmInfoProvider(null).getSourceRepo();
// We shouldn't throw an exception or get upset, we should just quietly return null
assertNull(scm);
}
Expand All @@ -38,29 +38,60 @@ public void shouldReturnNullWhenNoEnvironmentOrBuildConfigIsPresent() {
void testGetSourceControlCoordinatesWhenOnlyEnvironmentIsSet() {
String repoName = "org/place";
environment.set("GITHUB_REPOSITORY", repoName);
Map repo = getSourceRepo(null);
Map<String, String> repo = new ScmInfoProvider(null).getSourceRepo();
assertNotNull(repo);
assertEquals(repo.get("url").toString(), "https://github.com/org/place");
assertEquals(repo.get("url"), "https://github.com/org/place");
}

void testGetWarningWhenOnlyEnvironmentIsSet() {
String repoName = "org/place";
environment.set("GITHUB_REPOSITORY", repoName);
String warning = new ScmInfoProvider(null).getInconsistencyWarning();
assertNull(warning);
}

// Easy case - we just read the info from the pom
@Test
void testGetSourceControlCoordinatesWhenOnlyPomIsSet() {
final String scmUrl = "https://github.com/org/frompom";
Map repo = getSourceRepo(scmUrl);
Map<String, String> repo = new ScmInfoProvider(scmUrl).getSourceRepo();
assertNotNull(repo);
assertEquals(repo.get("url").toString(), scmUrl);
assertEquals(repo.get("url"), scmUrl);
}

void testGetWarningWhenOnlyPomIsSet() {
final String scmUrl = "https://github.com/org/frompom";
String warning = new ScmInfoProvider(scmUrl).getInconsistencyWarning();
assertNull(warning);
}

void testGetWarningWhenEnvironmentAndPomAgree() {
final String scmUrl = "https://github.com/org/place";
String repoName = "org/place";
environment.set("GITHUB_REPOSITORY", repoName);
String warning = new ScmInfoProvider(scmUrl).getInconsistencyWarning();
assertNull(warning);
}

// Case where the pom info conflicts with the environment info; honour the environment info
void testGetSourceControlCoordinatesWhenEnvironmentAndPomDisagree() {
final String scmUrl = "https://github.com/org/frompom";
String repoName = "org/place";
environment.set("GITHUB_REPOSITORY", repoName);
Map repo = getSourceRepo(null);
Map<String, String> repo = new ScmInfoProvider(null).getSourceRepo();
assertNotNull(repo);
// We should honour the environment info; out of the box, all quarkiverse extensions generated by the quarkus tooling will report a scm url of quarkiverse-parent/<project-name> in their model
assertEquals(repo.get("url").toString(), "https://github.com/org/place");
assertEquals(repo.get("url"), "https://github.com/org/place");
}

void testGetWarningWhenEnvironmentAndPomDisagree() {
final String scmUrl = "https://github.com/org/frompom";
String repoName = "org/place";
environment.set("GITHUB_REPOSITORY", repoName);
String warning = new ScmInfoProvider(scmUrl).getInconsistencyWarning();
assertNotNull(warning);
assertTrue(warning.contains(scmUrl), warning);
assertTrue(warning.contains(repoName), warning);
}

}

0 comments on commit 44a2be0

Please sign in to comment.