From 61d9e8699687342c2e32c32c8d4eb71760d5d290 Mon Sep 17 00:00:00 2001 From: dec05eba Date: Sat, 26 Jun 2021 17:33:24 +0200 Subject: Use fork/exec instead of popen. Add Path class --- src/CmakeModule.cpp | 53 ++++++++-------- src/Conf.cpp | 26 +++----- src/Exec.cpp | 173 +++++++++++++++++++++++++++++++++++++--------------- src/GlobalLib.cpp | 89 ++++++++++----------------- src/PkgConfig.cpp | 83 ++++++------------------- src/main.cpp | 78 ++++++----------------- 6 files changed, 226 insertions(+), 276 deletions(-) (limited to 'src') diff --git a/src/CmakeModule.cpp b/src/CmakeModule.cpp index 5bf9400..a706e64 100644 --- a/src/CmakeModule.cpp +++ b/src/CmakeModule.cpp @@ -131,10 +131,10 @@ namespace sibs // CMakeLists.txt may contain: set(CMAKE_MODULE_PATH "PathToModules"). This needs to be replaced with list append, // otherwise our added module path is replaced. // It may work to do like vcpkg instead - to use -DCMAKE_TOOLCHAIN_FILE program argument to specify path to script (https://github.com/Microsoft/vcpkg/blob/master/docs/examples/using-sqlite.md) - vector globalLibDependencies; + vector globalLibDependencies; #if OS_FAMILY == OS_FAMILY_POSIX - vector pkgConfigDependencies; - for(PackageListDependency *dependency : config.getPackageListDependencies()) + vector pkgConfigDependencies; + for(const PackageListDependency &dependency : config.getPackageListDependencies()) { Result pkgConfigDependencyValidation = PkgConfig::validatePkgConfigPackageVersionExists(dependency); if(pkgConfigDependencyValidation.isOk()) @@ -153,7 +153,7 @@ namespace sibs { printf("%s, using global lib...\n", pkgConfigLinkerFlagsResult.getErrMsg().c_str()); globalLibDependencies.reserve(globalLibDependencies.size() + pkgConfigDependencies.size()); - for (PackageListDependency *pkgConfigDependency : pkgConfigDependencies) + for (const PackageListDependency &pkgConfigDependency : pkgConfigDependencies) { globalLibDependencies.push_back(pkgConfigDependency); } @@ -165,7 +165,7 @@ namespace sibs dynamicLinkerFlagCallbackFunc(pkgConfigLinkerFlagsResult.unwrap()); } #else - for (PackageListDependency *dependency : config.getPackageListDependencies()) + for (const PackageListDependency &dependency : config.getPackageListDependencies()) { globalLibDependencies.push_back(dependency); } @@ -194,8 +194,7 @@ namespace sibs _putenv("CXXFLAGS=-fPIC"); #endif #endif - FileString cmd = cmakePath; - cmd += TINYDIR_STRING(" "); + std::vector cmd = { cmakePath }; FileString cflags = TINYDIR_STRING("-fPIC"); FileString cxxflags; @@ -240,56 +239,57 @@ namespace sibs } cxxflags = cflags; - cmd += TINYDIR_STRING(" \"-DCMAKE_C_FLAGS=") + cflags + TINYDIR_STRING("\""); - cmd += TINYDIR_STRING(" \"-DCMAKE_CXX_FLAGS=") + cxxflags + TINYDIR_STRING("\" "); + cmd.push_back(TINYDIR_STRING("-DCMAKE_C_FLAGS=") + cflags); + cmd.push_back(TINYDIR_STRING("-DCMAKE_CXX_FLAGS=") + cxxflags); switch(config.getPackageType()) { case PackageType::EXECUTABLE: { - cmd += config.getCmakeArgs(); + auto cmake_args = config.getCmakeArgs(); + cmd.insert(cmd.end(), cmake_args.begin(), cmake_args.end()); break; } case PackageType::STATIC: { - cmd += config.getCmakeArgsStatic(); + auto cmake_args = config.getCmakeArgsStatic(); + cmd.insert(cmd.end(), cmake_args.begin(), cmake_args.end()); break; } case PackageType::DYNAMIC: case PackageType::LIBRARY: { - cmd += config.getCmakeArgsDynamic(); + auto cmake_args = config.getCmakeArgsDynamic(); + cmd.insert(cmd.end(), cmake_args.begin(), cmake_args.end()); break; } } - //cmd += TINYDIR_STRING(" -DCMAKE_SKIP_RPATH=\"1\""); - cmd += TINYDIR_STRING(" -DCMAKE_EXPORT_COMPILE_COMMANDS=ON"); - cmd += TINYDIR_STRING(" \"-B"); - cmd += buildPath; - cmd += TINYDIR_STRING("\" \"-H"); + //cmd.push_back(TINYDIR_STRING("-DCMAKE_SKIP_RPATH=1")); + cmd.push_back(TINYDIR_STRING("-DCMAKE_EXPORT_COMPILE_COMMANDS=ON")); + cmd.push_back(TINYDIR_STRING("-B") + buildPath); + cmd.push_back(TINYDIR_STRING("-H")); switch(config.getPackageType()) { case PackageType::EXECUTABLE: { - cmd += config.getCmakeDir(); + cmd.back() += config.getCmakeDir(); break; } case PackageType::STATIC: { - cmd += config.getCmakeDirStatic(); + cmd.back() += config.getCmakeDirStatic(); break; } case PackageType::DYNAMIC: case PackageType::LIBRARY: { - cmd += config.getCmakeDirDynamic(); + cmd.back() += config.getCmakeDirDynamic(); break; } } - cmd += TINYDIR_STRING("\""); - nprintf("Compiling with cmake with arguments: %s\n", cmd.c_str()); + //nprintf("Compiling with cmake with arguments: %s\n", cmd.c_str()); - Result execResult = exec(cmd.c_str(), true); + Result execResult = exec(cmd, true); if(execResult.isOk()) { if(execResult.unwrap().exitCode != 0) @@ -298,11 +298,8 @@ namespace sibs else return Result::Err(execResult); - FileString ninjaCommand = TINYDIR_STRING("ninja -C \""); - ninjaCommand += buildPath; - ninjaCommand += TINYDIR_STRING("\""); - nprintf("Compiling cmake generated ninja file: %s\n", ninjaCommand.c_str()); - execResult = exec(ninjaCommand.c_str(), true); + //nprintf("Compiling cmake generated ninja file: %s\n", buildPath.c_str()); + execResult = exec({ TINYDIR_STRING("ninja"), TINYDIR_STRING("-C"), buildPath }, true); if(execResult.isOk()) { if(execResult.unwrap().exitCode != 0) diff --git a/src/Conf.cpp b/src/Conf.cpp index 12421e0..543c8d9 100644 --- a/src/Conf.cpp +++ b/src/Conf.cpp @@ -643,13 +643,7 @@ namespace sibs SibsConfig::~SibsConfig() { - // TODO: Fix this shit.. why does this cause segfault? - /* - for(PackageListDependency *dependency : packageListDependencies) - { - //delete dependency; - } - */ + } bool SibsConfig::isDefined(const std::string &name) const @@ -1102,10 +1096,10 @@ namespace sibs if(!dependencyVersionResult) throw ParserException("Dependency " + string(name.data, name.size) + " version is in invalid format, error: " + dependencyVersionResult.getErrMsg()); - PackageListDependency *dependency = new PackageListDependency(); - dependency->name = string(name.data, name.size); - dependency->version = dependencyVersionResult.unwrap(); - packageListDependencies.emplace_back(dependency); + PackageListDependency dependency; + dependency.name = string(name.data, name.size); + dependency.version = dependencyVersionResult.unwrap(); + packageListDependencies.push_back(std::move(dependency)); } else throw ParserException("Expected field under dependencies to be a single value or an object, was a list"); @@ -1287,7 +1281,7 @@ namespace sibs getLibFiles(parsePlatformConfigStatic(fieldName, fieldValue), releaseStaticLibs); } - void SibsConfig::parseCmake(const StringView &fieldName, const ConfigValue &fieldValue, FileString &cmakeDir, FileString &cmakeArgs) + void SibsConfig::parseCmake(const StringView &fieldName, const ConfigValue &fieldValue, FileString &cmakeDir, std::vector &cmakeArgs) { if(fieldName.equals("dir")) { @@ -1314,13 +1308,7 @@ namespace sibs { for(const StringView &arg : fieldValue.asList()) { - bool prependSpace = !cmakeArgs.empty(); - cmakeArgs.reserve(cmakeArgs.size() + 4 + (prependSpace ? 1 : 0) + arg.size); - if(prependSpace) - cmakeArgs += TINYDIR_STRING(" "); - cmakeArgs += TINYDIR_STRING("\"-D"); - cmakeArgs += toFileString(arg); - cmakeArgs += TINYDIR_STRING("\""); + cmakeArgs.push_back(TINYDIR_STRING("-D") + toFileString(arg)); } } else diff --git a/src/Exec.cpp b/src/Exec.cpp index 169858f..12e373d 100644 --- a/src/Exec.cpp +++ b/src/Exec.cpp @@ -3,6 +3,7 @@ #if OS_FAMILY == OS_FAMILY_POSIX #include +#include #endif using namespace std; @@ -13,60 +14,129 @@ const int BUFSIZE = 4096; namespace sibs { #if OS_FAMILY == OS_FAMILY_POSIX - Result exec(const _tinydir_char_t *cmd, bool print) + Result exec(const std::vector &args, bool print_instead_of_pipe) { char buffer[BUFSIZE]; std::string execStdout; - FILE *pipe = popen(cmd, "r"); - if(!pipe) - return Result::Err("popen() failed"); - while(!feof(pipe)) - { - if(fgets(buffer, BUFSIZE, pipe)) - { - int bytesRead = strlen(buffer); - execStdout.append(buffer, bytesRead); - if(print) - printf("%.*s", bytesRead, buffer); + if(args.empty()) + return Result::Err("exec requires at least one argument (the program name)"); + + std::vector exec_args; + for(const FileString &arg : args) { + exec_args.push_back(arg.c_str()); + } + exec_args.push_back(nullptr); + + int fd[2]; + if(!print_instead_of_pipe && pipe(fd) == -1) + return Result::Err(strerror(errno)); + + pid_t pid = fork(); + if(pid == -1) { + if(!print_instead_of_pipe) { + close(fd[0]); + close(fd[1]); } + return Result::Err("Failed to exec " + args[0] + " (failed to fork)"); + } else if(pid == 0) { // child + if(!print_instead_of_pipe) { + dup2(fd[1], STDOUT_FILENO); + close(fd[0]); + close(fd[1]); + } + execvp(exec_args[0], (char* const*)exec_args.data()); + perror("execvp"); + _exit(127); + } else { // parent + if(!print_instead_of_pipe) + close(fd[1]); } - int processCloseResult = pclose(pipe); - if(WIFEXITED(processCloseResult)) + if(!print_instead_of_pipe) { + for(;;) { + ssize_t bytes_read = read(fd[0], buffer, sizeof(buffer)); + if(bytes_read == 0) { + break; + } else if(bytes_read == -1) { + std::string err_msg = "Failed to read from pipe to program " + args[0] + ", error: " + strerror(errno); + kill(pid, SIGTERM); + close(fd[0]); + return Result::Err(err_msg); + } + + execStdout.append(buffer, bytes_read); + } + } + + int status = 0; + if(waitpid(pid, &status, 0) == -1) { + std::string err_msg = std::string("waitpid failed, error: ") + strerror(errno); + if(!print_instead_of_pipe) + close(fd[0]); + return Result::Err(err_msg); + } + if(!print_instead_of_pipe) + close(fd[0]); + + if(WIFEXITED(status)) { - int returned = WEXITSTATUS(processCloseResult); + int returned = WEXITSTATUS(status); ExecResult execResult; execResult.execStdout = move(execStdout); execResult.exitCode = returned; return Result::Ok(execResult); } - else if(WIFSIGNALED(processCloseResult)) + else if(WIFSIGNALED(status)) { - int signum = WSTOPSIG(processCloseResult); + int signum = WSTOPSIG(status); string errMsg = "Exited due to receiving signal "; errMsg += to_string(signum); return Result::Err(errMsg); } - else if(WIFSTOPPED(processCloseResult)) + else if(WIFSTOPPED(status)) { - int signum = WSTOPSIG(processCloseResult); + int signum = WSTOPSIG(status); string errMsg = "Stopped due to receiving signal "; errMsg += to_string(signum); return Result::Err(errMsg); } else { - string errMsg = "exec unexpected error on pclose: "; - errMsg += to_string(processCloseResult); + string errMsg = "exec unexpected error on waitpid: "; + errMsg += to_string(status); return Result::Err(errMsg); } } + #else + static FileString escape_arg(const FileString &arg) { + FileString escaped = TINYDIR_STRING("\""); + for(_tinydir_char_t c : arg) { + if(c == '"') { + escaped += TINYDIR_STRING("\"\""); + } else { + escaped += c; + } + } + escaped += TINYDIR_STRING("\""); + return escaped; + } + + static FileString command_list_to_command_string(const std::vector &args) { + FileString cmd; + for(size_t i = 0; i < args.size(); ++i) { + if(i > 0) + cmd += TINYDIR_STRING(" "); + cmd += escape_arg(args[i]); + } + return cmd; + } + // Currently stdout is read in text mode so \n is replaced with \r\n, should we read in binary mode instead? - Result exec(const _tinydir_char_t *cmd, bool print) + Result exec(const std::vector &args, bool print_instead_of_pipe) { - FileString cmdNonConst = cmd; + FileString cmdNonConst = command_list_to_command_string(args); std::string execStdout; SECURITY_ATTRIBUTES saAttr; @@ -77,15 +147,17 @@ namespace sibs HANDLE childReadHandle = nullptr; HANDLE childStdoutHandle = nullptr; - if (!CreatePipe(&childReadHandle, &childStdoutHandle, &saAttr, 0)) - { - string errMsg = "exec unexpected error: "; - errMsg += toUtf8(getLastErrorAsString()); - return Result::Err(errMsg); - } + if(!print_instead_of_pipe) { + if (!CreatePipe(&childReadHandle, &childStdoutHandle, &saAttr, 0)) + { + string errMsg = "exec unexpected error: "; + errMsg += toUtf8(getLastErrorAsString()); + return Result::Err(errMsg); + } - if (!SetHandleInformation(childReadHandle, HANDLE_FLAG_INHERIT, 0)) - goto cleanupAndExit; + if (!SetHandleInformation(childReadHandle, HANDLE_FLAG_INHERIT, 0)) + goto cleanupAndExit; + } PROCESS_INFORMATION piProcInfo; ZeroMemory(&piProcInfo, sizeof(PROCESS_INFORMATION)); @@ -94,7 +166,7 @@ namespace sibs ZeroMemory(&siStartInfo, sizeof(STARTUPINFO)); siStartInfo.cb = sizeof(STARTUPINFO); siStartInfo.hStdError = nullptr; - siStartInfo.hStdOutput = childStdoutHandle; + siStartInfo.hStdOutput = print_instead_of_pipe ? nullptr : childStdoutHandle; siStartInfo.hStdInput = nullptr; siStartInfo.dwFlags |= STARTF_USESTDHANDLES; @@ -103,19 +175,22 @@ namespace sibs if (!CreateProcessW(nullptr, (LPWSTR)cmdNonConst.data(), nullptr, nullptr, TRUE, 0, nullptr, nullptr, &siStartInfo, &piProcInfo)) goto cleanupAndExit; - CloseHandle(childStdoutHandle); - childStdoutHandle = nullptr; - DWORD bytesRead; - CHAR buffer[BUFSIZE]; - while (true) - { - BOOL bSuccess = ReadFile(childReadHandle, buffer, BUFSIZE, &bytesRead, nullptr); - if (!bSuccess || bytesRead == 0) - break; + if(!print_instead_of_pipe) { + CloseHandle(childStdoutHandle); + childStdoutHandle = nullptr; + + DWORD bytesRead; + CHAR buffer[BUFSIZE]; + while (true) + { + BOOL bSuccess = ReadFile(childReadHandle, buffer, BUFSIZE, &bytesRead, nullptr); + if (!bSuccess || bytesRead == 0) + break; - execStdout.append(buffer, bytesRead); - if (print) - printf("%.*s", bytesRead, buffer); + execStdout.append(buffer, bytesRead); + if (print) + printf("%.*s", bytesRead, buffer); + } } WaitForSingleObject(piProcInfo.hProcess, INFINITE); @@ -134,13 +209,11 @@ namespace sibs cleanupAndExit: string errMsg = "exec unexpected error: "; errMsg += toUtf8(getLastErrorAsString()); - CloseHandle(childReadHandle); - CloseHandle(childStdoutHandle); + if(childReadHandle) + CloseHandle(childReadHandle); + if(childStdoutHandle) + CloseHandle(childStdoutHandle); return Result::Err(errMsg); } #endif - Result exec(const FileString &cmd, bool print) - { - return exec(cmd.c_str(), print); - } } diff --git a/src/GlobalLib.cpp b/src/GlobalLib.cpp index 8ad1678..b390571 100644 --- a/src/GlobalLib.cpp +++ b/src/GlobalLib.cpp @@ -15,25 +15,20 @@ namespace sibs { Result GlobalLib::validatePackageExists(const FileString &globalLibRootDir, const std::string &name) { - FileString packageDir = globalLibRootDir + TINYDIR_STRING("/"); -#if OS_FAMILY == OS_FAMILY_POSIX - packageDir += name; -#else - packageDir += utf8To16(name); -#endif - FileType packageDirFileType = getFileType(packageDir.c_str()); + Path packageDir = Path(globalLibRootDir).join(toFileString(name)); + FileType packageDirFileType = getFileType(packageDir.data.c_str()); switch(packageDirFileType) { case FileType::FILE_NOT_FOUND: { string errMsg = "Global lib dependency not found: "; - errMsg += toUtf8(packageDir); + errMsg += toUtf8(packageDir.data); return Result::Err(errMsg, DependencyError::DEPENDENCY_NOT_FOUND); } case FileType::REGULAR: { string errMsg = "Corrupt library directory. "; - errMsg += toUtf8(packageDir); + errMsg += toUtf8(packageDir.data); errMsg += " is a file, expected it to be a directory"; return Result::Err(errMsg); } @@ -54,10 +49,8 @@ namespace sibs if (!libPathResult) return Result::Err(libPathResult); - FileString libArchivedFilePath = libPathResult.unwrap(); - libArchivedFilePath += TINYDIR_STRING("/.cache/sibs/archive/"); - libArchivedFilePath += toFileString(name); - FileType archive_path_file_type = getFileType(libArchivedFilePath.c_str()); + Path libArchivedFilePath = Path(libPathResult.unwrap()).join(".cache/sibs/archive").join(toFileString(name)); + FileType archive_path_file_type = getFileType(libArchivedFilePath.data.c_str()); if(archive_path_file_type == FileType::FILE_NOT_FOUND) return Result::Ok(true); @@ -66,7 +59,7 @@ namespace sibs return Result::Err("A previous download of package is corrupt, attempting to redownload..."); bool isEmpty = true; - walkDir(libArchivedFilePath.c_str(), [&isEmpty](tinydir_file *file) + walkDir(libArchivedFilePath.data.c_str(), [&isEmpty](tinydir_file *file) { isEmpty = false; return false; @@ -78,13 +71,13 @@ namespace sibs return Result::Ok(true); } - Result GlobalLib::getLibs(const std::vector &libs, const SibsConfig &parentConfig, const FileString &globalLibRootDir, LinkerFlagCallbackFunc staticLinkerFlagCallbackFunc, LinkerFlagCallbackFunc dynamicLinkerFlagCallbackFunc, GlobalIncludeDirCallbackFunc globalIncludeDirCallback) + Result GlobalLib::getLibs(const std::vector &libs, const SibsConfig &parentConfig, const FileString &globalLibRootDir, LinkerFlagCallbackFunc staticLinkerFlagCallbackFunc, LinkerFlagCallbackFunc dynamicLinkerFlagCallbackFunc, GlobalIncludeDirCallbackFunc globalIncludeDirCallback) { - for(PackageListDependency *globalLibDependency : libs) + for(const PackageListDependency &globalLibDependency : libs) { if(!parentConfig.packaging) - printf("Dependency %s in version range %s is missing from pkg-config, trying global lib\n", globalLibDependency->name.c_str(), globalLibDependency->version.toString().c_str()); - Result globalLibLinkerFlagsResult = GlobalLib::getLibsLinkerFlags(parentConfig, globalLibRootDir, globalLibDependency->name, globalLibDependency->version, staticLinkerFlagCallbackFunc, dynamicLinkerFlagCallbackFunc, globalIncludeDirCallback); + printf("Dependency %s in version range %s is missing from pkg-config, trying global lib\n", globalLibDependency.name.c_str(), globalLibDependency.version.toString().c_str()); + Result globalLibLinkerFlagsResult = GlobalLib::getLibsLinkerFlags(parentConfig, globalLibRootDir, globalLibDependency.name, globalLibDependency.version, staticLinkerFlagCallbackFunc, dynamicLinkerFlagCallbackFunc, globalIncludeDirCallback); if(!globalLibLinkerFlagsResult) { if(globalLibLinkerFlagsResult.getErrorCode() == GlobalLib::DependencyError::DEPENDENCY_NOT_FOUND || globalLibLinkerFlagsResult.getErrorCode() == GlobalLib::DependencyError::DEPENDENCY_VERSION_NO_MATCH) @@ -103,7 +96,7 @@ namespace sibs if(!downloadDependencyResult) return downloadDependencyResult; - globalLibLinkerFlagsResult = GlobalLib::getLibsLinkerFlags(parentConfig, globalLibRootDir, globalLibDependency->name, globalLibDependency->version, staticLinkerFlagCallbackFunc, dynamicLinkerFlagCallbackFunc, globalIncludeDirCallback); + globalLibLinkerFlagsResult = GlobalLib::getLibsLinkerFlags(parentConfig, globalLibRootDir, globalLibDependency.name, globalLibDependency.version, staticLinkerFlagCallbackFunc, dynamicLinkerFlagCallbackFunc, globalIncludeDirCallback); if(!globalLibLinkerFlagsResult) return globalLibLinkerFlagsResult; } @@ -122,17 +115,9 @@ namespace sibs if (packageExistsResult.isErr()) return packageExistsResult; -#if OS_FAMILY == OS_FAMILY_POSIX - FileString namePlatformNative = name; -#else - FileString namePlatformNative = utf8To16(name); -#endif - - FileString packageDir = globalLibRootDir + TINYDIR_STRING("/"); - packageDir += namePlatformNative; - + Path packageDir = Path(globalLibRootDir).join(toFileString(name)); FileString foundVersion; - walkDir(packageDir.c_str(), [&foundVersion, &versionRange](tinydir_file *file) + walkDir(packageDir.data.c_str(), [&foundVersion, &versionRange](tinydir_file *file) { if(file->is_dir) { @@ -150,10 +135,8 @@ namespace sibs if(foundVersion.empty()) return Result::Err("Global lib dependency found, but version isn't in range of version", DependencyError::DEPENDENCY_VERSION_NO_MATCH); - packageDir += TINYDIR_STRING("/"); - packageDir += foundVersion; - - return GlobalLib::getLibsLinkerFlagsCommon(parentConfig, packageDir, name, staticLinkerFlagCallbackFunc, dynamicLinkerFlagCallbackFunc, globalIncludeDirCallback); + packageDir.join(foundVersion); + return GlobalLib::getLibsLinkerFlagsCommon(parentConfig, packageDir.data, name, staticLinkerFlagCallbackFunc, dynamicLinkerFlagCallbackFunc, globalIncludeDirCallback); } Result GlobalLib::getLibsLinkerFlagsCommon(const SibsConfig &parentConfig, const FileString &packageDir, const string &dependencyName, LinkerFlagCallbackFunc staticLinkerFlagCallbackFunc, LinkerFlagCallbackFunc dynamicLinkerFlagCallbackFunc, GlobalIncludeDirCallbackFunc globalIncludeDirCallback) @@ -237,9 +220,9 @@ namespace sibs } } - Result GlobalLib::downloadDependency(PackageListDependency *dependency, Platform platform) + Result GlobalLib::downloadDependency(const PackageListDependency &dependency, Platform platform) { - Result packageResult = Package::getPackage(dependency->name.c_str(), dependency->version, platform); + Result packageResult = Package::getPackage(dependency.name.c_str(), dependency.version, platform); if(!packageResult) return Result::Err(packageResult); @@ -248,41 +231,35 @@ namespace sibs Result libPathResult = getHomeDir(); if (!libPathResult) return Result::Err(libPathResult); - FileString libPath = libPathResult.unwrap(); - libPath += TINYDIR_STRING("/.cache/sibs/lib/"); - libPath += toFileString(asString(platform)); - libPath += TINYDIR_STRING("/"); - libPath += toFileString(dependency->name); - libPath += TINYDIR_STRING("/"); - libPath += toFileString(package.version.toString()); + Path libPath = Path(libPathResult.unwrap()) + .join(TINYDIR_STRING(".cache/sibs/lib")) + .join(toFileString(asString(platform))) + .join(toFileString(dependency.name)) + .join(toFileString(package.version.toString())); - FileString libArchivedFilePath = libPathResult.unwrap(); - libArchivedFilePath += TINYDIR_STRING("/.cache/sibs/archive/"); - libArchivedFilePath += toFileString(dependency->name); - Result createArchiveDirResult = createDirectoryRecursive(libArchivedFilePath.c_str()); + Path libArchivedFilePath = Path(libPathResult.unwrap()).join(TINYDIR_STRING("/.cache/sibs/archive/")).join(toFileString(dependency.name)); + Result createArchiveDirResult = createDirectoryRecursive(libArchivedFilePath.data.c_str()); if(!createArchiveDirResult) return createArchiveDirResult; - FileString libArchivedDir = libArchivedFilePath; - libArchivedFilePath += TINYDIR_STRING("/"); - libArchivedFilePath += toFileString(package.version.toString()); - Result downloadResult = curl::downloadFile(package.urls[0].c_str(), libArchivedFilePath.c_str()); + Path libArchivedDir = Path(libArchivedFilePath).join(toFileString(package.version.toString())); + Result downloadResult = curl::downloadFile(package.urls[0].c_str(), libArchivedFilePath.data.c_str()); if(!downloadResult) return downloadResult; // Create build path. This is done here because we dont want to create it if download fails - Result createLibDirResult = createDirectoryRecursive(libPath.c_str()); + Result createLibDirResult = createDirectoryRecursive(libPath.data.c_str()); if(!createLibDirResult) return createLibDirResult; - Result archiveExtractResult = Archive::extract(libArchivedFilePath.c_str(), libPath.c_str()); + Result archiveExtractResult = Archive::extract(libArchivedFilePath.data.c_str(), libPath.data.c_str()); // We have extracted the archive, we dont need to cache it. If remove fails, it doesn't really matter, user can remove it himself #if OS_FAMILY == OS_FAMILY_POSIX - remove(libArchivedFilePath.c_str()); - remove(libArchivedDir.c_str()); + remove(libArchivedFilePath.data.c_str()); + remove(libArchivedDir.data.c_str()); #else - _wremove(libArchivedFilePath.c_str()); - _wremove(libArchivedDir.c_str()); + _wremove(libArchivedFilePath.data.c_str()); + _wremove(libArchivedDir.data.c_str()); #endif return archiveExtractResult; } diff --git a/src/PkgConfig.cpp b/src/PkgConfig.cpp index 89d3a44..73df008 100644 --- a/src/PkgConfig.cpp +++ b/src/PkgConfig.cpp @@ -27,28 +27,25 @@ namespace sibs pkgConfigPath = path; } - Result PkgConfig::validatePkgConfigPackageVersionExists(PackageListDependency *dependency) + Result PkgConfig::validatePkgConfigPackageVersionExists(const PackageListDependency &dependency) { - Result dependencyValidationResult = PkgConfig::validatePackageExists(dependency->name); + Result dependencyValidationResult = PkgConfig::validatePackageExists(dependency.name); if(dependencyValidationResult.isErr()) return Result::Err(dependencyValidationResult.getErrMsg()); - Result dependencyVersionResult = PkgConfig::getPackageVersion(dependency->name); + Result dependencyVersionResult = PkgConfig::getPackageVersion(dependency.name); if(!dependencyVersionResult) return Result::Err(dependencyVersionResult); - if(!dependency->version.isInRange(dependencyVersionResult.unwrap())) - return Result::Err("pkg-config package " + dependency->name + " exists but the version does not match our expected version range"); + if(!dependency.version.isInRange(dependencyVersionResult.unwrap())) + return Result::Err("pkg-config package " + dependency.name + " exists but the version does not match our expected version range"); return Result::Ok(true); } Result PkgConfig::validatePackageExists(const string &name) { - FileString command = pkgConfigPath + TINYDIR_STRING(" --exists '"); - command += toFileString(name); - command += TINYDIR_STRING("'"); - Result execResult = exec(command.c_str()); + Result execResult = exec({ pkgConfigPath, TINYDIR_STRING("--exists"), TINYDIR_STRING("--"), toFileString(name) }); if(execResult.isErr()) return Result::Err(execResult.getErrMsg()); @@ -72,45 +69,9 @@ namespace sibs return Result::Ok(true); } - Result PkgConfig::validatePackageVersionAtLeast(const string &name, const string &version) - { - FileString command = pkgConfigPath + TINYDIR_STRING(" '--atleast-version="); - command += toFileString(version); - command += TINYDIR_STRING("' '"); - command += toFileString(name); - command += TINYDIR_STRING("'"); - Result execResult = exec(command.c_str()); - if(execResult.isErr()) - return Result::Err(execResult.getErrMsg()); - - if(execResult.unwrap().exitCode == 1) - { - string errMsg = "Dependency "; - errMsg += name; - errMsg += " is installed but the version older than "; - errMsg += version; - return Result::Err(errMsg); - } - else if(execResult.unwrap().exitCode == 127) - { - return Result::Err("pkg-config is not installed"); - } - else if(execResult.unwrap().exitCode != 0) - { - string errMsg = "Failed to check pkg-config package version, Unknown error, exit code: "; - errMsg += to_string(execResult.unwrap().exitCode); - return Result::Err(errMsg); - } - - return Result::Ok(true); - } - Result PkgConfig::getPackageVersion(const std::string &name) { - FileString command = pkgConfigPath + TINYDIR_STRING(" --modversion '"); - command += toFileString(name); - command += TINYDIR_STRING("'"); - Result execResult = exec(command.c_str()); + Result execResult = exec({ pkgConfigPath, TINYDIR_STRING("--modversion"), TINYDIR_STRING("--"), toFileString(name) }); if(!execResult) return Result::Err(execResult.getErrMsg()); @@ -136,21 +97,17 @@ namespace sibs return parsePackageVersion({ execResult.unwrap().execStdout.data(), execResult.unwrap().execStdout.size() }, nullptr); } - Result PkgConfig::getDynamicLibsLinkerFlags(const vector &libs) + Result PkgConfig::getDynamicLibsLinkerFlags(const vector &libs) { if(libs.empty()) return Result::Ok(""); - string args; - for(PackageListDependency *lib : libs) + std::vector args = { pkgConfigPath, TINYDIR_STRING("--libs"), TINYDIR_STRING("--") }; + for(const PackageListDependency &lib : libs) { - args += " '"; - args += lib->name; - args += "'"; + args.push_back(toFileString(lib.name)); } - FileString command = pkgConfigPath + TINYDIR_STRING(" --libs"); - command += toFileString(args); - Result execResult = exec(command.c_str()); + Result execResult = exec(args); if(execResult.isErr()) return Result::Err(execResult.getErrMsg()); @@ -179,21 +136,17 @@ namespace sibs } } - Result PkgConfig::getDynamicLibsCflags(const vector &libs) + Result PkgConfig::getDynamicLibsCflags(const vector &libs) { if(libs.empty()) return Result::Ok(""); - string args; - for(PackageListDependency *lib : libs) + std::vector args = { pkgConfigPath, TINYDIR_STRING("--cflags"), TINYDIR_STRING("--") }; + for(const PackageListDependency &lib : libs) { - args += " '"; - args += lib->name; - args += "'"; + args.push_back(toFileString(lib.name)); } - FileString command = pkgConfigPath + TINYDIR_STRING(" --cflags"); - command += toFileString(args); - Result execResult = exec(command.c_str()); + Result execResult = exec(args); if(execResult.isErr()) return Result::Err(execResult.getErrMsg()); @@ -222,7 +175,7 @@ namespace sibs } } - Result PkgConfig::getDynamicLibsFlags(const vector &libs) + Result PkgConfig::getDynamicLibsFlags(const vector &libs) { PkgConfigFlags flags; diff --git a/src/main.cpp b/src/main.cpp index 713514d..ecb0e99 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -80,9 +80,6 @@ using namespace std::chrono; // TODO: Add program command for generating compile_commands.json without compiling code, without using Ninja -// TODO: Make Process::exec safe to use. Currently you pass an argument and it's run as a command, but the string can be escaped to perform malicious acts. -// Process::exec should be modified to take a list of arguments to execute command with. - // TODO: Verify paths (test path, ignore dirs, include dirs, expose include dir, sibs test --file dir) are sub directory of the project // TODO: When creating a package with `sibs package` copy LICENSE files into archive. @@ -97,6 +94,8 @@ using namespace std::chrono; // TODO: If dependencies are using a version that is not within our dependency version range then ask the user if they still want to use the dependency (the closest matching dependency). // Currently if dependency version does not match, build will always fail with no option to ignore version mismatch. +// TODO: Check if ninja args are properly escaped. + #if OS_FAMILY == OS_FAMILY_POSIX #define fout std::cout #define ferr std::cerr @@ -314,7 +313,8 @@ struct MicrosoftBuildTool static MicrosoftBuildTool locateLatestMicrosoftBuildTool() { MicrosoftBuildTool result = { 0 }; - Result execResult = exec(TINYDIR_STRING("locate_windows_sdk x64")); + // TODO: x86 + Result execResult = exec({ TINYDIR_STRING("locate_windows_sdk"), TINYDIR_STRING("x64") }); if (execResult && execResult.unwrap().exitCode == 0) { auto &str = execResult.unwrap().execStdout; @@ -363,7 +363,7 @@ static void appendBuildToolToPathEnv() #endif } -static int buildProject(const FileString &projectPath, const FileString &projectConfFilePath, SibsConfig &sibsConfig, bool run, FileString run_args) +static int buildProject(const FileString &projectPath, const FileString &projectConfFilePath, SibsConfig &sibsConfig, bool run, const std::vector &run_args) { FileString buildPath; readSibsConfig(projectPath, projectConfFilePath, sibsConfig, buildPath); @@ -455,7 +455,11 @@ static int buildProject(const FileString &projectPath, const FileString &project FileString executableName = toFileString(sibsConfig.getPackageName()); if(isSamePlatformFamily(sibsConfig.platform, PLATFORM_WIN)) executableName += TINYDIR_STRING(".exe"); - auto exec_result = exec(buildPath + TINYDIR_STRING("/") + executableName + TINYDIR_STRING(" ") + run_args, true); + + std::vector args = { Path(buildPath).join(executableName).data }; + args.insert(args.end(), run_args.begin(), run_args.end()); + + auto exec_result = exec(args, true); if(!exec_result) { ferr << "Failed to execute" << (buildPath + TINYDIR_STRING("/") + executableName) << ", error: " << toFileString(exec_result.getErrMsg()) << endl; return 1; @@ -466,35 +470,6 @@ static int buildProject(const FileString &projectPath, const FileString &project return 0; } -#if OS_FAMILY == OS_FAMILY_WINDOWS -#define NATIVE_CHAR_PREFIX L -#else -#define NATIVE_CHAR_PREFIX -#endif - -static FileString replace_all(const _tinydir_char_t *str) { - FileString result = TINYDIR_STRING("'"); - while(*str != NATIVE_CHAR_PREFIX'\0') { - if(*str == NATIVE_CHAR_PREFIX'\'') - result += TINYDIR_STRING("\\'"); - else - result += *str; - ++str; - } - result += NATIVE_CHAR_PREFIX'\''; - return result; -} - -static FileString escape_args(const std::vector &args) { - FileString result; - for(const _tinydir_char_t *arg : args) { - if(!result.empty()) - result += NATIVE_CHAR_PREFIX' '; - result += replace_all(arg); - } - return result; -} - static Sanitize sanitize_string_to_type(const _tinydir_char_t *str) { if(strcmp(str, TINYDIR_STRING("address")) == 0) return Sanitize::ADDRESS; @@ -517,7 +492,7 @@ static int buildProject(int argc, const _tinydir_char_t **argv, bool run) Sanitize sanitize = Sanitize::NONE; FileString platformName; bool use_lto = false; - std::vector run_args; + std::vector run_args; for(int i = 0; i < argc; ++i) { @@ -652,7 +627,7 @@ static int buildProject(int argc, const _tinydir_char_t **argv, bool run) sibsConfig.platform = platform; sibsConfig.setSanitize(sanitize); sibsConfig.use_lto = use_lto; - return buildProject(projectPath, projectConfFilePath, sibsConfig, run, escape_args(run_args)); + return buildProject(projectPath, projectConfFilePath, sibsConfig, run, run_args); } static int testProject(int argc, const _tinydir_char_t **argv) @@ -797,7 +772,7 @@ static int testProject(int argc, const _tinydir_char_t **argv) sibsConfig.zigTestFiles = move(filesToTest); sibsConfig.zigTestAllFiles = testAllFiles; - return buildProject(projectPath, projectConfFilePath, sibsConfig, false, TINYDIR_STRING("")); + return buildProject(projectPath, projectConfFilePath, sibsConfig, false, {}); } // Returns nullptr if @charToFind is not found @@ -844,10 +819,7 @@ static void createProjectFile(const FileString &projectFilePath, const string &f // so there is no reason to do it (right now) static Result gitInitProject(const FileString &projectPath) { - FileString cmd = TINYDIR_STRING("git init \""); - cmd += projectPath; - cmd += TINYDIR_STRING("\""); - return exec(cmd.c_str()); + return exec({ TINYDIR_STRING("git"), TINYDIR_STRING("init"), TINYDIR_STRING("--"), projectPath }); } static bool gitIgnoreContainsSibs(const FileString &gitIgnoreFilePath) @@ -1236,7 +1208,7 @@ static int packageProject(int argc, const _tinydir_char_t **argv) sibsConfig.packaging = packagingType == PackagingType::STATIC; sibsConfig.bundling = (packagingType == PackagingType::BUNDLE) || (packagingType == PackagingType::BUNDLE_INSTALL); sibsConfig.use_lto = true; - int result = buildProject(projectPath, projectConfFilePath, sibsConfig, false, TINYDIR_STRING("")); + int result = buildProject(projectPath, projectConfFilePath, sibsConfig, false, {}); if(result != 0) return result; @@ -1244,8 +1216,8 @@ static int packageProject(int argc, const _tinydir_char_t **argv) { case PackagingType::STATIC: { - string packagePath = toUtf8(projectPath + TINYDIR_STRING("/sibs-build/") + toFileString(asString(sibsConfig.platform)) + TINYDIR_STRING("/package")); - printf("Project %s was successfully packaged and can be found at %s\n", sibsConfig.getPackageName().c_str(), packagePath.c_str()); + Path packagePath = Path(projectPath).join(TINYDIR_STRING("sibs-build")).join(toFileString(asString(sibsConfig.platform))).join(TINYDIR_STRING("package")); + printf("Project %s was successfully packaged and can be found at %s\n", sibsConfig.getPackageName().c_str(), packagePath.data.c_str()); break; } case PackagingType::BUNDLE: @@ -1262,21 +1234,11 @@ static int packageProject(int argc, const _tinydir_char_t **argv) break; } - FileString packagePath = projectPath + TINYDIR_STRING("/sibs-build/") + toFileString(asString(sibsConfig.platform)) + TINYDIR_STRING("/package"); - FileString executablePath = projectPath + TINYDIR_STRING("/sibs-build/") + toFileString(asString(sibsConfig.platform)) + TINYDIR_STRING("/release/")+ toFileString(sibsConfig.getPackageName()); + Path packagePath = Path(projectPath).join(TINYDIR_STRING("sibs-build")).join(toFileString(asString(sibsConfig.platform))).join(TINYDIR_STRING("package")); + Path executablePath = Path(projectPath).join(TINYDIR_STRING("sibs-build")).join(toFileString(asString(sibsConfig.platform))).join(TINYDIR_STRING("release")).join(toFileString(sibsConfig.getPackageName())); printf("Creating a package from project and dependencies...\n"); // args: executable_path program_version destination_path <--bundle|--bundle-install> - FileString cmd = TINYDIR_STRING("python3 \"") + - packageScriptPath + - TINYDIR_STRING("\" \"") + - executablePath + - TINYDIR_STRING("\" \"") + - toFileString(sibsConfig.version.toString()) + - TINYDIR_STRING("\" \"") + - packagePath + - TINYDIR_STRING("\" ") + - bundleType; - Result bundleResult = exec(cmd.c_str(), true); + Result bundleResult = exec({ TINYDIR_STRING("python3"), packageScriptPath, executablePath.data, toFileString(sibsConfig.version.toString()), packagePath.data, bundleType }, true); if(!bundleResult) { fprintf(stderr, "Error: failed to package project as a bundle, reason: %s\n", bundleResult.getErrMsg().c_str()); -- cgit v1.2.3