Skip to content

Commit

Permalink
868: Skara should ignore .gitconfig and /etc/gitconfig
Browse files Browse the repository at this point in the history 
Reviewed-by: ehelin
  • Loading branch information
@magicus @edvbld
magicus authored and edvbld committed Feb 4, 2021
1 parent b0be0fa commit ae50218
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 17 deletions.
9 changes: 8 additions & 1 deletion process/src/main/java/org/openjdk/skara/process/Process.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -67,6 +67,13 @@ public Description environ(String key, String value) {
return this;
}

public Description environ(Map<String, String> keyValueMap) {
if (keyValueMap != null) {
getCurrentProcessBuilderSetup().environment.putAll(keyValueMap);
}
return this;
}

public Description timeout(Duration timeout) {
this.timeout = timeout;
return this;
Expand Down
9 changes: 7 additions & 2 deletions test/src/main/java/org/openjdk/skara/test/HostCredentials.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -330,18 +330,23 @@ public Hash commitLock(Repository localRepo) throws IOException {
return lockHash;
}

public HostedRepository getHostedRepository() {
public HostedRepository getHostedRepository() throws IOException {
var host = getRepositoryHost();
var repo = credentials.getHostedRepository(host);

var retryCount = 0;
while (credentialsLock == null) {
try {
if (getLock(repo)) {
credentialsLock = repo;
}
} catch (IOException e) {
if (retryCount > 3) {
throw e;
}
try {
Thread.sleep(Duration.ofSeconds(1).toMillis());
retryCount++;
} catch (InterruptedException ignored) {
}
}
Expand Down
3 changes: 2 additions & 1 deletion vcs/src/main/java/org/openjdk/skara/vcs/git/GitCommits.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -81,6 +81,7 @@ public Iterator<Commit> iterator() {
cmd.add(range);
var pb = new ProcessBuilder(cmd);
pb.directory(dir.toFile());
pb.environment().putAll(GitRepository.NO_CONFIG_ENV);
var command = pb.command();
try {
var p = pb.start();
Expand Down
31 changes: 27 additions & 4 deletions vcs/src/main/java/org/openjdk/skara/vcs/git/GitRepository.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -38,6 +38,12 @@
import java.util.stream.Collectors;

public class GitRepository implements Repository {
public final static Map<String, String> NO_CONFIG_ENV = Map.of(
"HOME", "/this-does-not-exist-and-if-you-create-it-you-are-in-trouble",
"XDG_CONFIG_HOME", "/this-does-not-exist-and-if-you-create-it-you-are-in-trouble",
"GIT_CONFIG_NOSYSTEM", "true"
);

private final Path dir;
private final Logger log = Logger.getLogger("org.openjdk.skara.vcs.git");
private Path cachedRoot = null;
Expand All @@ -51,6 +57,7 @@ private java.lang.Process start(List<String> cmd) throws IOException {
log.fine("Executing " + String.join(" ", cmd));
var pb = new ProcessBuilder(cmd);
pb.directory(dir.toFile());
pb.environment().putAll(NO_CONFIG_ENV);
pb.redirectError(ProcessBuilder.Redirect.DISCARD);
return pb.start();
}
Expand All @@ -75,13 +82,22 @@ private Execution capture(List<String> cmd) {
return capture(cmd.toArray(new String[0]));
}

private static Execution capture(Path cwd, Map<String, String> env, List<String> cmd) {
return capture(cwd, env, cmd.toArray(new String[0]));
}

private Execution capture(String... cmd) {
return capture(dir, cmd);
}

private static Execution capture(Path cwd, String... cmd) {
public static Execution capture(Path cwd, String... cmd) {
return capture(cwd, NO_CONFIG_ENV, cmd);
}

private static Execution capture(Path cwd, Map<String, String> env, String... cmd) {
return Process.capture(cmd)
.workdir(cwd)
.environ(env)
.execute();
}

Expand Down Expand Up @@ -718,6 +734,7 @@ public Hash commit(String message,
ZonedDateTime committerDate) throws IOException {
var cmd = Process.capture("git", "commit", "--message=" + message)
.workdir(dir)
.environ(NO_CONFIG_ENV)
.environ("GIT_AUTHOR_NAME", authorName)
.environ("GIT_AUTHOR_EMAIL", authorEmail)
.environ("GIT_COMMITTER_NAME", committerName)
Expand Down Expand Up @@ -753,6 +770,7 @@ public Hash commit(String message, String authorName, String authorEmail, ZonedD
}
var cmd = Process.capture(cmdLine.toArray(new String[0]))
.workdir(dir)
.environ(NO_CONFIG_ENV)
.environ("GIT_AUTHOR_NAME", authorName)
.environ("GIT_AUTHOR_EMAIL", authorEmail)
.environ("GIT_COMMITTER_NAME", committerName)
Expand Down Expand Up @@ -805,6 +823,7 @@ public Hash amend(String message, String authorName, String authorEmail, String
}
var cmd = Process.capture("git", "commit", "--amend", "--reset-author", "--message=" + message)
.workdir(dir)
.environ(NO_CONFIG_ENV)
.environ("GIT_AUTHOR_NAME", authorName)
.environ("GIT_AUTHOR_EMAIL", authorEmail)
.environ("GIT_COMMITTER_NAME", committerName)
Expand All @@ -819,6 +838,7 @@ public Hash amend(String message, String authorName, String authorEmail, String
public Tag tag(Hash hash, String name, String message, String authorName, String authorEmail, ZonedDateTime date) throws IOException {
var cmd = Process.capture("git", "tag", "--annotate", "--message=" + message, name, hash.hex())
.workdir(dir)
.environ(NO_CONFIG_ENV)
.environ("GIT_AUTHOR_NAME", authorName)
.environ("GIT_AUTHOR_EMAIL", authorEmail)
.environ("GIT_COMMITTER_NAME", authorName)
Expand Down Expand Up @@ -878,6 +898,7 @@ public void rebase(Hash hash, String committerName, String committerEmail) throw
.environ("GIT_COMMITTER_NAME", committerName)
.environ("GIT_COMMITTER_EMAIL", committerEmail)
.workdir(dir)
.environ(NO_CONFIG_ENV)
.execute()) {
await(p);
}
Expand Down Expand Up @@ -1133,7 +1154,8 @@ public Diff diff(Hash from, Hash to, List<Path> files, int similarity) throws IO

@Override
public List<String> config(String key) throws IOException {
try (var p = capture("git", "config", key)) {
// We must explicitly do this *with* the user's .gitconfig, so override NO_CONFIG_ENV
try (var p = capture(dir, Collections.emptyMap(), "git", "config", key)) {
var res = p.await();
return res.status() == 0 ? res.stdout() : List.of();
}
Expand Down Expand Up @@ -1531,7 +1553,8 @@ public void config(String section, String key, String value, boolean global) thr
}
cmd.add(section + "." + key);
cmd.add(value);
try (var p = capture(cmd)) {
// We must explicitly do this *with* the user's .gitconfig, so override NO_CONFIG_ENV
try (var p = capture(dir, Collections.emptyMap(), cmd)) {
await(p);
}
}
Expand Down
21 changes: 13 additions & 8 deletions vcs/src/main/java/org/openjdk/skara/vcs/hg/HgRepository.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -37,6 +37,14 @@
import java.net.URI;

public class HgRepository implements Repository {
public final static Map<String, String> NO_CONFIG_ENV = Map.of(
"HGRCPATH", "",
"HGPLAIN", "",
"HGEDITOR", "",
"EDITOR", "",
"VISUAL", ""
);

private static final String EXT_PY = "ext.py";
private final Path dir;
private final Logger log = Logger.getLogger("org.openjdk.skara.vcs.hg");
Expand All @@ -54,8 +62,7 @@ private java.lang.Process start(List<String> cmd) throws IOException {
var pb = new ProcessBuilder(cmd);
pb.directory(dir.toFile());
pb.redirectError(ProcessBuilder.Redirect.DISCARD);
pb.environment().put("HGRCPATH", "");
pb.environment().put("HGPLAIN", "");
pb.environment().putAll(NO_CONFIG_ENV);
return pb.start();
}

Expand Down Expand Up @@ -86,10 +93,9 @@ private Execution capture(String... cmd) {
private static Execution capture(Path cwd, List<String> cmd) {
return capture(cwd, cmd.toArray(new String[0]));
}
private static Execution capture(Path cwd, String... cmd) {
public static Execution capture(Path cwd, String... cmd) {
return Process.capture(cmd)
.environ("HGRCPATH", "")
.environ("HGPLAIN", "")
.environ(NO_CONFIG_ENV)
.workdir(cwd)
.execute();
}
Expand Down Expand Up @@ -589,8 +595,7 @@ private void export(String revset, Path to) throws IOException {
pb.directory(dir.toFile());
pb.redirectError(ProcessBuilder.Redirect.DISCARD);
pb.redirectOutput(to.toFile());
pb.environment().put("HGRCPATH", "");
pb.environment().put("HGPLAIN", "");
pb.environment().putAll(NO_CONFIG_ENV);
var p = pb.start();
try {
await(p);
Expand Down
43 changes: 42 additions & 1 deletion vcs/src/test/java/org/openjdk/skara/vcs/RepositoryTests.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -28,6 +28,8 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;
import org.openjdk.skara.vcs.git.GitRepository;
import org.openjdk.skara.vcs.hg.HgRepository;

import java.io.IOException;
import java.net.URI;
Expand Down Expand Up @@ -1653,6 +1655,7 @@ void testSingleEmptyCommit() throws IOException, InterruptedException {
pb.environment().put("GIT_COMMITTER_NAME", "duke");
pb.environment().put("GIT_COMMITTER_EMAIL", "duke@openjdk.org");
pb.directory(dir.path().toFile());
pb.environment().putAll(GitRepository.NO_CONFIG_ENV);

var res = pb.start().waitFor();
assertEquals(0, res);
Expand Down Expand Up @@ -1686,6 +1689,7 @@ void testEmptyCommitWithParent() throws IOException, InterruptedException {
pb.environment().put("GIT_COMMITTER_NAME", "duke");
pb.environment().put("GIT_COMMITTER_EMAIL", "duke@openjdk.org");
pb.directory(dir.path().toFile());
pb.environment().putAll(GitRepository.NO_CONFIG_ENV);

var res = pb.start().waitFor();
assertEquals(0, res);
Expand Down Expand Up @@ -2179,6 +2183,42 @@ void testWritingConfigValue(VCS vcs) throws IOException {
}
}

@ParameterizedTest
@EnumSource(VCS.class)
void testNoConfig(VCS vcs) throws IOException, InterruptedException {
// Verify that our method of disabling configuration works
try (var dir = new TemporaryDirectory()) {
switch (vcs) {
case GIT -> {
var gitRepo = new GitRepository(dir.path()).init();
var configResult = GitRepository.capture(dir.path(),
"git", "config", "--list").await();
assertEquals(configResult.status(), 0);

// We can't get a list of all settings except local, so compare all with local only
var localConfigResult = GitRepository.capture(dir.path(),
"git", "config", "--list", "--local").await();
assertEquals(localConfigResult.status(), 0);
assertEquals(localConfigResult.stdout(), configResult.stdout());
}

case HG -> {
var hgRepo = new HgRepository(dir.path()).init();
var settingsResult = HgRepository.capture(dir.path(),
"hg", "config").await();
assertEquals(settingsResult.status(), 0);

// There's no way to stop hg from picking up ui.editor or repo settings,
// nor to print only them, so hard-code these settings.
var filteredSettings = settingsResult.stdout().stream().filter(
s -> !(s.startsWith("bundle.mainreporoot=") || s.startsWith("ui.editor="))
).toArray();
assertTrue(filteredSettings.length == 0);
}
}
}
}

@ParameterizedTest
@EnumSource(VCS.class)
void testFetchRemote(VCS vcs) throws IOException {
Expand Down Expand Up @@ -2448,6 +2488,7 @@ void testLightweightTags() throws IOException, InterruptedException {
// so use a ProcessBuilder and invoke git directly here
var pb = new ProcessBuilder("git", "tag", "test-tag", head.hex());
pb.directory(repo.root().toFile());
pb.environment().putAll(GitRepository.NO_CONFIG_ENV);
assertEquals(0, pb.start().waitFor());

var tags = repo.tags();
Expand Down

1 comment on commit ae50218

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.