Skip to content
Snippets Groups Projects
Commit 82999edf authored by Georg Seibt's avatar Georg Seibt :nerd:
Browse files

Remove automatic trimming of the process output. 

Document ExecRes and add methods for getting the trimmed output (that use a lazily initialized field). Use the method for getting the trimmed output everywhere it seems necessary. Only initialize author / committer info in the getCommitInfo() method if they are null.
parent 862c4cac
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 129 additions and 43 deletions
src/de/uni_passau/fim/seibt/gitwrapper/process/ProcessExecutor.java
+ 64
− 13
View file @ 82999edf
package de.uni_passau.fim.seibt.gitwrapper.process;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.logging.Level.FINER;
import static java.util.logging.Level.WARNING;
import java.io.File;
import java.io.IOException;
import java.util.Collections;
......@@ -16,9 +20,6 @@ import java.util.logging.Logger;
import org.apache.commons.io.IOUtils;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.logging.Level.WARNING;
/**
* Contains static methods for executing commands and gathering their output and exit code.
*/
......@@ -37,15 +38,65 @@ public class ProcessExecutor {
* The result of a process execution.
*/
public static final class ExecRes {
/**
* The exit code of the process that produced this {@link ExecRes}.
*/
public final int exitCode;
/**
* The full standard output of the process that produced this {@link ExecRes}.
*/
public final String stdOut;
/**
* The full standard error output of the process that produced this {@link ExecRes}.
*/
public final String stdErr;
private String stdOutTrimmed;
private String stdErrTrimmed;
/**
* Constructs a new {@link ExecRes} containing the given results from the execution of a process.
*
* @param exitCode the exit code of the process
* @param stdOut the standard output of the process
* @param stdErr the standard error output of the process
*/
private ExecRes(int exitCode, String stdOut, String stdErr) {
this.exitCode = exitCode;
this.stdOut = stdOut;
this.stdErr = stdErr;
}
/**
* Returns a trimmed version of the standard output contained in this {@link ExecRes}.
*
* @return the trimmed standard output
*/
public String getStdOutTrimmed() {
if (stdOutTrimmed == null) {
stdOutTrimmed = stdOut.trim();
}
return stdOutTrimmed;
}
/**
* Returns a trimmed version of the standard error output contained in this {@link ExecRes}.
*
* @return the trimmed standard error output
*/
public String getStdErrTrimmed() {
if (stdErrTrimmed == null) {
stdErrTrimmed = stdErr.trim();
}
return stdErrTrimmed;
}
}
/**
......@@ -123,11 +174,11 @@ public class ProcessExecutor {
String stdErr;
if (builder.redirectErrorStream()) {
stdOut = IOUtils.toString(p.getInputStream(), UTF_8).trim();
stdErr = IOUtils.toString(p.getErrorStream(), UTF_8).trim();
stdOut = IOUtils.toString(p.getInputStream(), UTF_8);
stdErr = IOUtils.toString(p.getErrorStream(), UTF_8);
} else {
Future<String> outF = exService.submit(() -> IOUtils.toString(p.getInputStream(), UTF_8).trim());
Future<String> errF = exService.submit(() -> IOUtils.toString(p.getErrorStream(), UTF_8).trim());
Future<String> outF = exService.submit(() -> IOUtils.toString(p.getInputStream(), UTF_8));
Future<String> errF = exService.submit(() -> IOUtils.toString(p.getErrorStream(), UTF_8));
try {
stdOut = outF.get();
......@@ -168,18 +219,18 @@ public class ProcessExecutor {
LOG.fine(() -> String.format("Execution of '%s' returned exit code %d.", cmd, res.exitCode));
if (res.stdOut.isEmpty()) {
if (LOG.isLoggable(FINER) && res.getStdOutTrimmed().isEmpty()) {
LOG.finer(() -> String.format("Execution of '%s' returned no standard output.", cmd));
} else {
} else if (LOG.isLoggable(FINER)) {
LOG.finer(() -> String.format("Execution of '%s' returned standard output.", cmd));
LOG.finest(() -> String.format("Standard output of '%s' was:%n%s", cmd, res.stdOut));
LOG.finest(() -> String.format("Standard output of '%s' was:%n%s", cmd, res.getStdOutTrimmed()));
}
if (res.stdErr.isEmpty()) {
if (LOG.isLoggable(FINER) && res.getStdErrTrimmed().isEmpty()) {
LOG.finer(() -> String.format("Execution of '%s' returned no standard error output.", cmd));
} else {
} else if (LOG.isLoggable(FINER)) {
LOG.finer(() -> String.format("Execution of '%s' returned standard error output.", cmd));
LOG.finest(() -> String.format("Standard output of '%s' was:%n%s", cmd, res.stdErr));
LOG.finest(() -> String.format("Standard output of '%s' was:%n%s", cmd, res.getStdErrTrimmed()));
}
return Optional.of(res);
......
src/de/uni_passau/fim/seibt/gitwrapper/repo/Commit.java
+ 10
− 4
View file @ 82999edf
......@@ -189,8 +189,11 @@ public class Commit extends Reference {
String result = execRes.stdOut;
Matcher matcher = AUTHOR_INFO.matcher(result);
if (!matcher.find()) {
LOG.warning(() -> String.format("Unexpected output while getting commit info for %s.", this));
if (author != null && authorMail != null && authorTime != null) {
LOG.finer("Skipping initialization author information. Already initialized.");
} else if (!matcher.find()) {
LOG.warning(() -> String.format("Unexpected output while getting author info for %s.", this));
} else {
author = matcher.group(1);
authorMail = matcher.group(2);
......@@ -198,8 +201,11 @@ public class Commit extends Reference {
}
matcher = COMMITTER_INFO.matcher(result);
if (!matcher.find()) {
LOG.warning(() -> String.format("Unexpected output while getting commit info for %s.", this));
if (committer != null && committerMail != null && committerTime != null) {
LOG.finer("Skipping initialization committer information. Already initialized.");
} else if (!matcher.find()) {
LOG.warning(() -> String.format("Unexpected output while getting committer info for %s.", this));
} else {
committer = matcher.group(1);
committerMail = matcher.group(2);
......
src/de/uni_passau/fim/seibt/gitwrapper/repo/GitWrapper.java
+ 13
− 8
View file @ 82999edf
package de.uni_passau.fim.seibt.gitwrapper.repo;
import de.uni_passau.fim.seibt.gitwrapper.process.ProcessExecutor.ExecRes;
import de.uni_passau.fim.seibt.gitwrapper.process.ToolNotWorkingException;
import de.uni_passau.fim.seibt.gitwrapper.process.ToolWrapper;
import static de.uni_passau.fim.seibt.gitwrapper.process.ProcessExecutor.asList;
import java.io.File;
import java.util.HashMap;
......@@ -15,7 +13,9 @@ import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import static de.uni_passau.fim.seibt.gitwrapper.process.ProcessExecutor.asList;
import de.uni_passau.fim.seibt.gitwrapper.process.ProcessExecutor.ExecRes;
import de.uni_passau.fim.seibt.gitwrapper.process.ToolNotWorkingException;
import de.uni_passau.fim.seibt.gitwrapper.process.ToolWrapper;
/**
* A wrapper for executing git commands.
......@@ -55,7 +55,7 @@ public class GitWrapper extends ToolWrapper {
@Override
protected boolean isWorking() {
Function<ExecRes, Boolean> checkPrefix = execRes -> execRes.stdOut.startsWith(GIT_VERSION_PREFIX);
Function<ExecRes, Boolean> checkPrefix = execRes -> execRes.getStdOutTrimmed().startsWith(GIT_VERSION_PREFIX);
return exec(new File("."), true, asList(GIT_VERSION_C)).map(checkPrefix).orElse(false);
}
......@@ -77,7 +77,7 @@ public class GitWrapper extends ToolWrapper {
Optional<ExecRes> res = exec(parentDir, "clone", url);
Function<ExecRes, Repository> toRepo = execRes -> {
Scanner sc = new Scanner(execRes.stdOut);
Scanner sc = new Scanner(execRes.getStdOutTrimmed());
if (!sc.hasNextLine()) {
LOG.warning(() -> String.format("No output while cloning '%s'.", url));
......@@ -132,13 +132,17 @@ public class GitWrapper extends ToolWrapper {
return Optional.empty();
}
// TODO add fallback method for determining URL, add Logging
Optional<ExecRes> result = exec(directory, "config", "--get", "remote.origin.url");
Optional<String> url = result.map(res -> {
String repoUrl = null;
if (this.failed(res) || (repoUrl = res.stdOut.trim()).isEmpty()) {
if (failed(res) || (repoUrl = res.getStdOutTrimmed()).isEmpty()) {
// there was no remote url
repoUrl = directory.getAbsolutePath();
}
return repoUrl;
});
return Optional.of(new Repository(this, url.orElse(""), directory));
......@@ -170,7 +174,8 @@ public class GitWrapper extends ToolWrapper {
* @return whether the git command failed
*/
public boolean failed(ExecRes res) {
return res.exitCode != EXIT_SUCCESS || res.stdOut.startsWith(FATAL_PREFIX) || res.stdOut.startsWith(ERROR_PREFIX);
return res.exitCode != EXIT_SUCCESS || res.getStdOutTrimmed().startsWith(FATAL_PREFIX)
|| res.getStdOutTrimmed().startsWith(ERROR_PREFIX);
}
/**
......
src/de/uni_passau/fim/seibt/gitwrapper/repo/Reference.java
+ 9
− 5
View file @ 82999edf
package de.uni_passau.fim.seibt.gitwrapper.repo;
import de.uni_passau.fim.seibt.gitwrapper.process.ProcessExecutor;
import java.util.*;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Function;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import de.uni_passau.fim.seibt.gitwrapper.process.ProcessExecutor;
public abstract class Reference {
private static final Logger LOG = Logger.getLogger(Reference.class.getCanonicalName());
......@@ -50,7 +54,7 @@ public abstract class Reference {
return null;
}
Commit base = repo.getCommitUnchecked(res.stdOut);
Commit base = repo.getCommitUnchecked(res.getStdOutTrimmed());
LOG.fine(() -> String.format("Commits %s and %s have the merge base %s.", this, other, base));
......@@ -84,7 +88,7 @@ public abstract class Reference {
public List<Commit> getParents() {
Optional<ProcessExecutor.ExecRes> revList = git.exec(repo.getDir(), "rev-list", "--parents", "-n", "1", id);
Function<ProcessExecutor.ExecRes, List<Commit>> toParentsList = res -> {
String[] ids = res.stdOut.split("\\s+");
String[] ids = res.getStdOutTrimmed().split("\\s+");
LOG.fine(() -> String.format("Found %d parents for %s.", ids.length - 1, this));
LOG.finer(() -> String.format("Commit id and parents are:%n%s", String.join(System.lineSeparator(), ids)));
......
src/de/uni_passau/fim/seibt/gitwrapper/repo/Repository.java
+ 33
− 13
View file @ 82999edf
package de.uni_passau.fim.seibt.gitwrapper.repo;
import de.uni_passau.fim.seibt.gitwrapper.process.ProcessExecutor.ExecRes;
import org.apache.commons.io.FileUtils;
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
......@@ -10,12 +7,25 @@ import java.nio.file.Paths;
import java.time.Instant;
import java.time.OffsetDateTime;
import java.time.ZoneId;
import java.util.*;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.Optional;
import java.util.Scanner;
import java.util.function.Function;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import org.apache.commons.io.FileUtils;
import de.uni_passau.fim.seibt.gitwrapper.process.ProcessExecutor.ExecRes;
/**
* A git {@link Repository}.
*/
......@@ -33,7 +43,8 @@ public class Repository {
private static final String COMMITTER_TIME = "committer-time";
private static final String COMMITTER_TZ = "committer-tz";
private static final String FILENAME = "filename";
private static final String MESSAGE = "summary";
private static final String SUMMARY = "summary";
private static final String BOUNDARY = "boundary";
// The type returned for commits by 'git cat-file -t".
private static final String TYPE_COMMIT = "commit";
......@@ -158,7 +169,13 @@ public class Repository {
return null;
}
String[] lines = res.stdOut.isEmpty() ? new String[]{} : res.stdOut.split("[\\r?\\n]+");
String[] lines;
if (res.getStdOutTrimmed().isEmpty()) {
lines = new String[] {};
} else {
lines = res.getStdOutTrimmed().split("[\\r?\\n]+");
}
LOG.fine(() -> String.format("Found %d merge commits in %s.", lines.length, this));
......@@ -186,7 +203,7 @@ public class Repository {
return null;
}
return getCommitUnchecked(res.stdOut);
return getCommitUnchecked(res.getStdOutTrimmed());
};
return revParse.map(toHEAD);
......@@ -240,7 +257,7 @@ public class Repository {
return null;
}
boolean isCommit = res.stdOut.startsWith(TYPE_COMMIT);
boolean isCommit = res.getStdOutTrimmed().startsWith(TYPE_COMMIT);
if (isCommit) {
LOG.finer(() -> String.format("%s is a commit.", id));
......@@ -316,9 +333,9 @@ public class Repository {
return null;
}
LOG.finer(() -> String.format("Resolved %s to %s.", id, res.stdOut));
LOG.finer(() -> String.format("Resolved %s to %s.", id, res.getStdOutTrimmed()));
return res.stdOut;
return res.getStdOutTrimmed();
};
return revParse.map(toHash);
......@@ -464,11 +481,14 @@ public class Repository {
case FILENAME:
commitFile = dir.toPath().resolve(Paths.get(lineScanner.nextLine().trim())).toFile();
break;
case MESSAGE:
commit.setMessage(lineScanner.nextLine().trim());
case SUMMARY:
commit.setMessage(lineScanner.nextLine().trim()); // TODO this is NOT the message, it is the summary (first line of the message)
break;
case BOUNDARY:
// We do not store the information that this commit was the boundary of the 'git blame' call.
break;
default:
other.put(headerKey, lineScanner.nextLine().trim());
other.put(headerKey, lineScanner.hasNextLine() ? lineScanner.nextLine().trim() : "");
}
}
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment