arangodb · jsteemann · Jul 19, 2021 · Jul 16, 2021 · Jul 16, 2021 · Jul 16, 2021
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,12 @@
 devel
 -----
 
+* 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. 
+
 * Fixed BTS-408: treat positive or negative signed numbers as constants 
   immediately during AQL query parsing. 
   Previousl
8000
y, a value of `-1` was parsed initially as `unary minus(value(1))`,

diff --git a/arangod/V8Server/V8DealerFeature.cpp b/arangod/V8Server/V8DealerFeature.cpp
@@ -557,9 +557,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;
@@ -569,7 +569,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();
@@ -578,7 +578,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"))) {
@@ -587,7 +587,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;
@@ -606,40 +606,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");
 
-    std::regex const binRegex("[/\\\\]\\.bin[/\\\\]", std::regex::ECMAScript);
+    // .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;
 
-    auto filter = [&nodeModulesPath, &nodeModulesPathVersioned, &binRegex](std::string const& filename) -> bool {
-      if (std::regex_search(filename, binRegex)) {
+    size_t copied = 0;
+
+    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 '"
@@ -662,6 +668,9 @@ 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";
   }
   _startupDirectory = copyJSPath;
 }

diff --git a/lib/Basics/FileUtils.cpp b/lib/Basics/FileUtils.cpp
@@ -64,8 +64,50 @@
 namespace {
 std::function<bool(std::string const&)> const passAllFilter =
     [](std::string const&) { return false; };
+
+enum class StatResultType {
+  Error, // in case it cannot be determined
+  Directory,
+  SymLink,
+  File,
+  Other // potentially file
+};
+
+StatResultType statResultType(TRI_stat_t const& stbuf) {
+#ifdef _WIN32
+  if ((stbuf.st_mode & S_IFMT) == S_IFDIR) {
+    return StatResultType::Directory;
+  }
+#else
+  if (S_ISDIR(stbuf.st_mode)) {
+    return StatResultType::Directory;
+  }
+#endif
+
+#ifndef TRI_HAVE_WIN32_SYMBOLIC_LINK
+  if (S_ISLNK(stbuf.st_mode)) {
+    return StatResultType::SymLink;
+  }
+#endif
+
+  if ((stbuf.st_mode & S_IFMT) == S_IFREG) {
+    return StatResultType::File;
+  }
+
+  return StatResultType::Other;
 }
 
+StatResultType statResultType(std::string const& path) {
+  TRI_stat_t stbuf;
+  int res = TRI_STAT(path.c_str(), &stbuf);
+  if (res != 0) {
+    return StatResultType::Error;
+  }
+  return statResultType(stbuf);
+}
+
+} // namespace
+
 namespace arangodb {
 namespace basics {
 namespace FileUtils {
@@ -401,12 +443,15 @@ bool copyRecursive(std::string const& source, std::string const& target,
 bool copyDirectoryRecursive(std::string const& source, std::string const& target,
                             std::function<TRI_copy_recursive_e(std::string const&)> const& filter,
                             std::string& error) {
-  char* fn = nullptr;
   bool rc_bool = true;
+
+  // these strings will be recycled over and over
+  std::string dst = target + TRI_DIR_SEPARATOR_STR;
+  size_t const dstPrefixLength = dst.size();
+  std::string src = source + TRI_DIR_SEPARATOR_STR;
+  size_t const srcPrefixLength = src.size();
+
 
-  auto isSubDirectory = [](std::string const& name) -> bool {
-    return isDirectory(name);
-  };
 #ifdef TRI_HAVE_WIN32_LIST_FILES
   struct _wfinddata_t oneItem;
   intptr_t handle;
@@ -427,7 +472,7 @@ bool copyDirectoryRecursive(std::string const& source, std::string const& target
     rcs.clear();
     icu::UnicodeString d((wchar_t*)oneItem.name, static_cast<int32_t>(wcslen(oneItem.name)));
     d.toUTF8String<std::string>(rcs);
-    fn = (char*)rcs.c_str();
+    char const* fn = (char*)rcs.c_str();
 #else
   DIR* filedir = opendir(source.c_str());
 
@@ -445,57 +490,79 @@ bool copyDirectoryRecursive(std::string const& source, std::string const& target
   // to be thread-safe in reality, and newer versions of POSIX may require its
   // thread-safety formally, and in addition obsolete readdir_r() altogether
   while ((oneItem = (readdir(filedir))) != nullptr && rc_bool) {
-    fn = oneItem->d_name;
+    char const* fn = oneItem->d_name;
 #endif
 
     // Now iterate over the items.
     // check its not the pointer to the upper directory:
     if (!strcmp(fn, ".") || !strcmp(fn, "..")) {
       continue;
     }
-    std::string dst = target + TRI_DIR_SEPARATOR_STR + fn;
-    std::string src = source + TRI_DIR_SEPARATOR_STR + fn;
 
-    switch (filter(src)) {
-      case TRI_COPY_IGNORE:
-        break;
+    // add current filename to prefix
+    src.resize(srcPrefixLength);
+    TRI_ASSERT(src.back() == TRI_DIR_SEPARATOR_CHAR);
+    src.append(fn);
+
+    auto filterResult = filter(src);
+
+    if (filterResult != TRI_COPY_IGNORE) {
+      // prepare dst filename
+      dst.resize(dstPrefixLength);
+      TRI_ASSERT(dst.back() == TRI_DIR_SEPARATOR_CHAR);
+      dst.append(fn);
+
+      // figure out the type of the directory entry.
+      StatResultType type = StatResultType::Error;
+      TRI_stat_t stbuf;
+      int res = TRI_STAT(src.c_str(), &stbuf);
+      if (res == 0) {
+        type = ::statResultType(stbuf);
+      }
 
-      case TRI_COPY_COPY:
-        // Handle subdirectories:
-        if (isSubDirectory(src)) {
-          long systemError;
-          auto rc = TRI_CreateDirectory(dst.c_str(), systemError, error);
-          if (rc != TRI_ERROR_NO_ERROR && rc != TRI_ERROR_FILE_EXISTS) {
-            rc_bool = false;
-            break;
-          }
-          if (!copyDirectoryRecursive(src, dst, filter, error)) {
-            rc_bool = false;
-            break;
-          }
-          if (!TRI_CopyAttributes(src, dst, error)) {
-            rc_bool = false;
-            break;
-          }
-#ifndef _WIN32
-        } else if (isSymbolicLink(src)) {
-          if (!TRI_CopySymlink(src, dst, error)) {
-            rc_bool = false;
-          }
+      switch (filterResult) {
+        case TRI_COPY_IGNORE:
+          TRI_ASSERT(false);
+          break;
+
+        case TRI_COPY_COPY:
+          // Handle subdirectories:
+          if (type == StatResultType::Directory) {
+            long systemError;
+            auto rc = TRI_CreateDirectory(dst.c_str(), systemError, error);
+            if (rc != TRI_ERROR_NO_ERROR && rc != TRI_ERROR_FILE_EXISTS) {
+              rc_bool = false;
+              break;
+            }
+            if (!copyDirectoryRecursive(src, dst, filter, error)) {
+              rc_bool = false;
+              break;
+            }
+            if (!TRI_CopyAttributes(src, dst, error)) {
+              rc_bool = false;
+              break;
+            }
+          } else if (type == StatResultType::SymLink) {
+            if (!TRI_CopySymlink(src, dst, error)) {
+              rc_bool = false;
+            }
+          } else {
+#ifdef _WIN32
+            rc_bool = TRI_CopyFile(src, dst, error);
+#else
+            // optimized version that reuses the already retrieved stat data
+            rc_bool = TRI_CopyFile(src, dst, error, &stbuf);
 #endif
-        } else {
-          if (!TRI_CopyFile(src, dst, error)) {
-            rc_bool = false;
           }
-        }
-        break;
+          break;
 
-      case TRI_COPY_LINK:
-        if (!TRI_CreateHardlink(src, dst, error)) {
-          rc_bool = false;
-        } // if
-        break;
-    } // switch
+        case TRI_COPY_LINK:
+          if (!TRI_CreateHardlink(src, dst, error)) {
+            rc_bool = false;
+          } // if
+          break;
+      } // switch
+    }
 #ifdef TRI_HAVE_WIN32_LIST_FILES
   } while (_wfindnext(handle, &oneItem) != -1 && rc_bool);
 
@@ -578,48 +645,19 @@ std::vector<std::string> listFiles(std::string const& directory) {
 }
 
 bool isDirectory(std::string const& path) {
-  TRI_stat_t stbuf;
-  int res = TRI_STAT(path.c_str(), &stbuf);
-
-#ifdef _WIN32
-  return (res == 0) && ((stbuf.st_mode & S_IFMT) == S_IFDIR);
-#else
-  return (res == 0) && S_ISDIR(stbuf.st_mode);
-#endif
+  return ::statResultType(path) == ::StatResultType::Directory;
 }
 
 bool isSymbolicLink(std::string const& path) {
-#ifdef TRI_HAVE_WIN32_SYMBOLIC_LINK
-
-  // .....................................................................
-  // TODO: On the NTFS file system, there are the following file links:
-  // hard links -
-  // junctions -
-  // symbolic links -
-  // .....................................................................
-  return false;
-
-#else
-
-  struct stat stbuf;
-  int res = TRI_STAT(path.c_str(), &stbuf);
-
-  return (res == 0) && S_ISLNK(stbuf.st_mode);
-
-#endif
+  return ::statResultType(path) == ::StatResultType::SymLink;
 }
 
 bool isRegularFile(std::string const& path) {
-  TRI_stat_t stbuf;
-  int res = TRI_STAT(path.c_str(), &stbuf);
-  return (res == 0) && ((stbuf.st_mode & S_IFMT) == S_IFREG);
+  return ::statResultType(path) == ::StatResultType::File;
 }
 
 bool exists(std::string const& path) {
-  TRI_stat_t stbuf;
-  int res = TRI_STAT(path.c_str(), &stbuf);
-
-  return res == 0;
+  return ::statResultType(path) != ::StatResultType::Error;
 }
 
 off_t size(std::string const& path) {