From de2f26c71c2e9613f4468362de5bbd4ab8603255 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Thu, 17 Sep 2026 08:43:05 -0700 Subject: [PATCH 1/2] Search for libraries where OpenSCAD searches Three divergences from OpenSCAD's own parser_init(), any of which loses a library that OpenSCAD itself finds (BelfrySCAD #503): - OPENSCADPATH *replaced* the default libraries folder rather than being searched before it (`env = envPath ? envPath : dfltPath`), so setting it for one library hid every other, including a BOSL2 in the default folder. It now comes first and adds to the built-in dirs. - Windows assumed %USERPROFILE%\Documents. OneDrive's Known Folder Move -- on by default -- puts the real My Documents at %USERPROFILE%\OneDrive\ Documents, so a library OpenSCAD installed sat somewhere we never looked. Ask Windows instead, via the same SHGetFolderPathW(CSIDL_PERSONAL, SHGFP_TYPE_CURRENT) call OpenSCAD makes. - Libraries shipped beside the binary (OpenSCAD's resourcePath("libraries")) were never searched at all. Now they are, resolved from this code's own module path. The search order is public as librarySearchDirs(), and the not-found error lists every directory it tried. The old message named only the includer -- "Searched relative to: foo.scad" -- which is why the bug was filed as "search is only done relative to document folder" against a search that does cover the libraries folder. Co-Authored-By: Claude Opus 5 (1M context) --- include/openscad_cpp_parser/api.hpp | 11 ++- src/CMakeLists.txt | 5 + src/api.cpp | 141 ++++++++++++++++++++++------ tests/test_comments_and_files.cpp | 86 +++++++++++++++++ 4 files changed, 212 insertions(+), 31 deletions(-) diff --git a/include/openscad_cpp_parser/api.hpp b/include/openscad_cpp_parser/api.hpp index 835baa5..6043682 100644 --- a/include/openscad_cpp_parser/api.hpp +++ b/include/openscad_cpp_parser/api.hpp @@ -136,9 +136,14 @@ struct LibraryFileResult { LibraryFileResult getASTFromLibraryFile(const std::string& currFile, const std::string& libFile, bool includeComments = false, bool processIncludes = true); -// OpenSCAD's library search path: (1) directory of currFile, (2) -// OPENSCADPATH env var (':'-separated on POSIX, ';' on Windows), (3) -// platform default library dir. +// OpenSCAD's library search path, in order: (1) directory of currFile, +// (2) each OPENSCADPATH entry (':'-separated on POSIX, ';' on Windows), +// (3) the user's own libraries folder, (4) libraries shipped beside this +// binary. OPENSCADPATH ADDS to (3) and (4) rather than replacing them, +// which is what OpenSCAD's own parser_init() does. +std::vector librarySearchDirs(const std::string& currFile); + +// The first entry of librarySearchDirs() that holds `libFile`. std::optional findLibraryFile(const std::string& currFile, const std::string& libFile); // A file's statements with its `include <...>` directives resolved, where diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8a5e156..191ec80 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -39,6 +39,11 @@ target_include_directories(openscad_cpp_parser PRIVATE ${CMAKE_CURRENT_BINARY_DIR} ) target_link_libraries(openscad_cpp_parser PUBLIC nlohmann_json::nlohmann_json) +# SHGetFolderPathW, for the real My Documents rather than an assumed +# %USERPROFILE%\Documents -- see api.cpp userLibraryDir(). +if(WIN32) + target_link_libraries(openscad_cpp_parser PUBLIC shell32) +endif() target_compile_options(openscad_cpp_parser PRIVATE $<$>:-Wall;-Wextra> ) diff --git a/src/api.cpp b/src/api.cpp index 33617e5..369adb0 100644 --- a/src/api.cpp +++ b/src/api.cpp @@ -7,6 +7,12 @@ #include #include +#if defined(_WIN32) +#include +#include +#else +#include +#endif #include #include #include @@ -133,6 +139,22 @@ std::vector> getASTFromString(const std::string& code, namespace { +// A "not found" that says where we actually looked. The old message named +// only the includer, so "searched relative to X" read as "only X was +// searched" -- which is how #503 came to be filed against a search that +// does cover the libraries folder. +std::string notFound(const char* noun, const std::string& filename, const std::string& currentFile) { + std::string msg = std::string(noun) + " '" + filename + "' not found. Searched:"; + for (const auto& d : librarySearchDirs(currentFile)) { + msg += "\n " + d; + } + return msg; +} + +} // namespace + +namespace { + std::string readFile(const std::string& path) { std::ifstream in(path, std::ios::binary); if (!in) { @@ -163,8 +185,7 @@ std::vector> resolveIncludes(std::vectorval; auto libFile = findLibraryFile(currentFile, filename); if (!libFile) { - throw std::runtime_error("Included file '" + filename + "' not found. Searched relative to: " + - (currentFile.empty() ? "current directory" : currentFile)); + throw std::runtime_error(notFound("Included file", filename, currentFile)); } std::string absLib = fs::absolute(*libFile).string(); if (visited.count(absLib) != 0) { @@ -185,40 +206,98 @@ std::vector> resolveIncludes(std::vector findLibraryFile(const std::string& currFile, const std::string& libFile) { - std::vector dirs; - if (!currFile.empty()) { - dirs.push_back(fs::absolute(currFile).parent_path()); - } +namespace { - char pathsep = ':'; - std::string dfltPath; - const char* home = std::getenv("HOME"); +// The user's own libraries folder -- OpenSCAD's +// PlatformUtils::userLibraryPath(), and the one an installer or a git clone +// of BOSL2 lands in. +std::string userLibraryDir() { #if defined(_WIN32) - pathsep = ';'; + // ASK Windows where My Documents is rather than assuming + // %USERPROFILE%\Documents. OneDrive's Known Folder Move -- on by default + // on a new machine -- relocates it to %USERPROFILE%\OneDrive\Documents, + // and a library OpenSCAD itself installed then sat somewhere we never + // looked (BelfrySCAD #503). Same call and same flag OpenSCAD makes + // (PlatformUtils-win.cc getFolderPath). + wchar_t buf[MAX_PATH] = {0}; + if (SHGetFolderPathW(nullptr, CSIDL_PERSONAL, nullptr, SHGFP_TYPE_CURRENT, buf) == S_OK) { + return (fs::path(buf) / "OpenSCAD" / "libraries").string(); + } const char* userProfile = std::getenv("USERPROFILE"); - if (userProfile) { - dfltPath = std::string(userProfile) + "\\Documents\\OpenSCAD\\libraries"; + return userProfile ? (fs::path(userProfile) / "Documents" / "OpenSCAD" / "libraries").string() + : std::string(); +#else + const char* home = std::getenv("HOME"); + if (!home) { + return {}; } -#elif defined(__APPLE__) - if (home) { - dfltPath = std::string(home) + "/Documents/OpenSCAD/libraries"; +#if defined(__APPLE__) + return (fs::path(home) / "Documents" / "OpenSCAD" / "libraries").string(); +#else + return (fs::path(home) / ".local" / "share" / "OpenSCAD" / "libraries").string(); +#endif +#endif +} + +// Libraries shipped alongside this build -- OpenSCAD's +// PlatformUtils::resourcePath("libraries"). "Alongside" means beside the +// binary this code was linked into: the CLI executable, or the Python +// extension inside its installed package. +// ponytail: one candidate directory, no ../share/openscad/libraries walk -- +// add the walk if a packaging layout ever puts the libraries somewhere +// other than next to the binary. +std::string bundledLibraryDir() { + fs::path self; +#if defined(_WIN32) + HMODULE mod = nullptr; + if (GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS + | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, + reinterpret_cast(&userLibraryDir), &mod)) { + wchar_t buf[MAX_PATH] = {0}; + if (GetModuleFileNameW(mod, buf, MAX_PATH)) { + self = buf; + } } #else - if (home) { - dfltPath = std::string(home) + "/.local/share/OpenSCAD/libraries"; + Dl_info info; + if (dladdr(reinterpret_cast(&userLibraryDir), &info) && info.dli_fname) { + self = info.dli_fname; } #endif + if (self.empty()) { + return {}; + } + std::error_code ec; + return (fs::absolute(self, ec).parent_path() / "libraries").string(); +} + +} // namespace +std::vector librarySearchDirs(const std::string& currFile) { + std::vector dirs; + if (!currFile.empty()) { + dirs.push_back(fs::absolute(currFile).parent_path().string()); + } + +#if defined(_WIN32) + const char pathsep = ';'; +#else + const char pathsep = ':'; +#endif + + // OPENSCADPATH comes FIRST and adds to the built-in paths rather than + // replacing them -- exactly what OpenSCAD's parser_init() does. It used + // to replace them, so setting the variable for one library hid every + // other, including a BOSL2 sitting in the default folder (#503). const char* envPath = std::getenv("OPENSCADPATH"); - std::string env = envPath ? std::string(envPath) : dfltPath; - if (!env.empty()) { + if (envPath) { + std::string env(envPath); size_t start = 0; while (start <= env.size()) { size_t pos = env.find(pathsep, start); std::string part = (pos == std::string::npos) ? env.substr(start) : env.substr(start, pos - start); if (!part.empty()) { - dirs.emplace_back(part); + dirs.push_back(part); } if (pos == std::string::npos) { break; @@ -227,8 +306,17 @@ std::optional findLibraryFile(const std::string& currFile, const st } } - for (const auto& d : dirs) { - fs::path candidate = d / libFile; + for (const std::string& d : {userLibraryDir(), bundledLibraryDir()}) { + if (!d.empty()) { + dirs.push_back(d); + } + } + return dirs; +} + +std::optional findLibraryFile(const std::string& currFile, const std::string& libFile) { + for (const auto& d : librarySearchDirs(currFile)) { + fs::path candidate = fs::path(d) / libFile; std::error_code ec; if (fs::is_regular_file(candidate, ec)) { return candidate.string(); @@ -255,9 +343,7 @@ LibraryFileResult getASTFromLibraryFile(const std::string& currFile, const std:: bool processIncludes) { auto found = findLibraryFile(currFile, libFile); if (!found) { - throw std::runtime_error("Library file '" + libFile + - "' not found in search paths. Searched in: current file directory, OPENSCADPATH, and " - "platform default paths."); + throw std::runtime_error(notFound("Library file", libFile, currFile)); } auto ast = getASTFromFile(*found, includeComments, processIncludes); return LibraryFileResult{std::move(ast), *found}; @@ -346,8 +432,7 @@ void collectProgram(const FileAst& nodes, const std::string& currentFile, bool i const std::string& filename = inc.filepath->val; auto libFile = findLibraryFile(currentFile, filename); if (!libFile) { - throw std::runtime_error("Included file '" + filename + "' not found. Searched relative to: " + - (currentFile.empty() ? "current directory" : currentFile)); + throw std::runtime_error(notFound("Included file", filename, currentFile)); } std::string absLib = fs::absolute(*libFile).string(); if (!visited.insert(absLib).second) continue; diff --git a/tests/test_comments_and_files.cpp b/tests/test_comments_and_files.cpp index 43ff93d..7c9c1a8 100644 --- a/tests/test_comments_and_files.cpp +++ b/tests/test_comments_and_files.cpp @@ -2,6 +2,8 @@ #include +#include +#include #include #include #include @@ -138,3 +140,87 @@ TEST(FileApi, FindLibraryFileReturnsNulloptWhenMissing) { auto found = findLibraryFile("", "definitely_missing_file_xyz.scad"); EXPECT_FALSE(found.has_value()); } + +namespace { + +// RAII OPENSCADPATH, so one test's setting cannot leak into the next. +class ScopedOpenscadPath { +public: + explicit ScopedOpenscadPath(const std::string& value) { + const char* prev = std::getenv("OPENSCADPATH"); + had_ = prev != nullptr; + if (had_) prev_ = prev; + set(value.c_str()); + } + ~ScopedOpenscadPath() { + if (had_) { + set(prev_.c_str()); + } else { +#if defined(_WIN32) + _putenv_s("OPENSCADPATH", ""); +#else + unsetenv("OPENSCADPATH"); +#endif + } + } +private: + static void set(const char* v) { +#if defined(_WIN32) + _putenv_s("OPENSCADPATH", v); +#else + setenv("OPENSCADPATH", v, 1); +#endif + } + bool had_ = false; + std::string prev_; +}; + +} // namespace + +TEST(FileApi, OpenscadPathAddsToTheBuiltInDirsRatherThanReplacingThem) { + // BelfrySCAD #503: `env = envPath ? envPath : dfltPath` meant setting + // OPENSCADPATH for one library hid every library in the default folder. + std::vector without = librarySearchDirs(""); + TempDir dir; + ScopedOpenscadPath scoped(dir.path().string()); + std::vector with = librarySearchDirs(""); + + EXPECT_EQ(with.size(), without.size() + 1); + EXPECT_EQ(with.front(), dir.path().string()); // and it is searched FIRST + for (const auto& d : without) { + EXPECT_NE(std::find(with.begin(), with.end(), d), with.end()) << d; + } +} + +TEST(FileApi, FindLibraryFileSearchesOpenscadPath) { + TempDir libs; + fs::create_directories(libs.path() / "MYLIB"); + fs::path libFile = libs.path() / "MYLIB" / "std.scad"; + writeFile(libFile, "x = 1;\n"); + + TempDir docs; + fs::path mainFile = docs.path() / "main.scad"; + writeFile(mainFile, "x = 1;\n"); + + ScopedOpenscadPath scoped(libs.path().string()); + auto found = findLibraryFile(mainFile.string(), "MYLIB/std.scad"); + ASSERT_TRUE(found.has_value()); + EXPECT_EQ(fs::path(*found), libFile); +} + +TEST(FileApi, MissingIncludeErrorNamesEveryDirectorySearched) { + TempDir dir; + fs::path mainFile = dir.path() / "main.scad"; + writeFile(mainFile, "include \n"); + + try { + getASTFromFile(mainFile.string()); + FAIL() << "expected a missing-include error"; + } catch (const std::runtime_error& e) { + std::string msg = e.what(); + EXPECT_NE(msg.find("NOPE/std.scad"), std::string::npos) << msg; + for (const auto& d : librarySearchDirs(mainFile.string())) { + EXPECT_NE(msg.find(d), std::string::npos) << msg; + } + } +} From 5edf0de0cae0e1c925b8dc1bd19201b2af4793b9 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Thu, 17 Sep 2026 09:06:18 -0700 Subject: [PATCH 2/2] NOMINMAX: windows.h's max() macro broke std::max in api.cpp Co-Authored-By: Claude Opus 5 (1M context) --- src/api.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/api.cpp b/src/api.cpp index 369adb0..d74f457 100644 --- a/src/api.cpp +++ b/src/api.cpp @@ -8,6 +8,11 @@ #include #include #if defined(_WIN32) +// NOMINMAX or windows.h's max()/min() macros eat std::max below -- MSVC +// caught it as "illegal token on right side of '::'", which names neither +// the macro nor the header. +#define NOMINMAX +#define WIN32_LEAN_AND_MEAN #include #include #else