Skip to content

Commit 6a2061f

Browse files
idodeclareVladimir Kotal
idodeclare
authored and
Vladimir Kotal
committed
Handle git-log merge commits and use -m
Also: - Add FileHistoryCacheOctopusTest showing dupes for merges (before revising for this patch) - Update parsing of Git revision with labels - Fix #3166 "partial parse of git log"
1 parent e88d39d commit 6a2061f

File tree

12 files changed

+481
-62
lines changed

12 files changed

+481
-62
lines changed

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryCache.java

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
/*
2121
* Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved.
22-
* Portions Copyright (c) 2018, Chris Fraire <cfraire@me.com>.
22+
* Portions Copyright (c) 2018-2019, Chris Fraire <cfraire@me.com>.
2323
*/
2424

2525
package org.opengrok.indexer.history;
@@ -288,10 +288,8 @@ private void writeHistoryToFile(File dir, History history, File cacheFile) throw
288288
* @param histNew history object with new history entries
289289
* @param repo repository to where pre pre-image of the cacheFile belong
290290
* @return merged history (can be null if merge failed for some reason)
291-
* @throws HistoryException
292291
*/
293-
private History mergeOldAndNewHistory(File cacheFile, History histNew, Repository repo)
294-
throws HistoryException {
292+
private History mergeOldAndNewHistory(File cacheFile, History histNew, Repository repo) {
295293

296294
History histOld;
297295
History history = null;
@@ -338,7 +336,6 @@ private History mergeOldAndNewHistory(File cacheFile, History histNew, Repositor
338336
* @param repo repository for the file
339337
* @param mergeHistory whether to merge the history with existing or
340338
* store the histNew as is
341-
* @throws HistoryException
342339
*/
343340
private void storeFile(History histNew, File file, Repository repo,
344341
boolean mergeHistory) throws HistoryException {
@@ -364,11 +361,11 @@ private void storeFile(History histNew, File file, Repository repo,
364361

365362
// If the merge failed, null history will be returned.
366363
// In such case store at least new history as a best effort.
367-
if (history != null) {
368-
writeHistoryToFile(dir, history, cacheFile);
369-
} else {
370-
writeHistoryToFile(dir, histNew, cacheFile);
364+
if (history == null) {
365+
history = histNew;
371366
}
367+
368+
writeHistoryToFile(dir, history, cacheFile);
372369
}
373370

374371
private void storeFile(History histNew, File file, Repository repo)

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitHistoryParser.java

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
/*
2121
* Copyright (c) 2007, 2018, Oracle and/or its affiliates. All rights reserved.
22-
* Portions Copyright (c) 2017, Chris Fraire <cfraire@me.com>.
22+
* Portions Copyright (c) 2017, 2019, Chris Fraire <cfraire@me.com>.
2323
*/
2424
package org.opengrok.indexer.history;
2525

@@ -30,6 +30,7 @@
3030
import java.io.IOException;
3131
import java.io.InputStream;
3232
import java.io.InputStreamReader;
33+
import java.nio.charset.StandardCharsets;
3334
import java.nio.file.InvalidPathException;
3435
import java.text.ParseException;
3536
import java.util.ArrayList;
@@ -57,16 +58,25 @@ private enum ParseState {
5758
private String myDir;
5859
private GitRepository repository = new GitRepository();
5960
private List<HistoryEntry> entries = new ArrayList<>();
61+
private History history;
6062

6163
private final boolean handleRenamedFiles;
6264

6365
GitHistoryParser(boolean flag) {
6466
handleRenamedFiles = flag;
6567
}
66-
68+
6769
/**
68-
* Process the output from the log command and insert the HistoryEntries
69-
* into the history field.
70+
* Gets the instance from the most recent processing or parsing.
71+
* @return a defined instance or {@code null} if not yet processed
72+
*/
73+
public History getHistory() {
74+
return history;
75+
}
76+
77+
/**
78+
* Process the output from the log command, and insert the
79+
* {@link HistoryEntry} instances to the {@link #getHistory()} property.
7080
*
7181
* @param input The output from the process
7282
* @throws java.io.IOException If an error occurs while reading the stream
@@ -76,6 +86,7 @@ public void processStream(InputStream input) throws IOException {
7686
try (BufferedReader in = new BufferedReader(GitRepository.newLogReader(input))) {
7787
process(in);
7888
}
89+
history = new History(entries);
7990
}
8091

8192
private void process(BufferedReader in) throws IOException {
@@ -88,12 +99,39 @@ private void process(BufferedReader in) throws IOException {
8899
if (state == ParseState.HEADER) {
89100

90101
if (s.startsWith("commit")) {
102+
String commit = s.substring("commit".length()).trim();
103+
104+
/*
105+
* Git might show branch labels for a commit. E.g.
106+
* commit 3595fbc9 (HEAD -> master, origin/master, origin/HEAD)
107+
* or it might show a merge parent. E.g.
108+
* commit 4c3d5e8e (from 06b00dcb)
109+
* So trim those off too.
110+
*/
111+
int offset = commit.indexOf(' ');
112+
if (offset >= 1) {
113+
commit = commit.substring(0, offset);
114+
}
115+
91116
if (entry != null) {
92-
entries.add(entry);
117+
/*
118+
* With -m, a commit hash repeats for each merged head.
119+
* For OpenGrok's purposes, the same HistoryEntry would
120+
* get reused for each head. If the commit hash differs,
121+
* then add the entry to the list, and prepare to start
122+
* a new entry.
123+
*/
124+
if (commit.equals(entry.getRevision())) {
125+
entry.setMessage(""); // Avoid message repetition.
126+
} else {
127+
entries.add(entry);
128+
entry = null;
129+
}
130+
}
131+
if (entry == null) {
132+
entry = new HistoryEntry();
93133
}
94-
entry = new HistoryEntry();
95134
entry.setActive(true);
96-
String commit = s.substring("commit".length()).trim();
97135
entry.setRevision(commit);
98136
} else if (s.startsWith("Author:") && entry != null) {
99137
entry.setAuthor(s.substring("Author:".length()).trim());
@@ -210,14 +248,14 @@ History parse(File file, Repository repos, String sinceRevision) throws HistoryE
210248
*/
211249
History parse(String buffer) throws IOException {
212250
myDir = RuntimeEnvironment.getInstance().getSourceRootPath();
213-
processStream(new ByteArrayInputStream(buffer.getBytes("UTF-8")));
251+
processStream(new ByteArrayInputStream(buffer.getBytes(StandardCharsets.UTF_8)));
214252
return new History(entries);
215253
}
216254

217255
/**
218256
* Class for handling renamed files stream.
219257
*/
220-
private class RenamedFilesParser implements Executor.StreamHandler {
258+
private static class RenamedFilesParser implements Executor.StreamHandler {
221259

222260
private final List<String> renamedFiles = new ArrayList<>();
223261

@@ -262,8 +300,6 @@ public void processStream(InputStream input) throws IOException {
262300
* A foo2.f
263301
* A main.c
264302
*/
265-
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
266-
267303
try (BufferedReader in = new BufferedReader(new InputStreamReader(input))) {
268304
String line;
269305
Pattern pattern = Pattern.compile("^R\\d+\\s.*");

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,8 @@ Executor getHistoryLogExecutor(final File file, String sinceRevision)
130130
cmd.add("--name-only");
131131
cmd.add("--pretty=fuller");
132132
cmd.add(GIT_DATE_OPT);
133-
133+
cmd.add("-m");
134+
134135
if (file.isFile() && isHandleRenamedFiles()) {
135136
cmd.add("--follow");
136137
}

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/History.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
/*
2121
* Copyright (c) 2007, 2018, Oracle and/or its affiliates. All rights reserved.
22+
* Portions Copyright (c) 2019, Chris Fraire <cfraire@me.com>.
2223
*/
2324

2425
package org.opengrok.indexer.history;
@@ -115,11 +116,15 @@ public boolean hasTags() {
115116
}
116117
return false;
117118
}
118-
119+
120+
/**
121+
* Gets a value indicating if {@code file} is in the list of renamed files.
122+
* TODO: Warning -- this does a slow {@link List} search.
123+
*/
119124
public boolean isRenamed(String file) {
120125
return renamedFiles.contains(file);
121126
}
122-
127+
123128
public List<String> getRenamedFiles() {
124129
return renamedFiles;
125130
}

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
*/
2424
package org.opengrok.indexer.history;
2525

26+
import static org.opengrok.indexer.history.HistoryEntry.TAGS_SEPARATOR;
27+
2628
import java.io.ByteArrayInputStream;
2729
import java.io.ByteArrayOutputStream;
2830
import java.io.File;
@@ -278,12 +280,12 @@ TreeSet<TagEntry> getTagList() {
278280
* @see getTagList
279281
* @param hist History we want to assign tags to.
280282
*/
281-
void assignTagsInHistory(History hist) throws HistoryException {
283+
void assignTagsInHistory(History hist) {
282284
if (hist == null) {
283285
return;
284286
}
285287
if (this.getTagList() == null) {
286-
throw new HistoryException("Tag list was not created before assigning tags to changesets!");
288+
throw new IllegalStateException("getTagList() is null");
287289
}
288290
Iterator<TagEntry> it = this.getTagList().descendingIterator();
289291
TagEntry lastTagEntry = null;
@@ -301,7 +303,7 @@ void assignTagsInHistory(History hist) throws HistoryException {
301303
if (ent.getTags() == null) {
302304
ent.setTags(lastTagEntry.getTags());
303305
} else {
304-
ent.setTags(ent.getTags() + ", " + lastTagEntry.getTags());
306+
ent.setTags(ent.getTags() + TAGS_SEPARATOR + lastTagEntry.getTags());
305307
}
306308
} else {
307309
break;

‎opengrok-indexer/src/test/java/org/opengrok/indexer/history/FileHistoryCacheOctopusTest.java

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* CDDL HEADER START
3+
*
4+
* The contents of this file are subject to the terms of the
5+
* Common Development and Distribution License (the "License").
6+
* You may not use this file except in compliance with the License.
7+
*
8+
* See LICENSE.txt included in this distribution for the specific
9+
* language governing permissions and limitations under the License.
10+
*
11+
* When distributing Covered Code, include this CDDL HEADER in each
12+
* file and include the License file at LICENSE.txt.
13+
* If applicable, add the following below this CDDL HEADER, with the
14+
* fields enclosed by brackets "[]" replaced with your own identifying
15+
* information: Portions Copyright [yyyy] [name of copyright owner]
16+
*
17+
* CDDL HEADER END
18+
*/
19+
20+
/*
21+
* Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
22+
* Portions Copyright (c) 2018-2019, Chris Fraire <cfraire@me.com>.
23+
*/
24+
package org.opengrok.indexer.history;
25+
26+
import static org.junit.Assert.assertEquals;
27+
import static org.junit.Assert.assertNotNull;
28+
29+
import org.junit.After;
30+
import org.junit.Before;
31+
import org.junit.Rule;
32+
import org.junit.Test;
33+
import org.opengrok.indexer.condition.ConditionalRun;
34+
import org.opengrok.indexer.condition.ConditionalRunRule;
35+
import org.opengrok.indexer.condition.RepositoryInstalled;
36+
import org.opengrok.indexer.util.TestRepository;
37+
38+
import java.io.File;
39+
import java.util.List;
40+
41+
/**
42+
* Test file-based history cache with for git-octopus.
43+
* @author Vladimir Kotal
44+
*/
45+
public class FileHistoryCacheOctopusTest {
46+
47+
private TestRepository repositories;
48+
private FileHistoryCache cache;
49+
50+
@Rule
51+
public ConditionalRunRule rule = new ConditionalRunRule();
52+
53+
@Before
54+
public void setUp() throws Exception {
55+
repositories = new TestRepository();
56+
repositories.create(getClass().getResourceAsStream("/history/git-octopus.zip"));
57+
58+
cache = new FileHistoryCache();
59+
cache.initialize();
60+
}
61+
62+
@After
63+
public void tearDown() {
64+
repositories.destroy();
65+
repositories = null;
66+
67+
cache = null;
68+
}
69+
70+
@ConditionalRun(RepositoryInstalled.GitInstalled.class)
71+
@Test
72+
public void testStoreAndGet() throws Exception {
73+
File reposRoot = new File(repositories.getSourceRoot(), "git-octopus");
74+
Repository repo = RepositoryFactory.getRepository(reposRoot);
75+
History historyToStore = repo.getHistory(reposRoot);
76+
77+
cache.store(historyToStore, repo);
78+
79+
assertEquals("latest git-octopus commit", "206f862b",
80+
cache.getLatestCachedRevision(repo));
81+
82+
History dHist = cache.get(new File(reposRoot, "d"), repo, true);
83+
assertNotNull("cache get() for git-octopus/d", dHist);
84+
85+
List<HistoryEntry> entries = dHist.getHistoryEntries();
86+
String firstMessage = entries.get(0).getMessage();
87+
String lastMessage = entries.get(entries.size() - 1).getMessage();
88+
assertEquals("first message equals last", firstMessage, lastMessage);
89+
assertEquals("Merge branches 'file_a', 'file_b' and 'file_c' into master, and add d",
90+
firstMessage);
91+
assertEquals("git-octopus/d has one cached log", 1, entries.size());
92+
}
93+
}

0 commit comments

Comments
 (0)