Skip to content

handle symlinks for history based reindex #4532

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 12, 2024
Merged

handle symlinks for history based reindex #4532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
13fc2a3
handle symlinks for history based reindex
vladak Jan 17, 2024
265adf4
deal with Windows
vladak Jan 17, 2024
914c6da
more enclosing of paths
vladak Jan 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 62 additions & 43 deletions opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexDatabase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

/*
* Copyright (c) 2008, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights reserved.
* Portions Copyright (c) 2017, 2020, Chris Fraire <cfraire@me.com>.
*/
package org.opengrok.indexer.index;
Expand Down Expand Up @@ -800,7 +800,7 @@ public void update() throws IOException {

/**
* The traversal of the uid terms done in {@link #processFile(IndexDownArgs, File, String)}
* and {@link #processFileIncremental(IndexDownArgs, File, String)} needs to skip over deleted documents
* and {@link #processFileHistoryBased(IndexDownArgs, File, String)} needs to skip over deleted documents
* that are often found in multi-segment indexes. This method stores the uids of these documents
* and is expected to be called before the traversal for the top level directory is started.
* @throws IOException if the index cannot be read for some reason
Expand Down Expand Up @@ -932,8 +932,11 @@ void indexDownUsingHistory(File sourceRoot, IndexDownArgs args) throws IOExcepti
try (Progress progress = new Progress(LOGGER, String.format("collecting files for %s", project),
fileCollector.getFiles().size())) {
for (String path : fileCollector.getFiles()) {
if (isInterrupted()) {
return;
}
File file = new File(sourceRoot, path);
processFileIncremental(args, file, path);
processFileHistoryBased(args, file, path);
progress.increment();
}
}
Expand Down Expand Up @@ -1279,6 +1282,7 @@ private static void cleanupResources(Document doc) {

/**
* Check if I should accept this file into the index database.
* Directories are automatically accepted.
*
* @param file the file to check
* @param ret defined instance whose {@code localRelPath} property will be
Expand All @@ -1297,53 +1301,49 @@ private boolean accept(File file, AcceptSymlinkRet ret) {
}

if (!file.canRead()) {
LOGGER.log(Level.WARNING, "Could not read {0}", absolutePath);
LOGGER.log(Level.WARNING, "Could not read ''{0}''", absolutePath);
return false;
}

try {
Path absolute = Paths.get(absolutePath);
if (Files.isSymbolicLink(absolute)) {
File canonical = file.getCanonicalFile();
if (!absolutePath.equals(canonical.getPath()) &&
!acceptSymlink(absolute, canonical, ret)) {
if (!absolutePath.equals(canonical.getPath()) && !acceptSymlink(absolute, canonical, ret)) {
if (ret.localRelPath == null) {
LOGGER.log(Level.FINE, "Skipped symlink ''{0}'' -> ''{1}''",
new Object[] {absolutePath, canonical});
}
return false;
}
}
//below will only let go files and directories, anything else is considered special and is not added
// Below will only let go files and directories, anything else is considered special and is not added.
if (!file.isFile() && !file.isDirectory()) {
LOGGER.log(Level.WARNING, "Ignored special file {0}",
absolutePath);
LOGGER.log(Level.WARNING, "Ignored special file ''{0}''", absolutePath);
return false;
}
} catch (IOException exp) {
LOGGER.log(Level.WARNING, "Failed to resolve name: {0}",
absolutePath);
LOGGER.log(Level.WARNING, "Failed to resolve name: ''{0}''", absolutePath);
LOGGER.log(Level.FINE, "Stack Trace: ", exp);
}

if (file.isDirectory()) {
// always accept directories so that their files can be examined
// Always accept directories so that their files can be examined.
return true;
}


RuntimeEnvironment env = RuntimeEnvironment.getInstance();
// Lookup history if indexing versioned files only.
// Skip the lookup entirely (which is expensive) if unversioned files are allowed
if (env.isIndexVersionedFilesOnly()) {
if (HistoryGuru.getInstance().hasHistory(file)) {
// versioned files should always be accepted
// Versioned files should always be accepted.
return true;
}
LOGGER.log(Level.FINER, "not accepting unversioned {0}", absolutePath);
return false;
}
// unversioned files are allowed
// Unversioned files are allowed.
return true;
}

Expand All @@ -1365,7 +1365,7 @@ private boolean accept(File parent, File file, AcceptSymlinkRet ret) {
File f1 = parent.getCanonicalFile();
File f2 = file.getCanonicalFile();
if (f1.equals(f2)) {
LOGGER.log(Level.INFO, "Skipping links to itself...: {0} {1}",
LOGGER.log(Level.INFO, "Skipping links to itself...: ''{0}'' ''{1}''",
new Object[]{parent.getAbsolutePath(), file.getAbsolutePath()});
return false;
}
Expand All @@ -1374,15 +1374,15 @@ private boolean accept(File parent, File file, AcceptSymlinkRet ret) {
File t1 = f1;
while ((t1 = t1.getParentFile()) != null) {
if (f2.equals(t1)) {
LOGGER.log(Level.INFO, "Skipping links to parent...: {0} {1}",
LOGGER.log(Level.INFO, "Skipping links to parent...: ''{0}'' ''{1}''",
new Object[]{parent.getAbsolutePath(), file.getAbsolutePath()});
return false;
}
}

return accept(file, ret);
} catch (IOException ex) {
LOGGER.log(Level.WARNING, "Failed to resolve name: {0} {1}",
LOGGER.log(Level.WARNING, "Failed to resolve name: ''{0}'' ''{1}''",
new Object[]{parent.getAbsolutePath(), file.getAbsolutePath()});
}
return false;
Expand Down Expand Up @@ -1414,7 +1414,7 @@ private boolean acceptSymlink(Path absolute, File canonical, AcceptSymlinkRet re
if (isLocal(canonical1)) {
if (!isCanonicalDir) {
if (LOGGER.isLoggable(Level.FINEST)) {
LOGGER.log(Level.FINEST, "Local {0} has symlink from {1}",
LOGGER.log(Level.FINEST, "Local ''{0}'' has symlink from ''{1}''",
new Object[] {canonical1, absolute1});
}
/*
Expand Down Expand Up @@ -1443,7 +1443,7 @@ private boolean acceptSymlink(Path absolute, File canonical, AcceptSymlinkRet re
* the file already -- but we are forced to handle.
*/
LOGGER.log(Level.WARNING, String.format(
"Unexpected error getting relative for %s", canonical), e);
"Unexpected error getting relative for '%s'", canonical), e);
absolute0 = absolute1;
}
indexed1 = new IndexedSymlink(absolute0, canonical1, true);
Expand All @@ -1468,7 +1468,7 @@ private boolean acceptSymlink(Path absolute, File canonical, AcceptSymlinkRet re
Paths.get(indexed0.getAbsolute())).toString();

if (LOGGER.isLoggable(Level.FINEST)) {
LOGGER.log(Level.FINEST, "External dir {0} has symlink from {1} after first {2}",
LOGGER.log(Level.FINEST, "External dir ''{0}'' has symlink from ''{1}'' after first ''{2}''",
new Object[] {canonical1, absolute1, indexed0.getAbsolute()});
}
return false;
Expand All @@ -1486,7 +1486,7 @@ private boolean acceptSymlink(Path absolute, File canonical, AcceptSymlinkRet re
if (!isCanonicalDir) {
if (LOGGER.isLoggable(Level.FINEST)) {
LOGGER.log(Level.FINEST,
"External file {0} has symlink from {1} under previous {2}",
"External file ''{0}'' has symlink from ''{1}'' under previous ''{2}''",
new Object[] {canonical1, absolute1, absolute0});
}
// Do not add to indexedSymlinks for a non-directory.
Expand All @@ -1503,7 +1503,7 @@ private boolean acceptSymlink(Path absolute, File canonical, AcceptSymlinkRet re

if (LOGGER.isLoggable(Level.FINEST)) {
LOGGER.log(Level.FINEST,
"External dir {0} has symlink from {1} under previous {2}",
"External dir ''{0}'' has symlink from ''{1}'' under previous ''{2}''",
new Object[] {canonical1, absolute1, absolute0});
}
return false;
Expand All @@ -1514,7 +1514,7 @@ private boolean acceptSymlink(Path absolute, File canonical, AcceptSymlinkRet re
for (String canonicalRoot : canonicalRoots) {
if (canonical1.startsWith(canonicalRoot)) {
if (LOGGER.isLoggable(Level.FINEST)) {
LOGGER.log(Level.FINEST, "Allowed symlink {0} per canonical root {1}",
LOGGER.log(Level.FINEST, "Allowed symlink ''{0}'' per canonical root ''{1}''",
new Object[] {absolute1, canonical1});
}
if (isCanonicalDir) {
Expand All @@ -1531,7 +1531,7 @@ private boolean acceptSymlink(Path absolute, File canonical, AcceptSymlinkRet re
try {
allowedTarget = new File(allowedSymlink).getCanonicalPath();
} catch (IOException e) {
LOGGER.log(Level.FINE, "unresolvable symlink: {0}", allowedSymlink);
LOGGER.log(Level.FINE, "unresolvable symlink: ''{0}''", allowedSymlink);
continue;
}
/*
Expand Down Expand Up @@ -1601,9 +1601,8 @@ private void handleSymlink(String path, AcceptSymlinkRet ret) {
* {@link #removeFile(boolean)}. New or updated files are noted for indexing.
* @param dir the root indexDirectory to generate indexes for
* @param parent path to parent directory
* @param args arguments to control execution and for collecting a list of
* @param args arguments to control execution and for collecting a list of files for indexing
* @param progress {@link Progress} instance
* files for indexing
*/
@VisibleForTesting
void indexDown(File dir, String parent, IndexDownArgs args, Progress progress) throws IOException {
Expand All @@ -1620,8 +1619,7 @@ void indexDown(File dir, String parent, IndexDownArgs args, Progress progress) t

File[] files = dir.listFiles();
if (files == null) {
LOGGER.log(Level.SEVERE, "Failed to get file listing for: {0}",
dir.getPath());
LOGGER.log(Level.SEVERE, "Failed to get file listing for: ''{0}''", dir.getPath());
return;
}
Arrays.sort(files, FILENAME_COMPARATOR);
Expand Down Expand Up @@ -1651,7 +1649,7 @@ void indexDown(File dir, String parent, IndexDownArgs args, Progress progress) t
* @throws IOException on error
*/
@VisibleForTesting
void processFileIncremental(IndexDownArgs args, File file, String path) throws IOException {
void processFileHistoryBased(IndexDownArgs args, File file, String path) throws IOException {
final boolean fileExists = file.exists();

path = Util.fixPathIfWindows(path);
Expand Down Expand Up @@ -1680,9 +1678,7 @@ void processFileIncremental(IndexDownArgs args, File file, String path) throws I
checkSettings(termFile, termPath);
if (!matchOK) {
removeFile(false);

args.curCount++;
args.works.add(new IndexFileWork(termFile, termPath));
addWorkHistoryBased(args, termFile, termPath);
}
} else {
removeFile(!fileExists);
Expand All @@ -1694,10 +1690,33 @@ void processFileIncremental(IndexDownArgs args, File file, String path) throws I
}
}

// The function would not be called if the file was not changed in some way.
if (fileExists) {
// This function would not be called if the file was not changed in some way (including deletion).
// That said, it is necessary to check whether the file can be accepted. This is done in the function below.
// Also, allow for broken symbolic links (File.exists() returns false for these).
if (fileExists || Files.isSymbolicLink(file.toPath())) {
addWorkHistoryBased(args, file, path);
}
}

/**
* Check if file can be accepted into the index database. If yes, change the {@code args} argument appropriately.
* @param args {@link IndexDownArgs} instance to which an entry will be added if deemed acceptable
* @param file file object
* @param path path of the file relative to given source root (not necessarily global source root)
*/
private void addWorkHistoryBased(IndexDownArgs args, File file, String path) {
AcceptSymlinkRet ret = new AcceptSymlinkRet();
if (accept(file, ret)) {
// accept() returns true for directories because it was made to work with indexDown().
if (file.isDirectory()) {
LOGGER.log(Level.FINER, "not accepting directory ''{0}'' into the index", file);
return;
}

args.curCount++;
args.works.add(new IndexFileWork(file, path));
} else {
handleSymlink(file.getParent(), ret);
}
}

Expand Down Expand Up @@ -2201,7 +2220,7 @@ boolean checkSettings(File file, String path) throws IOException {
project.getTabSize() : 0;
Integer actTabSize = settings.getTabSize();
if (actTabSize != null && !actTabSize.equals(reqTabSize)) {
LOGGER.log(Level.FINE, "Tabsize mismatch: {0}", path);
LOGGER.log(Level.FINE, "Tabsize mismatch: ''{0}''", path);
return false;
}

Expand All @@ -2213,7 +2232,7 @@ boolean checkSettings(File file, String path) throws IOException {
// Read a limited-fields version of the document.
Document doc = storedFields.document(postsIter.docID(), CHECK_FIELDS);
if (doc == null) {
LOGGER.log(Level.FINER, "No Document: {0}", path);
LOGGER.log(Level.FINER, "No Document for ''{0}''", path);
continue;
}

Expand All @@ -2234,7 +2253,7 @@ boolean checkSettings(File file, String path) throws IOException {
fileTypeName = doc.get(QueryBuilder.TYPE);
if (fileTypeName == null) {
// (Should not get here, but break just in case.)
LOGGER.log(Level.FINEST, "Missing TYPE field: {0}", path);
LOGGER.log(Level.FINEST, "Missing TYPE field: ''{0}''", path);
break;
}

Expand All @@ -2248,14 +2267,14 @@ boolean checkSettings(File file, String path) throws IOException {
* selection of analyzer or return a value to indicate the
* analyzer is now mis-matched.
*/
LOGGER.log(Level.FINER, "Guru version mismatch: {0}", path);
LOGGER.log(Level.FINER, "Guru version mismatch: ''{0}''", path);

fa = getAnalyzerFor(file, path);
fileTypeName = fa.getFileTypeName();
String oldTypeName = doc.get(QueryBuilder.TYPE);
if (!fileTypeName.equals(oldTypeName)) {
if (LOGGER.isLoggable(Level.FINE)) {
LOGGER.log(Level.FINE, "Changed {0} to {1}: {2}",
LOGGER.log(Level.FINE, "Changed {0} to {1}: ''{2}''",
new Object[]{oldTypeName, fileTypeName, path});
}
return false;
Expand All @@ -2267,7 +2286,7 @@ boolean checkSettings(File file, String path) throws IOException {
Long actVersion = settings.getAnalyzerVersion(fileTypeName);
if (actVersion == null || !actVersion.equals(reqVersion)) {
if (LOGGER.isLoggable(Level.FINE)) {
LOGGER.log(Level.FINE, "{0} version mismatch: {1}",
LOGGER.log(Level.FINE, "{0} version mismatch: ''{1}''",
new Object[]{fileTypeName, path});
}
return false;
Expand All @@ -2281,14 +2300,14 @@ boolean checkSettings(File file, String path) throws IOException {
break;
}
if (n < 1) {
LOGGER.log(Level.FINER, "Missing index Documents: {0}", path);
LOGGER.log(Level.FINER, "Missing index Documents: ''{0}''", path);
return false;
}

// If the economy mode is on, this should be treated as a match.
if (!env.isGenerateHtml()) {
if (xrefExistsFor(path)) {
LOGGER.log(Level.FINEST, "Extraneous {0} , removing its xref file", path);
LOGGER.log(Level.FINEST, "Extraneous ''{0}'' , removing its xref file", path);
removeXrefFile(path);
}
return true;
Expand Down
Loading