8000 Improve startup file copying of JavaScript files (#14526) · arangodb/arangodb@6b3c327 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6b3c327

Browse files
jsteemannKVS85
jsteemann
and
KVS85
authored
Improve startup file copying of JavaScript files (#14526)
* Improve startup file copying of JavaScript files * 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. * more fixes for node_moduels copying * fix allowlisting Co-authored-by: Vadim <vadim@arangodb.com>
1 parent bbc2d26 commit 6b3c327

File tree

7 files changed

+306
-221
lines changed

7 files changed

+306
-221
lines changed

‎CHANGELOG

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
v3.8.1 (XXXX-XX-XX)
22
-------------------
33

4+
* Make `--javascript.copy-installation` also copy the `node_modules` sub
5+
directory. This is required so we have a full copy of the JavaScript
6+
dependencies and not one that excludes some infrequently changed modules.
7+
In addition, file copying now intentionally excludes .map files as they are
8+
not needed.
9+
410
* Updated JavaScript dependencies, including breaking changes to non-public
511
modules. We recommend always bundling your own copy of third-party modules,
612
even ones listed as public.

‎arangod/V8Server/V8DealerFeature.cpp

Lines changed: 55 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -343,34 +343,10 @@ void V8DealerFeature::validateOptions(std::shared_ptr<ProgramOptions> options) {
343343
v8security.addToInternalAllowList(_startupDirectory, FSAccessType::READ);
344344

345345
ctx->normalizePath(_moduleDirectories, "javascript.module-directory", false);
346-
347-
// try to append the current version name to the startup directory,
348-
// so instead of "/path/to/js" we will get "/path/to/js/3.4.0"
349-
std::string const versionAppendix =
350-
std::regex_replace(rest::Version::getServerVersion(), std::regex("-.*$"),
351-
"");
352-
std::string versionedPath =
353-
basics::FileUtils::buildFilename(_startupDirectory, versionAppendix);
354-
355-
LOG_TOPIC("604da", DEBUG, Logger::V8)
356-
<< "checking for existence of version-specific startup-directory '"
357-
<< versionedPath << "'";
358-
if (basics::FileUtils::isDirectory(versionedPath)) {
359-
// version-specific js path exists!
360-
_startupDirectory = versionedPath;
361-
}
362-
363-
for (auto& it : _moduleDirectories) {
364-
versionedPath = basics::FileUtils::buildFilename(it, versionAppendix);
365-
366-
LOG_TOPIC("8e21a", DEBUG, Logger::V8)
367-
<< "checking for existence of version-specific module-directory '"
368-
<< versionedPath << "'";
369-
if (basics::FileUtils::isDirectory(versionedPath)) {
370-
// version-specific js path exists!
371-
it = versionedPath;
346+
for (auto const& it : _moduleDirectories) {
347+
if (!it.empty()) {
348+
v8security.addToInternalAllowList(it, FSAccessType::READ);
372349
}
373-
v8security.addToInternalAllowList(it, FSAccessType::READ);
374350
}
375351

376352
// check whether app-path was specified
@@ -409,26 +385,38 @@ void V8DealerFeature::start() {
409385
// now check if we have a js directory inside the database directory, and if
410386
// it looks good
411387
auto& dbPathFeature = server().getFeature<DatabasePathFeature>();
412-
const std::string dbJSPath =
388+
std::string const dbJSPath =
413389
FileUtils::buildFilename(dbPathFeature.directory(), "js");
414-
const std::string checksumFile =
390+
std::string const checksumFile =
415391
FileUtils::buildFilename(dbJSPath, StaticStrings::checksumFileJs);
416-
const std::string serverPath = FileUtils::buildFilename(dbJSPath, "server");
417-
const std::string commonPath = FileUtils::buildFilename(dbJSPath, "common");
392+
std::string const serverPath = FileUtils::buildFilename(dbJSPath, "server");
393+
std::string const commonPath = FileUtils::buildFilename(dbJSPath, "common");
394+
std::string const nodeModulesPath = FileUtils::buildFilename(dbJSPath, "node", "node_modules");
418395
if (FileUtils::isDirectory(dbJSPath) && FileUtils::exists(checksumFile) &&
419396
FileUtils::isDirectory(serverPath) && FileUtils::isDirectory(commonPath)) {
420-
// only load node modules from original startup path
421-
_nodeModulesDirectory = _startupDirectory;
422397
// js directory inside database directory looks good. now use it!
423398
_startupDirectory = dbJSPath;
399+
// older versions didn't copy node_modules. so check if it exists inside the
400+
// database directory or not.
401+
if (FileUtils::isDirectory(nodeModulesPath)) {
402+
_nodeModulesDirectory = nodeModulesPath;
403+
} else {
404+
_nodeModulesDirectory = _startupDirectory;
405+
}
424406
}
425407
}
408+
409+
V8SecurityFeature& v8security = server().getFeature<V8SecurityFeature>();
410+
v8security.addToInternalAllowList(_startupDirectory, FSAccessType::READ);
411+
if (!_nodeModulesDirectory.empty()) {
412+
v8security.addToInternalAllowList(_nodeModulesDirectory, FSAccessType::READ);
413+
}
426414

427415
LOG_TOPIC("77c97", DEBUG, Logger::V8)
428416
<< "effective startup-directory: " << _startupDirectory
429417
<< ", effective module-directories: " << _moduleDirectories
430418
<< ", node-modules-directory: " << _nodeModulesDirectory;
431-
419+
432420
_startupLoader.setDirectory(_startupDirectory);
433421

434422
// dump paths
@@ -557,9 +545,9 @@ void V8DealerFeature::copyInstallationFiles() {
557545

558546
_nodeModulesDirectory = _startupDirectory;
559547

560-
const std::string checksumFile =
548+
std::string const checksumFile =
561549
FileUtils::buildFilename(_startupDirectory, StaticStrings::checksumFileJs);
562-
const std::string copyChecksumFile =
550+
std::string const copyChecksumFile =
563551
FileUtils::buildFilename(copyJSPath, StaticStrings::checksumFileJs);
564552

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

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

590-
LOG_TOPIC("dd1c0", DEBUG, Logger::V8)
578+
LOG_TOPIC("dd1c0", INFO, Logger::V8)
591579
<< "Copying JS installation files from '" << _startupDirectory
592580
<< "' to '" << copyJSPath << "'";
593581
auto res = TRI_ERROR_NO_ERROR;
@@ -606,40 +594,46 @@ void V8DealerFeature::copyInstallationFiles() {
606594
FATAL_ERROR_EXIT();
607595
}
608596

609-
// intentionally do not copy js/node/node_modules...
597+
// intentionally do not copy js/node/node_modules/estlint!
610598
// we avoid copying this directory because it contains 5000+ files at the
611-
// moment, and copying them one by one is darn slow at least on Windows...
599+
// moment, and copying them one by one is slow. In addition, eslint is not
600+
// needed in release builds
612601
std::string const versionAppendix =
613602
std::regex_replace(rest::Version::getServerVersion(),
614603
std::regex("-.*$"), "");
615-
std::string const nodeModulesPath =
616-
FileUtils::buildFilename("js", "node", "node_modules");
617-
std::string const nodeModulesPathVersioned =
618-
basics::FileUtils::buildFilename("js", versionAppendix, "node",
619-
"node_modules");
604+
std::string const eslintPath =
605+
FileUtils::buildFilename("js", "node", "node_modules", "eslint");
606+
607+
// .bin directories could be harmful, and .map files are large and unnecessary
608+
std::string const binDirectory = std::string(TRI_DIR_SEPARATOR_STR) + ".bin" + TRI_DIR_SEPARATOR_STR;
620609

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

623-
auto filter = [&nodeModulesPath, &nodeModulesPathVersioned, &binRegex](std::string const& filename) -> bool {
624-
if (std::regex_search(filename, binRegex)) {
612+
auto filter = [&eslintPath, &binDirectory, &copied](std::string const& filename) -> bool {
613+
if (filename.size() >= 4 && filename.compare(filename.size() - 4, 4, ".map") == 0) {
614+
// filename ends with ".map". filter it out!
615+
return true;
616+
}
617+
if (filename.find(binDirectory) != std::string::npos) {
625618
// don't copy files in .bin
626619
return true;
627620
}
621+
628622
std::string normalized = filename;
629623
FileUtils::normalizePath(normalized);
630-
if ((!nodeModulesPath.empty() &&
631-
normalized.size() >= nodeModulesPath.size() &&
632-
normalized.substr(normalized.size() - nodeModulesPath.size(), nodeModulesPath.size()) == nodeModulesPath) ||
633-
(!nodeModulesPathVersioned.empty() &&
634-
normalized.size() >= nodeModulesPathVersioned.size() &&
635-
normalized.substr(normalized.size() - nodeModulesPathVersioned.size(), nodeModulesPathVersioned.size()) == nodeModulesPathVersioned)) {
624+
if ((normalized.size() >= eslintPath.size() &&
625+
normalized.compare(normalized.size() - eslintPath.size(), eslintPath.size(), eslintPath) == 0)) {
636626
// filter it out!
637627
return true;
638628
}
629+
639630
// let the file/directory pass through
631+
++copied;
640632
return false;
641633
};
642634

635+
double start = TRI_microtime();
636+
643637
std::string error;
644638
if (!FileUtils::copyRecursive(_startupDirectory, copyJSPath, filter, error)) {
645639
LOG_TOPIC("45261", FATAL, Logger::V8) << "Error copying JS installation files to '"
@@ -662,8 +656,14 @@ void V8DealerFeature::copyInstallationFiles() {
662656
<< copyJSPath << "': " << error;
663657
}
664658
}
659+
660+
LOG_TOPIC("38e1e", INFO, Logger::V8)
661+
<< "copying " << copied << " JS installation file(s) took " << Logger::FIXED(TRI_microtime() - start, 6) << "s";
665662
}
663+
664+
// finally switch over the paths
666665
_startupDirectory = copyJSPath;
666+
_nodeModulesDirectory = basics::FileUtils::buildFilename(copyJSPath, "node", "node_modules");
667667
}
668668

669669
V8Context* V8DealerFeature::addContext() {

0 commit comments

Comments
 (0)
0