From 8195013cd890456274edd5198ca8e7b00dc74c50 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Fri, 16 Jul 2021 18:41:34 +0200 Subject: [PATCH 1/4] 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. --- CHANGELOG | 6 + arangod/V8Server/V8DealerFeature.cpp | 51 +++++--- lib/Basics/FileUtils.cpp | 189 ++++++++++++++++----------- lib/Basics/files.cpp | 177 ++++++++++++++----------- lib/Basics/files.h | 8 ++ lib/Basics/operating-system.h | 12 +- 6 files changed, 265 insertions(+), 178 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index d06bb8bdb64a..06ce2db96f21 100644 --- 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. + * Slightly increase internal AQL query and transaction timeout on DB servers from 3 to 5 minutes. Previously, queries and transactions on DB servers could expire quicker, diff --git a/arangod/V8Server/V8DealerFeature.cpp b/arangod/V8Server/V8DealerFeature.cpp index 0e298055d7c1..9e0d4d009991 100644 --- 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 index 389f9d1496d2..4918df47e7a5 100755 --- a/lib/Basics/FileUtils.cpp +++ b/lib/Basics/FileUtils.cpp @@ -64,8 +64,50 @@ namespace { std::function 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,8 @@ bool copyRecursive(std::string const& source, std::string const& target, bool copyDirectoryRecursive(std::string const& source, std::string const& target, std::function const& filter, std::string& error) { - char* fn = nullptr; bool rc_bool = true; - 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 +465,7 @@ bool copyDirectoryRecursive(std::string const& source, std::string const& target rcs.clear(); icu::UnicodeString d((wchar_t*)oneItem.name, static_cast(wcslen(oneItem.name))); d.toUTF8String(rcs); - fn = (char*)rcs.c_str(); + char const* fn = (char*)rcs.c_str(); #else DIR* filedir = opendir(source.c_str()); @@ -438,6 +476,12 @@ bool copyDirectoryRecursive(std::string const& source, std::string const& target struct dirent* oneItem = nullptr; + // these strings will be recycled over and over + std::string dst = target + TRI_DIR_SEPARATOR_STR; + size_t dstPrefixLength = dst.size(); + std::string src = source + TRI_DIR_SEPARATOR_STR; + size_t srcPrefixLength = src.size(); + // do not use readdir_r() here anymore as it is not safe and deprecated // in newer versions of libc: // http://man7.org/linux/man-pages/man3/readdir_r.3.html @@ -445,7 +489,7 @@ 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. @@ -453,49 +497,71 @@ bool copyDirectoryRecursive(std::string const& source, std::string const& target 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; - } + 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 { #ifndef _WIN32 - } else if (isSymbolicLink(src)) { - if (!TRI_CopySymlink(src, dst, error)) { - rc_bool = false; - } + 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 +644,19 @@ std::vector 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) { diff --git a/lib/Basics/files.cpp b/lib/Basics/files.cpp index 94e8e6c91211..e2fc137ae781 100644 --- a/lib/Basics/files.cpp +++ b/lib/Basics/files.cpp @@ -1796,7 +1796,10 @@ std::string TRI_GetInstallRoot(std::string const& binaryPath, char const* instal static bool CopyFileContents(int srcFD, int dstFD, TRI_read_t fileSize, std::string& error) { bool rc = true; -#if TRI_LINUX_SPLICE +#ifdef __linux__ + // Linux-specific file-copying code based on splice() + // The splice() system call first appeared in Linux 2.6.17; library support was added to glibc in version 2.5. + // libmusl also has bindings for it. so we simply assume it is there on Linux. int splicePipe[2]; ssize_t pipeSize = 0; long chunkSendRemain = fileSize; @@ -1806,25 +1809,30 @@ static bool CopyFileContents(int srcFD, int dstFD, TRI_read_t fileSize, std::str error = std::string("splice failed to create pipes: ") + strerror(errno); return false; } - while (chunkSendRemain > 0) { - if (pipeSize == 0) { - pipeSize = splice(srcFD, &totalSentAlready, splicePipe[1], nullptr, - chunkSendRemain, SPLICE_F_MOVE); - if (pipeSize == -1) { + try { + while (chunkSendRemain > 0) { + if (pipeSize == 0) { + pipeSize = splice(srcFD, &totalSentAlready, splicePipe[1], nullptr, + chunkSendRemain, SPLICE_F_MOVE); + if (pipeSize == -1) { + error = std::string("splice read failed: ") + strerror(errno); + rc = false; + break; + } + } + ssize_t sent = splice(splicePipe[0], nullptr, dstFD, nullptr, pipeSize, + SPLICE_F_MORE | SPLICE_F_MOVE | SPLICE_F_NONBLOCK); + if (sent == -1) { error = std::string("splice read failed: ") + strerror(errno); rc = false; break; } + pipeSize -= sent; + chunkSendRemain -= sent; } - ssize_t sent = splice(splicePipe[0], nullptr, dstFD, nullptr, pipeSize, - SPLICE_F_MORE | SPLICE_F_MOVE | SPLICE_F_NONBLOCK); - if (sent == -1) { - error = std::string("splice read failed: ") + strerror(errno); - rc = false; - break; - } - pipeSize -= sent; - chunkSendRemain -= sent; + } catch (...) { + // make sure we always close the pipes + rc = false; } close(splicePipe[0]); close(splicePipe[1]); @@ -1838,42 +1846,47 @@ static bool CopyFileContents(int srcFD, int dstFD, TRI_read_t fileSize, std::str rc = false; } - TRI_read_t chunkRemain = fileSize; - while (rc && (chunkRemain > 0)) { - auto readChunk = static_cast((std::min)(C128, static_cast(chunkRemain))); - TRI_read_return_t nRead = TRI_READ(srcFD, buf, readChunk); - - if (nRead < 0) { - error = std::string("failed to read a chunk: ") + strerror(errno); - rc = false; - break; - } - - if (nRead == 0) { - // EOF. done - break; - } + try { + TRI_read_t chunkRemain = fileSize; + while (rc && (chunkRemain > 0)) { + auto readChunk = static_cast((std::min)(C128, static_cast(chunkRemain))); + TRI_read_return_t nRead = TRI_READ(srcFD, buf, readChunk); - size_t writeOffset = 0; - size_t writeRemaining = static_cast(nRead); - while (writeRemaining > 0) { - // write can write less data than requested. so we must go on writing - // until we have written out all data - ssize_t nWritten = TRI_WRITE(dstFD, buf + writeOffset, - static_cast(writeRemaining)); - - if (nWritten < 0) { - // error during write + if (nRead < 0) { error = std::string("failed to read a chunk: ") + strerror(errno); rc = false; break; } - writeOffset += static_cast(nWritten); - writeRemaining -= static_cast(nWritten); - } + if (nRead == 0) { + // EOF. done + break; + } + + size_t writeOffset = 0; + size_t writeRemaining = static_cast(nRead); + while (writeRemaining > 0) { + // write can write less data than requested. so we must go on writing + // until we have written out all data + ssize_t nWritten = TRI_WRITE(dstFD, buf + writeOffset, + static_cast(writeRemaining)); + + if (nWritten < 0) { + // error during write + error = std::string("failed to read a chunk: ") + strerror(errno); + rc = false; + break; + } - chunkRemain -= nRead; + writeOffset += static_cast(nWritten); + writeRemaining -= static_cast(nWritten); + } + + chunkRemain -= nRead; + } + } catch (...) { + // make sure we always close the buffer + rc = false; } TRI_Free(buf); @@ -1885,8 +1898,8 @@ static bool CopyFileContents(int srcFD, int dstFD, TRI_read_t fileSize, std::str /// @brief copies the contents of a file //////////////////////////////////////////////////////////////////////////////// -bool TRI_CopyFile(std::string const& src, std::string const& dst, std::string& error) { #ifdef _WIN32 +bool TRI_CopyFile(std::string const& src, std::string const& dst, std::string& error) { TRI_ERRORBUF; bool rc = CopyFileW(toWString(src).data(), toWString(dst).data(), true) != 0; @@ -1896,57 +1909,72 @@ bool TRI_CopyFile(std::string const& src, std::string const& dst, std::string& e } return rc; +} #else - size_t dsize; - int srcFD, dstFD; - struct stat statbuf; - - srcFD = open(src.c_str(), O_RDONLY); +bool TRI_CopyFile(std::string const& src, std::string const& dst, std::string& error, struct stat* statbuf /*= nullptr*/) { + int srcFD = open(src.c_str(), O_RDONLY); if (srcFD < 0) { error = "failed to open source file " + src + ": " + strerror(errno); return false; } - dstFD = open(dst.c_str(), O_EXCL | O_CREAT | O_NONBLOCK | O_WRONLY, S_IRUSR | S_IWUSR); + int dstFD = open(dst.c_str(), O_EXCL | O_CREAT | O_NONBLOCK | O_WRONLY, S_IRUSR | S_IWUSR); if (dstFD < 0) { close(srcFD); error = "failed to open destination file " + dst + ": " + strerror(errno); return false; } - TRI_FSTAT(srcFD, &statbuf); - dsize = statbuf.st_size; + bool rc = true; + try { + struct stat localStat; - bool rc = CopyFileContents(srcFD, dstFD, dsize, error); - timeval times[2]; - memset(times, 0, sizeof(times)); - times[0].tv_sec = TRI_STAT_ATIME_SEC(statbuf); - times[1].tv_sec = TRI_STAT_MTIME_SEC(statbuf); + if (statbuf == nullptr) { + TRI_FSTAT(srcFD, &localStat); + statbuf = &localStat; + } + TRI_ASSERT(statbuf != nullptr); - if (fchown(dstFD, -1 /*statbuf.st_uid*/, statbuf.st_gid) != 0) { - error = std::string("failed to chown ") + dst + ": " + strerror(errno); - // rc = false; no, this is not fatal... - } - if (fchmod(dstFD, statbuf.st_mode) != 0) { - error = std::string("failed to chmod ") + dst + ": " + strerror(errno); - rc = false; - } + size_t dsize = statbuf->st_size; + + if (dsize > 0) { + // only copy non-empty files + rc = CopyFileContents(srcFD, dstFD, dsize, error); + } + timeval times[2]; + memset(times, 0, sizeof(times)); + times[0].tv_sec = TRI_STAT_ATIME_SEC(*statbuf); + times[1].tv_sec = TRI_STAT_MTIME_SEC(*statbuf); + + if (fchown(dstFD, -1 /*statbuf.st_uid*/, statbuf->st_gid) != 0) { + error = std::string("failed to chown ") + dst + ": " + strerror(errno); + // rc = false; no, this is not fatal... + } + if (fchmod(dstFD, statbuf->st_mode) != 0) { + error = std::string("failed to chmod ") + dst + ": " + strerror(errno); + rc = false; + } #ifdef HAVE_FUTIMES - if (futimes(dstFD, times) != 0) { - error = std::string("failed to adjust age: ") + dst + ": " + strerror(errno); - rc = false; - } + if (futimes(dstFD, times) != 0) { + error = std::string("failed to adjust age: ") + dst + ": " + strerror(errno); + rc = false; + } #else - if (utimes(dst.c_str(), times) != 0) { - error = std::string("failed to adjust age: ") + dst + ": " + strerror(errno); + if (utimes(dst.c_str(), times) != 0) { + error = std::string("failed to adjust age: ") + dst + ": " + strerror(errno); + rc = false; + } +#endif + } catch (...) { + // make sure we always close file handles rc = false; } -#endif + close(srcFD); close(dstFD); return rc; -#endif } +#endif //////////////////////////////////////////////////////////////////////////////// /// @brief copies the filesystem attributes of a file @@ -2005,7 +2033,6 @@ bool TRI_CopySymlink(std::string const& srcItem, std::string const& dstItem, return true; } - //////////////////////////////////////////////////////////////////////////////// /// @brief creates a hard link; the link target is not altered. //////////////////////////////////////////////////////////////////////////////// diff --git a/lib/Basics/files.h b/lib/Basics/files.h index 0eda568de9e4..d03c63ea1ee7 100644 --- a/lib/Basics/files.h +++ b/lib/Basics/files.h @@ -42,6 +42,8 @@ #include "Enterprise/Encryption/EncryptionFeature.h" #endif +struct stat; + //////////////////////////////////////////////////////////////////////////////// /// @brief returns the size of a file /// @@ -351,7 +353,13 @@ ErrorCode TRI_GetTempName(char const* directory, std::string& result, bool creat /// @brief copies a file from source to dest. //////////////////////////////////////////////////////////////////////////////// +#ifdef _WIN32 bool TRI_CopyFile(std::string const& src, std::string const& dst, std::string& error); +#else +// this API allows passing already retrieved stat info to the copy routine, in +// order to avoid extra stat calls +bool TRI_CopyFile(std::string const& src, std::string const& dst, std::string& error, struct stat* statbuf = nullptr); +#endif //////////////////////////////////////////////////////////////////////////////// /// @brief copies the file Attributes from source to dest. diff --git a/lib/Basics/operating-system.h b/lib/Basics/operating-system.h index c91ba9a5eff1..e92c3f893c89 100644 --- a/lib/Basics/operating-system.h +++ b/lib/Basics/operating-system.h @@ -187,8 +187,8 @@ #define TRI_DUP ::dup #define TRI_RMDIR ::rmdir #define TRI_STAT ::stat -#define TRI_STAT_ATIME_SEC(statbuf) statbuf.st_atimespec.tv_sec -#define TRI_STAT_MTIME_SEC(statbuf) statbuf.st_mtimespec.tv_sec +#define TRI_STAT_ATIME_SEC(statbuf) (statbuf).st_atimespec.tv_sec +#define TRI_STAT_MTIME_SEC(statbuf) (statbuf).st_mtimespec.tv_sec #define TRI_UNLINK ::unlink #define TRI_WRITE ::write #define TRI_FDOPEN(a, b) ::fdopen((a), (b)) @@ -333,8 +333,8 @@ #define TRI_DUP ::dup #define TRI_RMDIR ::rmdir #define TRI_STAT ::stat -#define TRI_STAT_ATIME_SEC(statbuf) statbuf.st_atimespec.tv_sec -#define TRI_STAT_MTIME_SEC(statbuf) statbuf.st_mtimespec.tv_sec +#define TRI_STAT_ATIME_SEC(statbuf) (statbuf).st_atimespec.tv_sec +#define TRI_STAT_MTIME_SEC(statbuf) (statbuf).st_mtimespec.tv_sec #define TRI_UNLINK ::unlink #define TRI_WRITE ::write #define TRI_FDOPEN(a, b) ::fdopen((a), (b)) @@ -492,8 +492,8 @@ #define TRI_DUP ::dup #define TRI_RMDIR ::rmdir #define TRI_STAT ::stat -#define TRI_STAT_ATIME_SEC(statbuf) statbuf.st_atim.tv_sec -#define TRI_STAT_MTIME_SEC(statbuf) statbuf.st_mtim.tv_sec +#define TRI_STAT_ATIME_SEC(statbuf) (statbuf).st_atim.tv_sec +#define TRI_STAT_MTIME_SEC(statbuf) (statbuf).st_mtim.tv_sec #define TRI_UNLINK ::unlink #define TRI_WRITE ::write #define TRI_FDOPEN(a, b) ::fdopen((a), (b)) From 5089dc286aac88bb5f72b6c8442413f1304d6d27 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Fri, 16 Jul 2021 18:55:09 +0200 Subject: [PATCH 2/4] added test for copy installation --- .../test-copy-installation.js | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/tests/js/client/server_parameters/test-copy-installation.js b/tests/js/client/server_parameters/test-copy-installation.js index 2514859d1846..7e763a143049 100644 --- a/tests/js/client/server_parameters/test-copy-installation.js +++ b/tests/js/client/server_parameters/test-copy-installation.js @@ -1,5 +1,5 @@ /*jshint globalstrict:false, strict:false */ -/* global getOptions, assertEqual, arango */ +/* global getOptions, assertEqual, assertTrue, assertFalse, arango */ //////////////////////////////////////////////////////////////////////////////// /// @brief test for security-related server options @@ -34,7 +34,10 @@ if (getOptions === true) { 'javascript.allow-admin-execute': 'true', }; } -var jsunity = require('jsunity'); + +const db = require("@arangodb").db; +const fs = require("fs"); +const jsunity = require('jsunity'); function testSuite() { const errors = require('@arangodb').errors; @@ -44,12 +47,15 @@ function testSuite() { tearDown: function() {}, testCanExecuteAction : function() { - // this test just does something, and it doesn't really matter - // the key thing in this test is to get the server started using - // the `--javascript.copy-installation true` setting without failing - let data = "return 'test!'"; - let result = arango.POST("/_admin/execute", data); - assertEqual("test!", result); + // fetch server-side database directory name + let data = "return require('@arangodb').db._path();"; + let dbPath = arango.POST("/_admin/execute", data); + let jsPath = fs.join(dbPath, "js"); + assertTrue(fs.exists(jsPath)); + assertTrue(fs.exists(fs.join(jsPath, "node"))); + assertTrue(fs.exists(fs.join(jsPath, "node", "node_modules"))); + assertTrue(fs.exists(fs.join(jsPath, "node", "node_modules", "lodash"))); + assertFalse(fs.exists(fs.join(jsPath, "node", "eslint"))); }, }; From 0833fbb0ddaa8230f348e914f24737bb2b0185fa Mon Sep 17 00:00:00 2001 From: jsteemann Date: Fri, 16 Jul 2021 21:24:13 +0200 Subject: [PATCH 3/4] try to fix compile issue on Windows --- lib/Basics/FileUtils.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/Basics/FileUtils.cpp b/lib/Basics/FileUtils.cpp index 4918df47e7a5..92ab1840609a 100755 --- a/lib/Basics/FileUtils.cpp +++ b/lib/Basics/FileUtils.cpp @@ -444,6 +444,13 @@ bool copyDirectoryRecursive(std::string const& source, std::string const& target std::function const& filter, std::string& error) { 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(); + #ifdef TRI_HAVE_WIN32_LIST_FILES struct _wfinddata_t oneItem; @@ -476,12 +483,6 @@ bool copyDirectoryRecursive(std::string const& source, std::string const& target struct dirent* oneItem = nullptr; - // these strings will be recycled over and over - std::string dst = target + TRI_DIR_SEPARATOR_STR; - size_t dstPrefixLength = dst.size(); - std::string src = source + TRI_DIR_SEPARATOR_STR; - size_t srcPrefixLength = src.size(); - // do not use readdir_r() here anymore as it is not safe and deprecated // in newer versions of libc: // http://man7.org/linux/man-pages/man3/readdir_r.3.html From c9aed17bcd5e29410eb56a50f8dd7f43ce6a5795 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Mon, 19 Jul 2021 10:23:45 +0200 Subject: [PATCH 4/4] :facepalm: --- lib/Basics/FileUtils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Basics/FileUtils.cpp b/lib/Basics/FileUtils.cpp index 92ab1840609a..17d45cbef2c5 100755 --- a/lib/Basics/FileUtils.cpp +++ b/lib/Basics/FileUtils.cpp @@ -547,7 +547,7 @@ bool copyDirectoryRecursive(std::string const& source, std::string const& target rc_bool = false; } } else { -#ifndef _WIN32 +#ifdef _WIN32 rc_bool = TRI_CopyFile(src, dst, error); #else // optimized version that reuses the already retrieved stat data