diff --git a/cli/src/main/java/org/openjdk/skara/cli/GitWebrev.java b/cli/src/main/java/org/openjdk/skara/cli/GitWebrev.java index 711e8f861..a7daf9f00 100644 --- a/cli/src/main/java/org/openjdk/skara/cli/GitWebrev.java +++ b/cli/src/main/java/org/openjdk/skara/cli/GitWebrev.java @@ -36,6 +36,7 @@ import java.nio.file.*; import java.util.*; import java.util.regex.Pattern; +import java.util.stream.Collectors; public class GitWebrev { private static void clearDirectory(Path directory) { @@ -134,7 +135,13 @@ private static void generate(String[] args) throws IOException { .helptext("Print the version of this tool") .optional()); - var parser = new ArgumentParser("git webrev", flags); + var inputs = List.of( + Input.position(0) + .describe("FILE") + .singular() + .optional()); + + var parser = new ArgumentParser("git webrev", flags, inputs); var arguments = parser.parse(args); var version = Version.fromManifest().orElse("unknown"); @@ -246,6 +253,11 @@ private static void generate(String[] args) throws IOException { clearDirectory(output); } + List files = List.of(); + if (arguments.at(0).isPresent()) { + var path = arguments.at(0).via(Path::of); + files = Files.readAllLines(path).stream().map(Path::of).collect(Collectors.toList()); + } Webrev.repository(repo) .output(output) .title(title) @@ -253,6 +265,7 @@ private static void generate(String[] args) throws IOException { .username(username) .issue(issue) .version(version) + .files(files) .generate(rev); } diff --git a/jcheck/src/test/java/org/openjdk/skara/jcheck/TestRepository.java b/jcheck/src/test/java/org/openjdk/skara/jcheck/TestRepository.java index 4e1d2ce29..c267de82f 100644 --- a/jcheck/src/test/java/org/openjdk/skara/jcheck/TestRepository.java +++ b/jcheck/src/test/java/org/openjdk/skara/jcheck/TestRepository.java @@ -189,10 +189,18 @@ public Diff diff(Hash base, Hash head) throws IOException { return null; } + public Diff diff(Hash base, Hash head, List files) throws IOException { + return null; + } + public Diff diff(Hash head) throws IOException { return null; } + public Diff diff(Hash head, List files) throws IOException { + return null; + } + public List config(String key) throws IOException { return null; } diff --git a/vcs/src/main/java/org/openjdk/skara/vcs/ReadOnlyRepository.java b/vcs/src/main/java/org/openjdk/skara/vcs/ReadOnlyRepository.java index 88e7ee9da..6634e49ae 100644 --- a/vcs/src/main/java/org/openjdk/skara/vcs/ReadOnlyRepository.java +++ b/vcs/src/main/java/org/openjdk/skara/vcs/ReadOnlyRepository.java @@ -77,7 +77,9 @@ default List files(Hash h, Path... paths) throws IOException { void dump(FileEntry entry, Path to) throws IOException; List status(Hash from, Hash to) throws IOException; Diff diff(Hash base, Hash head) throws IOException; + Diff diff(Hash base, Hash head, List files) throws IOException; Diff diff(Hash head) throws IOException; + Diff diff(Hash head, List files) throws IOException; List config(String key) throws IOException; Repository copyTo(Path destination) throws IOException; String pullPath(String remote) throws IOException; diff --git a/vcs/src/main/java/org/openjdk/skara/vcs/git/GitRepository.java b/vcs/src/main/java/org/openjdk/skara/vcs/git/GitRepository.java index 6e2847f6d..22a528392 100644 --- a/vcs/src/main/java/org/openjdk/skara/vcs/git/GitRepository.java +++ b/vcs/src/main/java/org/openjdk/skara/vcs/git/GitRepository.java @@ -819,11 +819,21 @@ public List status(Hash from, Hash to) throws IOException { @Override public Diff diff(Hash from) throws IOException { - return diff(from, null); + return diff(from, List.of()); + } + + @Override + public Diff diff(Hash from, List files) throws IOException { + return diff(from, null, files); } @Override public Diff diff(Hash from, Hash to) throws IOException { + return diff(from, to, List.of()); + } + + @Override + public Diff diff(Hash from, Hash to, List files) throws IOException { var cmd = new ArrayList<>(List.of("git", "diff", "--patch", "--find-renames=99%", "--find-copies=99%", @@ -838,6 +848,13 @@ public Diff diff(Hash from, Hash to) throws IOException { cmd.add(to.hex()); } + if (files != null && !files.isEmpty()) { + cmd.add("--"); + for (var file : files) { + cmd.add(file.toString()); + } + } + var p = start(cmd); try { var patches = UnifiedDiffParser.parseGitRaw(p.getInputStream()); diff --git a/vcs/src/main/java/org/openjdk/skara/vcs/hg/HgRepository.java b/vcs/src/main/java/org/openjdk/skara/vcs/hg/HgRepository.java index 59c391a93..ff67b2430 100644 --- a/vcs/src/main/java/org/openjdk/skara/vcs/hg/HgRepository.java +++ b/vcs/src/main/java/org/openjdk/skara/vcs/hg/HgRepository.java @@ -736,11 +736,21 @@ public void revert(Hash parent) throws IOException { @Override public Diff diff(Hash from) throws IOException { - return diff(from, null); + return diff(from, List.of()); + } + + @Override + public Diff diff(Hash from, List files) throws IOException { + return diff(from, null, files); } @Override public Diff diff(Hash from, Hash to) throws IOException { + return diff(from, to, List.of()); + } + + @Override + public Diff diff(Hash from, Hash to, List files) throws IOException { var ext = Files.createTempFile("ext", ".py"); copyResource(EXT_PY, ext); @@ -750,6 +760,11 @@ public Diff diff(Hash from, Hash to) throws IOException { cmd.add(to.hex()); } + if (files != null) { + var filenames = files.stream().map(Path::toString).collect(Collectors.toList()); + cmd.add("--files=" + String.join(",", filenames)); + } + var p = start(cmd); try { var patches = UnifiedDiffParser.parseGitRaw(p.getInputStream()); diff --git a/vcs/src/main/resources/ext.py b/vcs/src/main/resources/ext.py index 9dc1ad90d..2d6d90b0c 100644 --- a/vcs/src/main/resources/ext.py +++ b/vcs/src/main/resources/ext.py @@ -155,8 +155,8 @@ def decorator(func): revsingle = mercurial.cmdutil.revsingle revrange = mercurial.cmdutil.revrange -@command(b'diff-git-raw', [(b'', b'patch', False, b'')], b'hg diff-git-raw rev1 [rev2]') -def diff_git_raw(ui, repo, rev1, rev2=None, **opts): +@command(b'diff-git-raw', [(b'', b'patch', False, b''), (b'', b'files', b'', b'')], b'hg diff-git-raw rev1 [rev2]') +def diff_git_raw(ui, repo, rev1, rev2=None, *files, **opts): ctx1 = revsingle(repo, rev1) if rev2 != None: @@ -167,6 +167,14 @@ def diff_git_raw(ui, repo, rev1, rev2=None, **opts): status = repo.status(ctx1) modified, added, removed = [set(l) for l in status[:3]] + + files = opts['files'] + if files != b'': + wanted = set(files.split(b',')) + modified = modified & wanted + added = added & wanted + removed = removed & wanted + _diff_git_raw(repo, ctx1, ctx2, modified, added, removed, opts['patch']) @command(b'log-git', [(b'', b'reverse', False, b''), (b'l', b'limit', -1, b'')], b'hg log-git ') diff --git a/vcs/src/test/java/org/openjdk/skara/vcs/RepositoryTests.java b/vcs/src/test/java/org/openjdk/skara/vcs/RepositoryTests.java index 1b1c1edea..a0bc7ec15 100644 --- a/vcs/src/test/java/org/openjdk/skara/vcs/RepositoryTests.java +++ b/vcs/src/test/java/org/openjdk/skara/vcs/RepositoryTests.java @@ -1944,4 +1944,54 @@ void testAnnotateTagOnEmptyRepo(VCS vcs) throws IOException { assertEquals(Optional.empty(), repo.annotate(new Tag("unknown"))); } } + + @ParameterizedTest + @EnumSource(VCS.class) + void testDiffWithFileList(VCS vcs) throws IOException { + try (var dir = new TemporaryDirectory(false)) { + var repo = Repository.init(dir.path(), vcs); + var readme = repo.root().resolve("README"); + Files.writeString(readme, "Hello\n"); + repo.add(readme); + + var contribute = repo.root().resolve("CONTRIBUTE"); + Files.writeString(contribute, "1. Make changes\n"); + repo.add(contribute); + + var first = repo.commit("Added README and CONTRIBUTE", "duke", "duke@openjdk.org"); + Files.writeString(readme, "World\n", WRITE, APPEND); + Files.writeString(contribute, "2. Run git commit", WRITE, APPEND); + + var diff = repo.diff(first, List.of(Path.of("README"))); + assertEquals(1, diff.added()); + assertEquals(0, diff.modified()); + assertEquals(0, diff.removed()); + var patches = diff.patches(); + assertEquals(1, patches.size()); + var patch = patches.get(0); + assertTrue(patch.isTextual()); + assertTrue(patch.status().isModified()); + assertEquals(Path.of("README"), patch.source().path().get()); + assertEquals(Path.of("README"), patch.target().path().get()); + + repo.add(readme); + repo.add(contribute); + var second = repo.commit("Updates to both README and CONTRIBUTE", "duke", "duke@openjdk.org"); + + diff = repo.diff(first, second, List.of(Path.of("CONTRIBUTE"))); + assertEquals(1, diff.added()); + assertEquals(0, diff.modified()); + assertEquals(0, diff.removed()); + patches = diff.patches(); + assertEquals(1, patches.size()); + patch = patches.get(0); + assertTrue(patch.isTextual()); + assertTrue(patch.status().isModified()); + assertEquals(Path.of("CONTRIBUTE"), patch.source().path().get()); + assertEquals(Path.of("CONTRIBUTE"), patch.target().path().get()); + + diff = repo.diff(first, second, List.of(Path.of("DOES_NOT_EXIST"))); + assertEquals(0, diff.patches().size()); + } + } } diff --git a/webrev/src/main/java/org/openjdk/skara/webrev/Webrev.java b/webrev/src/main/java/org/openjdk/skara/webrev/Webrev.java index 3222c4b55..90921fa50 100644 --- a/webrev/src/main/java/org/openjdk/skara/webrev/Webrev.java +++ b/webrev/src/main/java/org/openjdk/skara/webrev/Webrev.java @@ -61,6 +61,7 @@ public static class Builder { private String branch; private String issue; private String version; + private List files = List.of(); Builder(ReadOnlyRepository repository, Path output) { this.repository = repository; @@ -102,6 +103,11 @@ public Builder version(String version) { return this; } + public Builder files(List files) { + this.files = files; + return this; + } + public void generate(Hash tailEnd) throws IOException { generate(tailEnd, null); } @@ -114,9 +120,36 @@ public void generate(Hash tailEnd, Hash head) throws IOException { copyResource(CSS); copyResource(ICON); - var diff = head == null ? repository.diff(tailEnd) : repository.diff(tailEnd, head); + var diff = head == null ? + repository.diff(tailEnd, files) : + repository.diff(tailEnd, head, files); var patchFile = output.resolve(Path.of(title).getFileName().toString() + ".patch"); + var patches = diff.patches(); + if (files != null && !files.isEmpty()) { + // Sort the patches according to how they are listed in the `files` list. + var byTargetPath = new HashMap(); + var bySourcePath = new HashMap(); + for (var patch : patches) { + if (patch.target().path().isPresent()) { + byTargetPath.put(patch.target().path().get(), patch); + } else { + bySourcePath.put(patch.source().path().get(), patch); + } + } + + var sorted = new ArrayList(); + for (var file : files) { + if (byTargetPath.containsKey(file)) { + sorted.add(byTargetPath.get(file)); + } else if (bySourcePath.containsKey(file)) { + sorted.add(bySourcePath.get(file)); + } else { + throw new IOException("Filename not present in diff: " + file); + } + } + patches = sorted; + } var modified = new ArrayList(); for (var i = 0; i < patches.size(); i++) {