Skip to content

Commit 4e9d8bc

Browse files
committed
more thorough ctags check
actually run ctags on a known temporary file under source root to see if ctags can extract some symbols out of it. fixes #4125
1 parent 3b47b2e commit 4e9d8bc

File tree

10 files changed

+509
-108
lines changed

10 files changed

+509
-108
lines changed

opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/Ctags.java

Lines changed: 53 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved.
2222
* Portions Copyright (c) 2017, 2020, Chris Fraire <cfraire@me.com>.
2323
*/
2424
package org.opengrok.indexer.analysis;
@@ -35,7 +35,9 @@
3535
import java.time.Duration;
3636
import java.util.ArrayList;
3737
import java.util.Collections;
38+
import java.util.HashSet;
3839
import java.util.List;
40+
import java.util.Set;
3941
import java.util.concurrent.ExecutionException;
4042
import java.util.concurrent.ExecutorService;
4143
import java.util.concurrent.Future;
@@ -48,6 +50,7 @@
4850
import org.opengrok.indexer.configuration.RuntimeEnvironment;
4951
import org.opengrok.indexer.index.IndexerParallelizer;
5052
import org.opengrok.indexer.logger.LoggerFactory;
53+
import org.opengrok.indexer.util.CtagsUtil;
5154
import org.opengrok.indexer.util.Executor;
5255
import org.opengrok.indexer.util.IOUtils;
5356
import org.opengrok.indexer.util.SourceSplitter;
@@ -65,15 +68,17 @@ public class Ctags implements Resettable {
6568
private volatile boolean closing;
6669
private LangMap langMap;
6770
private List<String> command;
68-
private Process ctags;
71+
private Process ctagsProcess;
6972
private OutputStreamWriter ctagsIn;
7073
private BufferedReader ctagsOut;
7174
private static final String CTAGS_FILTER_TERMINATOR = "__ctags_done_with_file__";
72-
private String CTagsExtraOptionsFile = null;
75+
private String cTagsExtraOptionsFile = null;
7376
private int tabSize;
7477
private Duration timeout = Duration.ofSeconds(10);
7578

76-
private boolean junit_testing = false;
79+
private final Set<String> ctagsLanguages = new HashSet<>();
80+
81+
private boolean junitTesting = false;
7782

7883
/**
7984
* Initializes an instance with the current
@@ -82,16 +87,15 @@ public class Ctags implements Resettable {
8287
public Ctags() {
8388
env = RuntimeEnvironment.getInstance();
8489
langMap = AnalyzerGuru.getLangMap();
90+
cTagsExtraOptionsFile = env.getCTagsExtraOptionsFile();
8591
}
8692

8793
/**
88-
* Gets a value indicating if a subprocess of ctags was started and it is
89-
* not alive.
90-
* @return {@code true} if the instance should be considered closed and no
91-
* longer usable.
94+
* Gets a value indicating if a subprocess of ctags was started, and it is not alive.
95+
* @return {@code true} if the instance should be considered closed and no longer usable.
9296
*/
9397
public boolean isClosed() {
94-
return ctags != null && !ctags.isAlive();
98+
return ctagsProcess != null && !ctagsProcess.isAlive();
9599
}
96100

97101
public void setLangMap(LangMap langMap) {
@@ -107,7 +111,7 @@ public void setTabSize(int tabSize) {
107111
}
108112

109113
public void setCTagsExtraOptionsFile(String ctagsExtraOptionsFile) {
110-
this.CTagsExtraOptionsFile = ctagsExtraOptionsFile;
114+
this.cTagsExtraOptionsFile = ctagsExtraOptionsFile;
111115
}
112116

113117
public void setTimeout(long timeout) {
@@ -133,10 +137,10 @@ public void reset() {
133137
public void close() {
134138
reset();
135139
IOUtils.close(ctagsIn);
136-
if (ctags != null) {
140+
if (ctagsProcess != null) {
137141
closing = true;
138142
LOGGER.log(Level.FINE, "Destroying ctags command");
139-
ctags.destroyForcibly();
143+
ctagsProcess.destroyForcibly();
140144
}
141145
}
142146

@@ -150,15 +154,19 @@ public List<String> getArgv() {
150154
}
151155

152156
private void initialize() {
153-
/*
154-
* Call the following principally to properly initialize when running
155-
* JUnit tests. opengrok-indexer and opengrok-web call it too but
156-
* validating its return code and logging (and possibly aborting).
157-
*/
158-
env.validateUniversalCtags();
159-
160157
command = new ArrayList<>();
161-
command.add(env.getCtags());
158+
String ctagsCommand = env.getCtags();
159+
command.add(ctagsCommand);
160+
161+
// Normally, the indexer or the webapp will call validateUniversalCtags()
162+
// that would set the set of ctags languages returned by env.getCtagsLanguages(),
163+
// however for tests this might not be always the case so do it here.
164+
if (env.getCtagsLanguages().isEmpty()) {
165+
ctagsLanguages.addAll(CtagsUtil.getLanguages(ctagsCommand));
166+
} else {
167+
ctagsLanguages.addAll(env.getCtagsLanguages());
168+
}
169+
162170
command.add("--kinds-c=+l");
163171

164172
// Workaround for bug #14924: Don't get local variables in Java
@@ -203,9 +211,9 @@ private void initialize() {
203211
}
204212

205213
/* Add extra command line options for ctags. */
206-
if (CTagsExtraOptionsFile != null) {
214+
if (cTagsExtraOptionsFile != null) {
207215
LOGGER.log(Level.FINER, "Adding extra options to ctags");
208-
command.add("--options=" + CTagsExtraOptionsFile);
216+
command.add("--options=" + cTagsExtraOptionsFile);
209217
}
210218
}
211219

@@ -215,20 +223,19 @@ private void run() throws IOException {
215223

216224
ProcessBuilder processBuilder = new ProcessBuilder(command);
217225

218-
ctags = processBuilder.start();
226+
ctagsProcess = processBuilder.start();
219227
ctagsIn = new OutputStreamWriter(
220-
ctags.getOutputStream(), StandardCharsets.UTF_8);
221-
ctagsOut = new BufferedReader(new InputStreamReader(ctags.getInputStream(),
228+
ctagsProcess.getOutputStream(), StandardCharsets.UTF_8);
229+
ctagsOut = new BufferedReader(new InputStreamReader(ctagsProcess.getInputStream(),
222230
StandardCharsets.UTF_8));
223231

224232
Thread errThread = new Thread(() -> {
225-
try (BufferedReader error = new BufferedReader(new InputStreamReader(ctags.getErrorStream(),
233+
try (BufferedReader error = new BufferedReader(new InputStreamReader(ctagsProcess.getErrorStream(),
226234
StandardCharsets.UTF_8))) {
227235
String s;
228236
while ((s = error.readLine()) != null) {
229237
if (s.length() > 0) {
230-
LOGGER.log(Level.WARNING, "Error from ctags: {0}",
231-
s);
238+
LOGGER.log(Level.WARNING, "Error from ctags: {0}", s);
232239
}
233240
if (closing) {
234241
break;
@@ -243,7 +250,7 @@ private void run() throws IOException {
243250
}
244251

245252
private void addRustSupport(List<String> command) {
246-
if (!env.getCtagsLanguages().contains("Rust")) { // Built-in would be capitalized.
253+
if (!ctagsLanguages.contains("Rust")) { // Built-in would be capitalized.
247254
command.add("--langdef=rust"); // Lower-case if user-defined.
248255
}
249256

@@ -262,7 +269,7 @@ private void addRustSupport(List<String> command) {
262269
}
263270

264271
private void addPowerShellSupport(List<String> command) {
265-
if (!env.getCtagsLanguages().contains("PowerShell")) { // Built-in would be capitalized.
272+
if (!ctagsLanguages.contains("PowerShell")) { // Built-in would be capitalized.
266273
command.add("--langdef=powershell"); // Lower-case if user-defined.
267274
}
268275

@@ -282,7 +289,7 @@ private void addPowerShellSupport(List<String> command) {
282289
}
283290

284291
private void addPascalSupport(List<String> command) {
285-
if (!env.getCtagsLanguages().contains("Pascal")) { // Built-in would be capitalized.
292+
if (!ctagsLanguages.contains("Pascal")) { // Built-in would be capitalized.
286293
command.add("--langdef=pascal"); // Lower-case if user-defined.
287294
}
288295

@@ -307,7 +314,7 @@ private void addPascalSupport(List<String> command) {
307314
}
308315

309316
private void addSwiftSupport(List<String> command) {
310-
if (!env.getCtagsLanguages().contains("Swift")) { // Built-in would be capitalized.
317+
if (!ctagsLanguages.contains("Swift")) { // Built-in would be capitalized.
311318
command.add("--langdef=swift"); // Lower-case if user-defined.
312319
}
313320
command.add("--kinddef-swift=n,enum,Enums");
@@ -330,7 +337,7 @@ private void addSwiftSupport(List<String> command) {
330337
}
331338

332339
private void addKotlinSupport(List<String> command) {
333-
if (!env.getCtagsLanguages().contains("Kotlin")) { // Built-in would be capitalized.
340+
if (!ctagsLanguages.contains("Kotlin")) { // Built-in would be capitalized.
334341
command.add("--langdef=kotlin"); // Lower-case if user-defined.
335342
}
336343
command.add("--kinddef-kotlin=d,dataClass,Data\\ classes");
@@ -360,7 +367,7 @@ private void addKotlinSupport(List<String> command) {
360367
* Override Clojure support with patterns from https://gist.github.com/kul/8704283.
361368
*/
362369
private void addClojureSupport(List<String> command) {
363-
if (!env.getCtagsLanguages().contains("Clojure")) { // Built-in would be capitalized.
370+
if (!ctagsLanguages.contains("Clojure")) { // Built-in would be capitalized.
364371
command.add("--langdef=clojure"); // Lower-case if user-defined.
365372
}
366373
command.add("--kinddef-clojure=d,definition,Definitions");
@@ -386,7 +393,7 @@ private void addClojureSupport(List<String> command) {
386393
}
387394

388395
private void addHaskellSupport(List<String> command) {
389-
if (!env.getCtagsLanguages().contains("Haskell")) { // Built-in would be capitalized.
396+
if (!ctagsLanguages.contains("Haskell")) { // Built-in would be capitalized.
390397
command.add("--langdef=haskell"); // below added with #912. Lowercase if user-defined.
391398
}
392399

@@ -401,7 +408,7 @@ private void addHaskellSupport(List<String> command) {
401408
}
402409

403410
private void addScalaSupport(List<String> command) {
404-
if (!env.getCtagsLanguages().contains("Scala")) { // Built-in would be capitalized.
411+
if (!ctagsLanguages.contains("Scala")) { // Built-in would be capitalized.
405412
command.add("--langdef=scala"); // below is bug 61 to get full scala support. Lower-case
406413
}
407414
command.add("--kinddef-scala=c,class,Classes");
@@ -436,7 +443,7 @@ private void addScalaSupport(List<String> command) {
436443
}
437444

438445
private void addTerraformSupport(List<String> command) {
439-
if (!env.getCtagsLanguages().contains("Terraform")) { // Built-in would be capitalized.
446+
if (!ctagsLanguages.contains("Terraform")) { // Built-in would be capitalized.
440447
command.add("--langdef=terraform"); // Lower-case if user-defined.
441448
}
442449

@@ -494,9 +501,9 @@ public Definitions doCtags(String file) throws IOException, InterruptedException
494501
return null;
495502
}
496503

497-
if (ctags != null) {
504+
if (ctagsProcess != null) {
498505
try {
499-
int exitValue = ctags.exitValue();
506+
int exitValue = ctagsProcess.exitValue();
500507
// If it is possible to retrieve exit value without exception
501508
// this means the ctags process is dead.
502509
LOGGER.log(Level.WARNING, "Ctags process exited with exit value {0}",
@@ -583,9 +590,9 @@ Definitions testCtagsParser(String bufferTags)
583590
tagsBuilder.append("\n");
584591
bufferTags = tagsBuilder.toString();
585592

586-
junit_testing = true;
593+
junitTesting = true;
587594
ctagsOut = new BufferedReader(new StringReader(bufferTags));
588-
ctags = new Process() {
595+
ctagsProcess = new Process() {
589596
@Override
590597
public OutputStream getOutputStream() {
591598
return null;
@@ -632,20 +639,20 @@ private void readTags(CtagsReader reader) throws InterruptedException {
632639

633640
//log.fine("Tagline:-->" + tagLine+"<----ONELINE");
634641
if (tagLine == null) {
635-
if (!junit_testing) {
642+
if (!junitTesting) {
636643
LOGGER.warning("ctags: Unexpected end of file!");
637644
}
638645
try {
639-
int val = ctags.exitValue();
640-
if (!junit_testing) {
646+
int val = ctagsProcess.exitValue();
647+
if (!junitTesting) {
641648
LOGGER.log(Level.WARNING, "ctags exited with code: {0}", val);
642649
}
643650
} catch (IllegalThreadStateException e) {
644651
LOGGER.log(Level.WARNING, "ctags EOF but did not exit");
645-
ctags.destroyForcibly();
652+
ctagsProcess.destroyForcibly();
646653
} catch (Exception e) {
647654
LOGGER.log(Level.WARNING, "ctags problem:", e);
648-
ctags.destroyForcibly();
655+
ctagsProcess.destroyForcibly();
649656
}
650657
// Throw the following to indicate non-I/O error for retry.
651658
throw new InterruptedException("tagLine == null");

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/RuntimeEnvironment.java

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2006, 2023, Oracle and/or its affiliates. All rights reserved.
2222
* Portions Copyright (c) 2017, 2020, Chris Fraire <cfraire@me.com>.
2323
*/
2424
package org.opengrok.indexer.configuration;
@@ -586,8 +586,7 @@ public String getUrlPrefix() {
586586
* Gets the name of the ctags program to use: either the last value passed
587587
* successfully to {@link #setCtags(java.lang.String)}, or
588588
* {@link Configuration#getCtags()}, or the system property for
589-
* {@code "org.opengrok.indexer.analysis.Ctags"}, or "ctags" as a
590-
* default.
589+
* {@code "org.opengrok.indexer.analysis.Ctags"}, or "{@code ctags}" as a default.
591590
* @return a defined value
592591
*/
593592
public String getCtags() {
@@ -663,35 +662,42 @@ public void setHitsPerPage(int hitsPerPage) {
663662
}
664663

665664
/**
666-
* Validate that there is a Universal ctags program.
665+
* Validate that there is a Universal ctags program that can actually process input files
666+
* under source root.
667+
* <br>
668+
* As a side effect, this sets the set of ctags languages that is used by the ctags program.
667669
*
668670
* @return true if success, false otherwise
669671
*/
670672
public boolean validateUniversalCtags() {
671673
if (ctagsFound == null) {
672-
String ctagsBinary = getCtags();
673674
try (ResourceLock resourceLock = configLock.writeLockAsResource()) {
674675
//noinspection ConstantConditions to avoid warning of no reference to auto-closeable
675676
assert resourceLock != null;
676677
if (ctagsFound == null) {
677-
ctagsFound = CtagsUtil.validate(ctagsBinary);
678-
if (ctagsFound) {
679-
List<String> languages = CtagsUtil.getLanguages(ctagsBinary);
680-
if (languages != null) {
681-
ctagsLanguages.addAll(languages);
682-
}
678+
String ctagsBinary = getCtags();
679+
if (ctagsLanguages.isEmpty()) {
680+
// The ctagsLanguages are necessary for the call to validate() below.
681+
Set<String> languages = CtagsUtil.getLanguages(ctagsBinary);
682+
ctagsLanguages.addAll(languages);
683683
}
684+
685+
ctagsFound = CtagsUtil.validate(ctagsBinary);
684686
}
685687
}
686688
}
689+
690+
if (ctagsFound) {
691+
LOGGER.log(Level.INFO, "Using ctags: {0}", CtagsUtil.getCtagsVersion(getCtags()));
692+
}
693+
687694
return ctagsFound;
688695
}
689696

690697
/**
691698
* Gets the base set of supported Ctags languages.
692699
* @return a defined set which may be empty if
693-
* {@link #validateUniversalCtags()} has not yet been called or if the call
694-
* fails
700+
* {@link #validateUniversalCtags()} has not yet been called or if the call fails
695701
*/
696702
public Set<String> getCtagsLanguages() {
697703
return Collections.unmodifiableSet(ctagsLanguages);
@@ -1114,6 +1120,10 @@ public void setWebappLAF(String webappLAF) {
11141120
syncWriteConfiguration(webappLAF, Configuration::setWebappLAF);
11151121
}
11161122

1123+
public void setWebappCtags(boolean ctags) {
1124+
syncWriteConfiguration(ctags, Configuration::setWebappCtags);
1125+
}
1126+
11171127
/**
11181128
* Gets a value indicating if the web app should run ctags as necessary.
11191129
* @return the value of {@link Configuration#isWebappCtags()}
@@ -1757,10 +1767,10 @@ public void applyConfig(String configuration, boolean reindex, CommandTimeoutTyp
17571767
* Set configuration from the incoming parameter. The configuration could
17581768
* have come from the Indexer (in which case some extra work is needed) or
17591769
* is it just a request to set new configuration in place.
1760-
*
1770+
* <p>
17611771
* The classes that have registered their listener will be pinged here.
17621772
* @see ConfigurationChangedListener
1763-
*
1773+
* </p>
17641774
* @param config the incoming configuration
17651775
* @param reindex is the message result of reindex
17661776
* @param cmdType command timeout type

0 commit comments

Comments
 (0)