From 33d4cc41569cb138e908205791ef7c059bcca0d3 Mon Sep 17 00:00:00 2001 From: Anthony Printup <92564080+anthonyprintup@users.noreply.github.com> Date: Thu, 30 May 2024 00:36:14 +0200 Subject: [PATCH 1/2] Fixed globbing when multiple extensions are present in the file name --- docs/examples/globbing.md | 2 +- src/cmake_generator.cpp | 25 +++++++++++++------- tests/globbing/cmake.toml | 2 +- tests/globbing/mylib/include/mylib/mylib.hpp | 1 + tests/globbing/mylib/src/mylib/mylib.cxx | 1 + tests/globbing/mylib/src/mylib/mylib.gen.cxx | 5 ++++ 6 files changed, 25 insertions(+), 11 deletions(-) create mode 100644 tests/globbing/mylib/src/mylib/mylib.cxx create mode 100644 tests/globbing/mylib/src/mylib/mylib.gen.cxx diff --git a/docs/examples/globbing.md b/docs/examples/globbing.md index 5ff0b65..326216e 100644 --- a/docs/examples/globbing.md +++ b/docs/examples/globbing.md @@ -20,7 +20,7 @@ description = "Globbing sources" [target.mylib] type = "static" alias = "mylib::mylib" -sources = ["mylib/**.hpp", "mylib/**.cpp"] +sources = ["mylib/**.hpp", "mylib/**.cpp", "mylib/**.gen.cxx"] include-directories = ["mylib/include"] # Single-folder glob in example/src/ diff --git a/src/cmake_generator.cpp b/src/cmake_generator.cpp index f3ed4b0..7ec1368 100644 --- a/src/cmake_generator.cpp +++ b/src/cmake_generator.cpp @@ -55,31 +55,38 @@ static std::string format(const char *format, const tsl::ordered_map expand_cmake_path(const fs::path &name, const fs::path &toml_dir, bool is_root_project) { - std::vector temp; - auto extract_suffix = [](const fs::path &base, const fs::path &full) { + auto const extract_suffix = [](const fs::path &base, const fs::path &full) { auto fullpath = full.string(); auto base_len = base.string().length(); - auto delet = fullpath.substr(base_len + 1, fullpath.length() - base_len); - return delet; + return fullpath.substr(base_len + 1, fullpath.length() - base_len); }; - - auto stem = name.filename().stem().string(); - auto ext = name.extension(); + auto const extract_path_parts = [](const fs::path &file_path) -> std::pair { + const auto path_as_string = file_path.string(); + auto const dot_position = path_as_string.find('.'); + if (dot_position != std::string::npos) { + return {path_as_string.substr(0, dot_position), path_as_string.substr(dot_position)}; + } + return {path_as_string, {}}; + }; + auto const path_parts = extract_path_parts(name.filename()); + auto const &stem = path_parts.first; + auto const &extension = path_parts.second; if (is_root_project && stem == "**" && name == name.filename()) { throw std::runtime_error("Recursive globbing not allowed in project root: " + name.string()); } + std::vector temp; if (stem == "*") { for (const auto &f : fs::directory_iterator(toml_dir / name.parent_path(), fs::directory_options::follow_directory_symlink)) { - if (!f.is_directory() && f.path().extension() == ext) { + if (!f.is_directory() && extract_path_parts(f.path().filename()).second == extension) { temp.push_back(extract_suffix(toml_dir, f)); } } } else if (stem == "**") { for (const auto &f : fs::recursive_directory_iterator(toml_dir / name.parent_path(), fs::directory_options::follow_directory_symlink)) { - if (!f.is_directory() && f.path().extension() == ext) { + if (!f.is_directory() && extract_path_parts(f.path().filename()).second == extension) { temp.push_back(extract_suffix(toml_dir, f.path())); } } diff --git a/tests/globbing/cmake.toml b/tests/globbing/cmake.toml index e174f84..4afe427 100644 --- a/tests/globbing/cmake.toml +++ b/tests/globbing/cmake.toml @@ -6,7 +6,7 @@ description = "Globbing sources" [target.mylib] type = "static" alias = "mylib::mylib" -sources = ["mylib/**.hpp", "mylib/**.cpp"] +sources = ["mylib/**.hpp", "mylib/**.cpp", "mylib/**.gen.cxx"] include-directories = ["mylib/include"] # Single-folder glob in example/src/ diff --git a/tests/globbing/mylib/include/mylib/mylib.hpp b/tests/globbing/mylib/include/mylib/mylib.hpp index 3774e8f..bffd6fa 100644 --- a/tests/globbing/mylib/include/mylib/mylib.hpp +++ b/tests/globbing/mylib/include/mylib/mylib.hpp @@ -4,4 +4,5 @@ namespace mylib { std::string message(); +std::string message2(); } diff --git a/tests/globbing/mylib/src/mylib/mylib.cxx b/tests/globbing/mylib/src/mylib/mylib.cxx new file mode 100644 index 0000000..bf14392 --- /dev/null +++ b/tests/globbing/mylib/src/mylib/mylib.cxx @@ -0,0 +1 @@ +#error This file should not be included in the sources diff --git a/tests/globbing/mylib/src/mylib/mylib.gen.cxx b/tests/globbing/mylib/src/mylib/mylib.gen.cxx new file mode 100644 index 0000000..c700be6 --- /dev/null +++ b/tests/globbing/mylib/src/mylib/mylib.gen.cxx @@ -0,0 +1,5 @@ +#include + +std::string mylib::message2() { + return "cmkr is awesome2!"; +} From f32aac63aa4f1dfcf29fd3cb6f0c4a48c8867d34 Mon Sep 17 00:00:00 2001 From: Duncan Ogilvie Date: Fri, 31 May 2024 21:25:38 +0200 Subject: [PATCH 2/2] Review improvements --- CMakeLists.txt | 18 ++-- docs/examples/globbing.md | 2 +- src/cmake_generator.cpp | 97 ++++++++++++-------- tests/globbing/cmake.toml | 2 +- tests/globbing/mylib/include/mylib/mylib.hpp | 3 +- tests/globbing/mylib/src/mylib/mylib.cxx | 1 - tests/globbing/mylib/src/mylib/mylib.gen.cxx | 5 - 7 files changed, 71 insertions(+), 57 deletions(-) delete mode 100644 tests/globbing/mylib/src/mylib/mylib.cxx delete mode 100644 tests/globbing/mylib/src/mylib/mylib.gen.cxx diff --git a/CMakeLists.txt b/CMakeLists.txt index 2686571..eef5f0c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -58,12 +58,9 @@ generate_documentation() # Target: cmkr set(cmkr_SOURCES - "src/arguments.cpp" - "src/build.cpp" - "src/cmake_generator.cpp" - "src/help.cpp" - "src/main.cpp" - "src/project_parser.cpp" + cmake.toml + "cmake/cmkr.cmake" + "cmake/version.hpp.in" "include/arguments.hpp" "include/build.hpp" "include/cmake_generator.hpp" @@ -71,9 +68,12 @@ set(cmkr_SOURCES "include/help.hpp" "include/literals.hpp" "include/project_parser.hpp" - "cmake/cmkr.cmake" - "cmake/version.hpp.in" - cmake.toml + "src/arguments.cpp" + "src/build.cpp" + "src/cmake_generator.cpp" + "src/help.cpp" + "src/main.cpp" + "src/project_parser.cpp" ) add_executable(cmkr) diff --git a/docs/examples/globbing.md b/docs/examples/globbing.md index 326216e..5ff0b65 100644 --- a/docs/examples/globbing.md +++ b/docs/examples/globbing.md @@ -20,7 +20,7 @@ description = "Globbing sources" [target.mylib] type = "static" alias = "mylib::mylib" -sources = ["mylib/**.hpp", "mylib/**.cpp", "mylib/**.gen.cxx"] +sources = ["mylib/**.hpp", "mylib/**.cpp"] include-directories = ["mylib/include"] # Single-folder glob in example/src/ diff --git a/src/cmake_generator.cpp b/src/cmake_generator.cpp index 7ec1368..75118ff 100644 --- a/src/cmake_generator.cpp +++ b/src/cmake_generator.cpp @@ -54,64 +54,85 @@ static std::string format(const char *format, const tsl::ordered_map expand_cmake_path(const fs::path &name, const fs::path &toml_dir, bool is_root_project) { - - auto const extract_suffix = [](const fs::path &base, const fs::path &full) { - auto fullpath = full.string(); - auto base_len = base.string().length(); - return fullpath.substr(base_len + 1, fullpath.length() - base_len); - }; - auto const extract_path_parts = [](const fs::path &file_path) -> std::pair { - const auto path_as_string = file_path.string(); - auto const dot_position = path_as_string.find('.'); - if (dot_position != std::string::npos) { - return {path_as_string.substr(0, dot_position), path_as_string.substr(dot_position)}; +static std::vector expand_cmake_path(const fs::path &source_path, const fs::path &toml_dir, bool is_root_project) { + auto is_subdir = [](fs::path p, const fs::path &root) { + while (true) { + if (p == root) { + return true; + } + auto parent = p.parent_path(); + if (parent == p) { + break; + } + p = parent; } - return {path_as_string, {}}; + return false; }; - auto const path_parts = extract_path_parts(name.filename()); - auto const &stem = path_parts.first; - auto const &extension = path_parts.second; + if (!is_subdir(fs::absolute(toml_dir / source_path), toml_dir)) { + throw std::runtime_error("Path traversal is not allowed: " + source_path.string()); + } + + // Split the path at the first period (since fs::path::stem() and fs::path::extension() split at the last period) + std::string stem, extension; + auto filename = source_path.filename().string(); + auto dot_position = filename.find('.'); + if (dot_position != std::string::npos) { + stem = filename.substr(0, dot_position); + extension = filename.substr(dot_position); + } else { + stem = filename; + } - if (is_root_project && stem == "**" && name == name.filename()) { - throw std::runtime_error("Recursive globbing not allowed in project root: " + name.string()); + if (is_root_project && stem == "**" && !source_path.has_parent_path()) { + throw std::runtime_error("Recursive globbing not allowed in project root: " + source_path.string()); } - std::vector temp; + auto has_extension = [](const fs::path &file_path, const std::string &extension) { + auto path = file_path.string(); + return path.rfind(extension) == path.length() - extension.length(); + }; + + std::vector paths; if (stem == "*") { - for (const auto &f : fs::directory_iterator(toml_dir / name.parent_path(), fs::directory_options::follow_directory_symlink)) { - if (!f.is_directory() && extract_path_parts(f.path().filename()).second == extension) { - temp.push_back(extract_suffix(toml_dir, f)); + for (const auto &f : fs::directory_iterator(toml_dir / source_path.parent_path(), fs::directory_options::follow_directory_symlink)) { + if (!f.is_directory() && has_extension(f.path(), extension)) { + paths.push_back(fs::relative(f, toml_dir)); } } } else if (stem == "**") { - for (const auto &f : fs::recursive_directory_iterator(toml_dir / name.parent_path(), fs::directory_options::follow_directory_symlink)) { - if (!f.is_directory() && extract_path_parts(f.path().filename()).second == extension) { - temp.push_back(extract_suffix(toml_dir, f.path())); + for (const auto &f : + fs::recursive_directory_iterator(toml_dir / source_path.parent_path(), fs::directory_options::follow_directory_symlink)) { + if (!f.is_directory() && has_extension(f.path(), extension)) { + paths.push_back(fs::relative(f, toml_dir)); } } } else { - temp.push_back(name.string()); + paths.push_back(source_path); } - // Normalize all paths to work with CMake (it needs a / on Windows as well) - for (auto &path : temp) { - std::replace(path.begin(), path.end(), '\\', '/'); - } - // Sort paths alphabetically for consistent cross-OS generation - std::sort(temp.begin(), temp.end()); - return temp; + + return paths; } static std::vector expand_cmake_paths(const std::vector &sources, const fs::path &toml_dir, bool is_root_project) { - // TODO: add duplicate checking - std::vector result; + std::vector paths; for (const auto &src : sources) { auto expanded = expand_cmake_path(src, toml_dir, is_root_project); for (const auto &f : expanded) { - result.push_back(f); + paths.push_back(f.string()); } } - return result; + + // Normalize all paths to work with CMake (it needs a / on Windows as well) + for (auto &path : paths) { + std::replace(path.begin(), path.end(), '\\', '/'); + } + + // Sort paths alphabetically for consistent cross-OS generation + std::sort(paths.begin(), paths.end()); + + // TODO: remove duplicates + + return paths; } static void create_file(const fs::path &path, const std::string &contents) { @@ -681,7 +702,7 @@ void generate_cmake(const char *path, const parser::Project *parent_project) { parser::Project project(parent_project, path, false); - for (auto const &lang : project.project_languages) { + for (const auto &lang : project.project_languages) { if (known_languages.find(lang) == known_languages.end()) { if (project.project_allow_unknown_languages) { printf("[warning] Unknown language '%s' specified\n", lang.c_str()); diff --git a/tests/globbing/cmake.toml b/tests/globbing/cmake.toml index 4afe427..e174f84 100644 --- a/tests/globbing/cmake.toml +++ b/tests/globbing/cmake.toml @@ -6,7 +6,7 @@ description = "Globbing sources" [target.mylib] type = "static" alias = "mylib::mylib" -sources = ["mylib/**.hpp", "mylib/**.cpp", "mylib/**.gen.cxx"] +sources = ["mylib/**.hpp", "mylib/**.cpp"] include-directories = ["mylib/include"] # Single-folder glob in example/src/ diff --git a/tests/globbing/mylib/include/mylib/mylib.hpp b/tests/globbing/mylib/include/mylib/mylib.hpp index bffd6fa..e449c9b 100644 --- a/tests/globbing/mylib/include/mylib/mylib.hpp +++ b/tests/globbing/mylib/include/mylib/mylib.hpp @@ -4,5 +4,4 @@ namespace mylib { std::string message(); -std::string message2(); -} +} // namespace mylib diff --git a/tests/globbing/mylib/src/mylib/mylib.cxx b/tests/globbing/mylib/src/mylib/mylib.cxx deleted file mode 100644 index bf14392..0000000 --- a/tests/globbing/mylib/src/mylib/mylib.cxx +++ /dev/null @@ -1 +0,0 @@ -#error This file should not be included in the sources diff --git a/tests/globbing/mylib/src/mylib/mylib.gen.cxx b/tests/globbing/mylib/src/mylib/mylib.gen.cxx deleted file mode 100644 index c700be6..0000000 --- a/tests/globbing/mylib/src/mylib/mylib.gen.cxx +++ /dev/null @@ -1,5 +0,0 @@ -#include - -std::string mylib::message2() { - return "cmkr is awesome2!"; -}