8000 Improve startup file copying of JavaScript files by jsteemann · Pull Request #14526 · arangodb/arangodb · GitHub
[go: up one dir, main page]
Skip to content

Improve startup file copying of JavaScript files #14526

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 4 commits into from
Jul 28, 2021
Merged

Improve startup file copying of JavaScript files #14526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a267658
Improve startup file copying of JavaScript files
jsteemann Jul 19, 2021
f786d8c
more fixes for node_moduels copying
jsteemann Jul 20, 2021
e67cb2a
fix allowlisting
jsteemann Jul 20, 2021
78a7689
Merge branch '3.8' into bug-fix-3.8/bts-516-file-copying
KVS85 Jul 28, 2021
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
6 changes: 6 additions & 0 deletions CHANGELOG
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
v3.8.1 (XXXX-XX-XX)
-------------------

* Make `--javascript.copy-installation` also copy the `node_modules` sub
directory. This is required so we have a full copy of the JavaScript
dependencies and not one that excludes some infrequently changed modules.
In addition, file copying now intentionally excludes .map files as they are
not needed.

* Updated JavaScript dependencies, including breaking changes to non-public
modules. We recommend always bundling your own copy of third-party modules,
even ones listed as public.
Expand Down
110 changes: 55 additions & 55 deletions arangod/V8Server/V8DealerFeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,34 +343,10 @@ void V8DealerFeature::validateOptions(std::shared_ptr<ProgramOptions> options) {
v8security.addToInternalAllowList(_startupDirectory, FSAccessType::READ);

ctx->normalizePath(_moduleDirectories, "javascript.module-directory", false);

// try to append the current version name to the startup directory,
// so instead of "/path/to/js" we will get "/path/to/js/3.4.0"
std::string const versionAppendix =
std::regex_replace(rest::Version::getServerVersion(), std::regex("-.*$"),
"");
std::string versionedPath =
basics::FileUtils::buildFilename(_startupDirectory, versionAppendix);

LOG_TOPIC("604da", DEBUG, Logger::V8)
<< "checking for existence of version-specific startup-directory '"
<< versionedPath << "'";
if (basics::FileUtils::isDirectory(versionedPath)) {
// version-specific js path exists!
_startupDirectory = versionedPath;
}

for (auto& it : _moduleDirectories) {
versionedPath = basics::FileUtils::buildFilename(it, versionAppendix);

LOG_TOPIC("8e21a", DEBUG, Logger::V8)
<< "checking for existence of version-specific module-directory '"
<< versionedPath << "'";
if (basics::FileUtils::isDirectory(versionedPath)) {
// version-specific js path exists!
it = versionedPath;
for (auto const& it : _moduleDirectories) {
if (!it.empty()) {
v8security.addToInternalAllowList(it, FSAccessType::READ);
}
v8security.addToInternalAllowList(it, FSAccessType::READ);
}

// check whether app-path was specified
Expand Down Expand Up @@ -409,26 +385,38 @@ void V8DealerFeature::start() {
// now check if we have a js directory inside the database directory, and if
// it looks good
auto& dbPathFeature = server().getFeature<DatabasePathFeature>();
const std::string dbJSPath =
std::string const dbJSPath =
FileUtils::buildFilename(dbPathFeature.directory(), "js");
const std::string checksumFile =
std::string const checksumFile =
FileUtils::buildFilename(dbJSPath, StaticStrings::checksumFileJs);
const std::string serverPath = FileUtils::buildFilename(dbJSPath, "server");
const std::string commonPath = FileUtils::buildFilename(dbJSPath, "common");
std::string const serverPath = FileUtils::buildFilename(dbJSPath, "server");
std::string const commonPath = FileUtils::buildFilename(dbJSPath, "common");
std::string const nodeModulesPath = FileUtils::buildFilename(dbJSPath, "node", "node_modules");
if (FileUtils::isDirectory(dbJSPath) && FileUtils::exists(checksumFile) &&
FileUtils::isDirectory(serverPath) && FileUtils::isDirectory(commonPath)) {
// only load node modules from original startup path
_nodeModulesDirectory = _startupDirectory;
// js directory inside database directory looks good. now use it!
_startupDirectory = dbJSPath;
// older versions didn't copy node_modules. so check if it exists inside the
// database directory or not.
if (FileUtils::isDirectory(nodeModulesPath)) {
_nodeModulesDirectory = nodeModulesPath;
} else {
_nodeModulesDirectory = _startupDirectory;
}
}
}

V8SecurityFeature& v8security = server().getFeature<V8SecurityFeature>();
v8security.addToInternalAllowList(_startupDirectory, FSAccessType::READ);
if (!_nodeModulesDirectory.empty()) {
v8security.addToInternalAllowList(_nodeModulesDirectory, FSAccessType::READ);
}

LOG_TOPIC("77c97", DEBUG, Logger::V8)
<< "effective startup-directory: " << _startupDirectory
<< ", effective module-directories: " << _moduleDirectories
<< ", node-modules-directory: " << _nodeModulesDirectory;

_startupLoader.setDirectory(_startupDirectory);

// dump paths
Expand Down Expand Up @@ -557,9 +545,9 @@ void V8DealerFeature::copyInstallationFiles() {

_nodeModulesDirectory = _startupDirectory;

const std::string checksumFile =
std::string const checksumFile =
FileUtils::buildFilename(_startupDirectory, StaticStrings::checksumFileJs);
const std::string copyChecksumFile =
std::string const copyChecksumFile =
FileUtils::buildFilename(copyJSPath, StaticStrings::checksumFileJs);

bool overwriteCopy = false;
Expand All @@ -569,7 +557,7 @@ void V8DealerFeature::copyInstallationFiles() {
} else {
try {
overwriteCopy =
(FileUtils::slurp(copyChecksumFile) != FileUtils::slurp(checksumFile));
(StringUtils::trim(FileUtils::slurp(copyChecksumFile)) != StringUtils::trim(FileUtils::slurp(checksumFile)));
} catch (basics::Exception const& e) {
LOG_TOPIC("efa47", ERR, Logger::V8) << "Error reading '" << StaticStrings::checksumFileJs
<< "' from disk: " << e.what();
Expand All @@ -578,7 +566,7 @@ void V8DealerFeature::copyInstallationFiles() {
}

if (overwriteCopy) {
// basics security checks before removing an existing directory:
// basic security checks before removing an existing directory:
// check if for some reason we will be trying to remove the entire database
// directory...
if (FileUtils::exists(FileUtils::buildFilename(copyJSPath, "ENGINE"))) {
Expand All @@ -587,7 +575,7 @@ void V8DealerFeature::copyInstallationFiles() {
FATAL_ERROR_EXIT();
}

LOG_TOPIC("dd1c0", DEBUG, Logger::V8)
LOG_TOPIC("dd1c0", INFO, Logger::V8)
<< "Copying JS installation files from '" << _startupDirectory
<< "' to '" << copyJSPath << "'";
auto res = TRI_ERROR_NO_ERROR;
Expand All @@ -606,40 +594,46 @@ void V8DealerFeature::copyInstallationFiles() {
FATAL_ERROR_EXIT();
}

// intentionally do not copy js/node/node_modules...
// intentionally do not copy js/node/node_modules/estlint!
// we avoid copying this directory because it contains 5000+ files at the
// moment, and copying them one by one is darn slow at least on Windows...
// moment, and copying them one by one is slow. In addition, eslint is not
// needed in release builds
std::string const versionAppendix =
std::regex_replace(rest::Version::getServerVersion(),
std::regex("-.*$"), "");
std::string const nodeModulesPath =
FileUtils::buildFilename("js", "node", "node_modules");
std::string const nodeModulesPathVersioned =
basics::FileUtils::buildFilename("js", versionAppendix, "node",
"node_modules");
std::string const eslintPath =
FileUtils::buildFilename("js", "node", "node_modules", "eslint");

// .bin directories could be harmful, and .map files are large and unnecessary
std::string const binDirectory = std::string(TRI_DIR_SEPARATOR_STR) + ".bin" + TRI_DIR_SEPARATOR_STR;

std::regex const binRegex("[/\\\\]\\.bin[/\\\\]", std::regex::ECMAScript);
size_t copied = 0;

auto filter = [&nodeModulesPath, &nodeModulesPathVersioned, &binRegex](std::string const& filename) -> bool {
if (std::regex_search(filename, binRegex)) {
auto filter = [&eslintPath, &binDirectory, &copied](std::string const& filename) -> bool {
if (filename.size() >= 4 && filename.compare(filename.size() - 4, 4, ".map") == 0) {
// filename ends with ".map". filter it out!
return true;
}
if (filename.find(binDirectory) != std::string::npos) {
// don't copy files in .bin
return true;
}

std::string normalized = filename;
FileUtils::normalizePath(normalized);
if ((!nodeModulesPath.empty() &&
normalized.size() >= nodeModulesPath.size() &&
normalized.substr(normalized.size() - nodeModulesPath.size(), nodeModulesPath.size()) == nodeModulesPath) ||
(!nodeModulesPathVersioned.empty() &&
normalized.size() >= nodeModulesPathVersioned.size() &&
normalized.substr(normalized.size() - nodeModulesPathVersioned.size(), nodeModulesPathVersioned.size()) == nodeModulesPathVersioned)) {
if ((normalized.size() >= eslintPath.size() &&
normalized.compare(normalized.size() - eslintPath.size(), eslintPath.size(), eslintPath) == 0)) {
// filter it out!
return true;
}

// let the file/directory pass through
++copied;
return false;
};

double start = TRI_microtime();

std::string error;
if (!FileUtils::copyRecursive(_startupDirectory, copyJSPath, filter, error)) {
LOG_TOPIC("45261", FATAL, Logger::V8) << "Error copying JS installation files to '"
Expand All @@ -662,8 +656,14 @@ void V8DealerFeature::copyInstallationFiles() {
<< copyJSPath << "': " << error;
}
}

LOG_TOPIC("38e1e", INFO, Logger::V8)
<< "copying " << copied << " JS installation file(s) took " << Logger::FIXED(TRI_microtime() - start, 6) << "s";
}

// finally switch over the paths
_startupDirectory = copyJSPath;
_nodeModulesDirectory = basics::FileUtils::buildFilename(copyJSPath, "node", "node_modules");
}

V8Context* V8DealerFeature::addContext() {
Expand Down
Loading
0