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

pr: align handleCommit and handle #926

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -24,6 +24,7 @@

import org.openjdk.skara.census.*;
import org.openjdk.skara.forge.*;
import org.openjdk.skara.vcs.Hash;
import org.openjdk.skara.host.HostUser;
import org.openjdk.skara.jcheck.JCheckConfiguration;

@@ -66,13 +67,21 @@ private static Namespace namespace(Census census, String hostNamespace) {
return namespace;
}

private static JCheckConfiguration configuration(HostedRepositoryPool hostedRepositoryPool, HostedRepository remoteRepo, String name, String ref) throws IOException {
var confFile = hostedRepositoryPool.lines(remoteRepo, Path.of(name), ref).orElseThrow(
() -> new IOException("Failed to read jcheck configuration from " + name + ":" + ref));
private static JCheckConfiguration configuration(HostedRepositoryPool hostedRepositoryPool, HostedRepository remoteRepo, String name, Hash hash) throws IOException {
var confFile = hostedRepositoryPool.lines(remoteRepo, Path.of(name), hash).orElseThrow(
() -> new IOException("Failed to read jcheck configuration from " + name + ":" + hash.hex()));
return JCheckConfiguration.parse(confFile);
}

static CensusInstance create(HostedRepositoryPool hostedRepositoryPool, HostedRepository censusRepo, String censusRef, Path folder, PullRequest pr,
static CensusInstance create(HostedRepositoryPool hostedRepositoryPool,
HostedRepository censusRepo, String censusRef, Path folder, PullRequest pr,
HostedRepository confOverrideRepo, String confOverrideName, String confOverrideRef) {
return create(hostedRepositoryPool, censusRepo, censusRef, folder, pr.repository(), pr.targetHash(),
confOverrideRepo, confOverrideName, confOverrideRef);
}

static CensusInstance create(HostedRepositoryPool hostedRepositoryPool,
HostedRepository censusRepo, String censusRef, Path folder, HostedRepository repository, Hash hash,
HostedRepository confOverrideRepo, String confOverrideName, String confOverrideRef) {
var repoName = censusRepo.url().getHost() + "/" + censusRepo.name();
var repoFolder = folder.resolve(URLEncoder.encode(repoName, StandardCharsets.UTF_8));
@@ -85,13 +94,17 @@ static CensusInstance create(HostedRepositoryPool hostedRepositoryPool, HostedRe
try {
JCheckConfiguration configuration;
if (confOverrideRepo == null) {
configuration = configuration(hostedRepositoryPool, pr.repository(), ".jcheck/conf", pr.targetRef());
configuration = configuration(hostedRepositoryPool, repository, ".jcheck/conf", hash);
} else {
configuration = configuration(hostedRepositoryPool, confOverrideRepo, confOverrideName, confOverrideRef);
var confOverrideHash = confOverrideRepo.branchHash(confOverrideRef);
configuration = configuration(hostedRepositoryPool,
confOverrideRepo,
confOverrideName,
confOverrideHash);
}
var census = Census.parse(repoFolder);
var project = project(configuration, census);
var namespace = namespace(census, pr.repository().namespace());
var namespace = namespace(census, repository.namespace());
return new CensusInstance(census, configuration, project, namespace);
} catch (IOException e) {
throw new UncheckedIOException("Cannot parse census at " + repoFolder, e);
@@ -22,6 +22,7 @@
*/
package org.openjdk.skara.bots.pr;

import org.openjdk.skara.forge.HostedCommit;
import org.openjdk.skara.forge.PullRequest;
import org.openjdk.skara.issuetracker.Comment;
import org.openjdk.skara.vcs.*;
@@ -36,7 +37,7 @@ interface CommandHandler {
default void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInstance, Path scratchPath, CommandInvocation command, List<Comment> allComments, PrintWriter reply)
{
}
default void handleCommit(PullRequestBot bot, Hash hash, Path scratchPath, CommandInvocation command, List<Comment> allComments, PrintWriter reply) {
default void handle(PullRequestBot bot, HostedCommit commit, CensusInstance censusInstance, Path scratchPath, CommandInvocation command, List<Comment> allComments, PrintWriter reply) {
}

default boolean multiLine() {
@@ -72,7 +72,7 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
}

@Override
public void handleCommit(PullRequestBot bot, Hash hash, Path scratchPath, CommandInvocation command, List<Comment> allComments, PrintWriter reply) {
public void handle(PullRequestBot bot, HostedCommit hash, CensusInstance censusInstance, Path scratchPath, CommandInvocation command, List<Comment> allComments, PrintWriter reply) {
reply.println("Available commands:");
Stream.concat(
commandHandlers.entrySet().stream()
@@ -159,7 +159,8 @@ private void processCommand(PullRequest pr, CensusInstance censusInstance, Path
if (handler.get().allowedInCommit()) {
var hash = resultingCommitHash(allComments);
if (hash.isPresent()) {
handler.get().handleCommit(bot, hash.get(), scratchPath, command, allComments, printer);
var commit = pr.repository().commit(hash.get()).orElseThrow();
handler.get().handle(bot, commit, censusInstance, scratchPath, command, allComments, printer);
} else {
printer.print("The command `");
printer.print(command.name());
@@ -49,7 +49,7 @@ public class CommitCommandWorkItem implements WorkItem {

static class HelpCommand implements CommandHandler {
@Override
public void handleCommit(PullRequestBot bot, Hash hash, Path scratchPath, CommandInvocation command, List<Comment> allComments, PrintWriter reply) {
public void handle(PullRequestBot bot, HostedCommit commit, CensusInstance censusInstance, Path scratchPath, CommandInvocation command, List<Comment> allComments, PrintWriter reply) {
reply.println("Available commands:");
Stream.concat(
commandHandlers.entrySet().stream()
@@ -112,7 +112,7 @@ private Optional<CommandInvocation> nextCommand(List<CommitComment> allComments)
}
}

private void processCommand(Path scratchPath, CommandInvocation command, List<CommitComment> allComments) {
private void processCommand(Path scratchPath, HostedCommit commit, CensusInstance censusInstance, CommandInvocation command, List<CommitComment> allComments) {
var writer = new StringWriter();
var printer = new PrintWriter(writer);

@@ -124,7 +124,7 @@ private void processCommand(Path scratchPath, CommandInvocation command, List<Co
var comments = allComments.stream()
.map(cc -> (Comment)cc)
.collect(Collectors.toList());
handler.get().handleCommit(bot, commitComment.commit(), scratchPath, command, comments, printer);
handler.get().handle(bot, commit, censusInstance, scratchPath, command, comments, printer);
} else {
printer.print("The command `");
printer.print(command.name());
@@ -142,13 +142,22 @@ private void processCommand(Path scratchPath, CommandInvocation command, List<Co
public Collection<WorkItem> run(Path scratchPath) {
log.info("Looking for commit comment commands");

var allComments = bot.repo().commitComments(commitComment.commit());
var commit = bot.repo().commit(commitComment.commit()).orElseThrow(() ->
new IllegalStateException("Commit with hash " + commitComment.commit() + " missing"));
var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds"));
var hostedRepositoryPool = new HostedRepositoryPool(seedPath);
var census = CensusInstance.create(hostedRepositoryPool, bot.censusRepo(), bot.censusRef(),
scratchPath.resolve("census"), bot.repo(), commit.hash(),
bot.confOverrideRepository().orElse(null),
bot.confOverrideName(),
bot.confOverrideRef());
var allComments = bot.repo().commitComments(commit.hash());
var nextCommand = nextCommand(allComments);

if (nextCommand.isEmpty()) {
log.info("No new commit comments found, stopping further processing");
} else {
processCommand(scratchPath, nextCommand.get(), allComments);
processCommand(scratchPath, commit, census, nextCommand.get(), allComments);
}

return List.of();
@@ -193,17 +193,13 @@ public Repository checkoutAllowStale(HostedRepository hostedRepository, String r
return checkout(hostedRepository, ref, path, true);
}

public Optional<List<String>> lines(HostedRepository hostedRepository, Path p, String ref) throws IOException {
public Optional<List<String>> lines(HostedRepository hostedRepository, Path p, Hash hash) throws IOException {
var hostedRepositoryInstance = new HostedRepositoryInstance(hostedRepository);
var seedRepo = hostedRepositoryInstance.seedRepository(true);
var refHash = seedRepo.resolve(ref);
if (refHash.isEmpty()) {
if (!seedRepo.contains(hash)) {
// It may fail because the seed is stale - need to refresh it now
seedRepo.fetchAll(hostedRepository.url(), true);
refHash = seedRepo.resolve(ref);
}

var hash = refHash.orElseThrow(() -> new IOException("Ref not found: " + ref));
return seedRepo.lines(p, hash);
}