Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log a warning during extension build if two sources of scm information disagree #31862

Merged
merged 1 commit into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
}

}