Skip to content

Commit

Permalink
Use absolute paths for external symlinks in Builder#copyDirectory.
Browse files Browse the repository at this point in the history
Thanks to @worldsayshi for reporting this issue with a reproduction.

Fixes #8005.
Fixes #2876.
Fixes #7154.
  • Loading branch information
Ben Newman committed Nov 4, 2016
1 parent 2e00dc3 commit af51b81
Showing 1 changed file with 57 additions and 6 deletions.
63 changes: 57 additions & 6 deletions tools/isobuild/builder.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import assert from "assert";
import {WatchSet, readAndWatchFile, sha1} from '../fs/watch.js';
import files from '../fs/files.js';
import NpmDiscards from './npm-discards.js';
Expand Down Expand Up @@ -65,6 +66,8 @@ export default class Builder {
this.writtenHashes = {};
this.previousWrittenHashes = {};

this._realpathCache = Object.create(null);

// foo/bar => foo/.build1234.bar
// Should we include a random number? The advantage is that multiple
// builds can run in parallel. The disadvantage is that stale build
Expand Down Expand Up @@ -443,7 +446,11 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}`
});
}

let walk = (absFrom, relTo) => {
const walk = (
absFrom,
relTo,
_currentRealRootDir = absFrom
) => {
if (symlink && ! (relTo in this.usedAsFile)) {
this._ensureDirectory(files.pathDirname(relTo));
const absTo = files.pathResolve(this.buildPath, relTo);
Expand All @@ -454,14 +461,55 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}`
this._ensureDirectory(relTo);

optimisticReaddir(absFrom).forEach(item => {
const thisAbsFrom = files.pathResolve(absFrom, item);
let thisAbsFrom = files.pathResolve(absFrom, item);
const thisRelTo = files.pathJoin(relTo, item);

if (specificPaths && !(thisRelTo in specificPaths)) {
return;
}

const fileStatus = optimisticLStat(thisAbsFrom);
// Returns files.realpath(thisAbsFrom), iff it is external to
// _currentRealRootDir, using caching because this function might
// be called more than once.
let cachedExternalPath;
const getExternalPath = () => {
if (typeof cachedExternalPath !== "undefined") {
return cachedExternalPath;
}

const real = files.realpath(thisAbsFrom, this._realpathCache);
const isExternal =
files.pathRelative(_currentRealRootDir, real).startsWith("..");

// Now cachedExternalPath is either a string or false.
return cachedExternalPath = isExternal && real;
};

let fileStatus = optimisticLStat(thisAbsFrom);

if (! symlink && fileStatus.isSymbolicLink()) {
// If copyDirectory is not allowed to create symbolic links to
// external files, and this file is a symbolic link that points
// to an external file, update fileStatus so that we copy this
// file as a normal file rather than as a symbolic link.

const externalPath = getExternalPath();
if (externalPath) {
// Copy from the real path rather than the link path.
thisAbsFrom = externalPath;

// Update fileStatus to match the actual file rather than the
// symbolic link, thus forcing the file to be copied below.
fileStatus = optimisticLStat(externalPath);

if (fileStatus.isDirectory()) {
// Update _currentRealRootDir so that we can judge
// isExternal relative to this new root directory when
// traversing nested directories.
_currentRealRootDir = externalPath;
}
}
}

let itemForMatch = item;
const isDirectory = fileStatus.isDirectory();
Expand All @@ -482,12 +530,15 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}`
if (isDirectory) {
if (typeof directoryFilter !== "function" ||
directoryFilter(thisAbsFrom)) {
walk(thisAbsFrom, thisRelTo);
walk(thisAbsFrom, thisRelTo, _currentRealRootDir);
}

} else if (fileStatus.isSymbolicLink()) {
symlinkWithOverwrite(
files.readlink(thisAbsFrom),
// Symbolic links pointing to relative external paths are less
// portable than absolute links, so getExternalPath() is
// preferred if it returns a path.
getExternalPath() || files.readlink(thisAbsFrom),
files.pathResolve(this.buildPath, thisRelTo)
);

Expand Down Expand Up @@ -518,7 +569,7 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}`
});
};

walk(from, to);
walk(files.realpath(from, this._realpathCache), to);
}

// Returns a new Builder-compatible object that works just like a
Expand Down

0 comments on commit af51b81

Please sign in to comment.