Skip to content

Commit 48723a8

Browse files
Copilotkgilpindividedmind
authored
fix: Don't crash when running in git repositories without remotes
The metadata writing code assumed that a repository always has a remote, which lead to crashes when it wasn't true. Now if the git repository has no remotes, `repository` key in the metadata is simply not included. Additionally, the code now searches for the git repository based on the config file path instead of the working directory (which is a vague-ish concept in JVM-land). Co-authored-by: kgilpin <86395+kgilpin@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Rafał Rzepecki <divided.mind@gmail.com>
1 parent a2d21e0 commit 48723a8

6 files changed

Lines changed: 108 additions & 10 deletions

File tree

‎agent/src/main/java/com/appland/appmap/config/AppMapConfig.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.tinylog.TaggedLogger;
2828
import org.tinylog.configuration.Configuration;
2929

30-
import com.alibaba.fastjson.JSON;
3130
import com.appland.appmap.Agent;
3231
import com.appland.appmap.cli.CLI;
3332
import com.appland.appmap.util.FullyQualifiedName;
@@ -115,7 +114,7 @@ private static Path findConfig(Path configFile, boolean mustExist) throws FileNo
115114
*
116115
* @return The AppMapConfig singleton
117116
*/
118-
static AppMapConfig load(Path configFile, boolean mustExist) {
117+
public static AppMapConfig load(Path configFile, boolean mustExist) {
119118
InputStream inputStream = null;
120119

121120
try {

‎agent/src/main/java/com/appland/appmap/record/AppMapSerializer.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,11 @@ private void writeMetadata(GitUtil git, Metadata metadata) throws IOException {
185185
this.json.writeKey("git");
186186
this.json.startObject();
187187
{
188-
this.json.writeKey("repository");
189-
this.json.writeValue(git.getRepositoryURL());
188+
String repositoryURL = git.getRepositoryURL();
189+
if (repositoryURL != null) {
190+
this.json.writeKey("repository");
191+
this.json.writeValue(repositoryURL);
192+
}
190193
this.json.writeKey("branch");
191194
this.json.writeValue(git.getBranch());
192195
this.json.writeKey("commit");

‎agent/src/main/java/com/appland/appmap/util/GitUtil.java‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
import java.util.Map;
1414
import java.util.Optional;
1515

16+
import javax.annotation.Nullable;
17+
1618
import org.eclipse.jgit.api.Git;
1719
import org.eclipse.jgit.api.errors.GitAPIException;
1820
import org.eclipse.jgit.errors.RevisionSyntaxException;
@@ -60,7 +62,8 @@ public static GitUtil open() throws IOException {
6062
FileRepositoryBuilder builder = new FileRepositoryBuilder()
6163
.readEnvironment();
6264

63-
builder.findGitDir();
65+
Path fsBase = AppMapConfig.get().configFile.toAbsolutePath().getParent();
66+
builder.findGitDir(fsBase.toFile());
6467
if (builder.getGitDir() == null) {
6568
logger.debug("Working directory {}, not in a git repo", () -> Paths.get("").toAbsolutePath());
6669
return null;
@@ -73,7 +76,6 @@ public static GitUtil open() throws IOException {
7376
return null;
7477
}
7578

76-
Path fsBase = AppMapConfig.get().configFile.toAbsolutePath().getParent();
7779
ObjectId tree = repository.resolve(Constants.HEAD + "^{tree}");
7880
if (tree == null) {
7981
logger.warn("Couldn't resolve HEAD to a tree in {}, source paths may be incorrect", fsBase);
@@ -96,16 +98,23 @@ public Repository getRepository() {
9698
return git.getRepository();
9799
}
98100

99-
public String getRepositoryURL() {
101+
public @Nullable String getRepositoryURL() {
100102
try {
101103
List<RemoteConfig> remotes = git.remoteList().call();
104+
if (remotes.isEmpty()) {
105+
return null;
106+
}
102107
Optional<RemoteConfig> originConfig = remotes.stream().filter(r -> r.getName().equals("origin")).findFirst();
103-
List<URIish> uris = originConfig.isPresent() ? originConfig.get().getURIs() : remotes.get(0).getURIs();
108+
RemoteConfig remote = originConfig.orElseGet(() -> remotes.get(0));
109+
List<URIish> uris = remote.getURIs();
110+
if (uris.isEmpty()) {
111+
return null;
112+
}
104113
return uris.get(0).toASCIIString();
105114
} catch (GitAPIException e) {
106115
logger.warn(e);
107116
}
108-
return "";
117+
return null;
109118
}
110119

111120
public String getBranch() {
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package com.appland.appmap.record;
2+
3+
import static org.junit.jupiter.api.Assertions.assertFalse;
4+
import static org.mockito.Mockito.mock;
5+
import static org.mockito.Mockito.when;
6+
7+
import java.io.StringWriter;
8+
import java.nio.file.Files;
9+
import java.nio.file.Path;
10+
import java.util.Map;
11+
12+
import org.eclipse.jgit.api.Git;
13+
import org.junit.jupiter.api.BeforeEach;
14+
import org.junit.jupiter.api.Test;
15+
import org.junit.jupiter.api.io.TempDir;
16+
17+
import com.appland.appmap.config.AppMapConfig;
18+
import com.appland.appmap.record.Recorder.Metadata;
19+
20+
public class AppMapSerializerTest {
21+
@TempDir
22+
Path tempDir;
23+
24+
@BeforeEach
25+
public void setup() throws Exception {
26+
Path configFile = tempDir.resolve("appmap.yml");
27+
Files.write(configFile, "name: test\npackages: []".getBytes());
28+
AppMapConfig.load(configFile, true);
29+
}
30+
31+
@Test
32+
public void testMetadataNoRepositoryURL() throws Exception {
33+
try (var gitRepo = Git.init().setDirectory(tempDir.toFile()).call()) {
34+
Path testFile = tempDir.resolve("test.txt");
35+
Files.write(testFile, "test".getBytes());
36+
gitRepo.add().addFilepattern("test.txt").call();
37+
gitRepo.commit().setMessage("initial commit").call();
38+
}
39+
40+
Metadata metadata = new Metadata("test-recorder", "test-type");
41+
42+
StringWriter writer = new StringWriter();
43+
try (AppMapSerializer serializer = AppMapSerializer.open(writer)) {
44+
serializer.write(new CodeObjectTree(), metadata, Map.of());
45+
}
46+
47+
String json = writer.toString();
48+
assertFalse(json.contains("\"repository\""), "Metadata should not contain 'repository' if URL is null. JSON: " + json);
49+
}
50+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package com.appland.appmap.util;
2+
3+
import static org.junit.jupiter.api.Assertions.assertNull;
4+
5+
import java.nio.file.Files;
6+
import java.nio.file.Path;
7+
8+
import org.eclipse.jgit.api.Git;
9+
import org.junit.jupiter.api.Test;
10+
import org.junit.jupiter.api.io.TempDir;
11+
12+
import com.appland.appmap.config.AppMapConfig;
13+
14+
public class GitUtilTest {
15+
@TempDir
16+
Path tempDir;
17+
18+
@Test
19+
public void testGetRepositoryURLNoRemotes() throws Exception {
20+
try (Git git = Git.init().setDirectory(tempDir.toFile()).call()) {
21+
Path testFile = tempDir.resolve("test.txt");
22+
Files.write(testFile, "test".getBytes());
23+
git.add().addFilepattern("test.txt").call();
24+
git.commit().setMessage("initial commit").call();
25+
26+
Path configFile = tempDir.resolve("appmap.yml");
27+
Files.write(configFile, "name: test\npackages: []".getBytes());
28+
29+
AppMapConfig.load(configFile, true);
30+
31+
try (GitUtil gitUtil = GitUtil.open()) {
32+
assertNull(gitUtil.getRepositoryURL(), "Repository URL should be null when no remotes exist");
33+
}
34+
}
35+
}
36+
}

‎agent/test/helper.bash‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,11 +245,12 @@ start_petclinic() {
245245
AGENT_JAR="$(find_agent_jar)"
246246

247247
local cfg="$WD/test/petclinic/appmap.yml"
248+
cp "$cfg" "${fixture_dir}/appmap.yml"
248249
local out="${fixture_dir}/tmp/appmap"
249250
pushd build/fixtures/spring-petclinic >/dev/null
250251
./mvnw ${MAVEN_PROFILE} ${BATS_VERSION+--quiet} -DskipTests -Dcheckstyle.skip=true -Dspring-javaformat.skip=true \
251252
-Dspring-boot.run.agents=$AGENT_JAR \
252-
-Dspring-boot.run.jvmArguments="-Dappmap.config.file='${cfg}' -Dappmap.output.directory='${out}' $jvmargs" \
253+
-Dspring-boot.run.jvmArguments="-Dappmap.config.file='${fixture_dir}/appmap.yml' -Dappmap.output.directory='${out}' $jvmargs" \
253254
spring-boot:run \
254255
&>$LOG 3>&- &
255256
export JVM_MAIN_CLASS=PetClinicApplication

0 commit comments

Comments
 (0)