From 8626ce5c4352634d18d62ab328448abfa99bdc02 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Mon, 27 Jan 2025 23:31:59 -0800 Subject: [PATCH] Rely on std::filesystem for file_utils (#3042) Co-authored-by: Andrew Johnson Co-authored-by: Paul Romano --- include/openmc/file_utils.h | 8 +++- src/cross_sections.cpp | 25 +++++----- src/dagmc.cpp | 4 +- src/file_utils.cpp | 61 ++++++++---------------- src/simulation.cpp | 3 +- src/state_point.cpp | 10 ++-- tests/cpp_unit_tests/test_file_utils.cpp | 9 ++++ 7 files changed, 53 insertions(+), 67 deletions(-) diff --git a/include/openmc/file_utils.h b/include/openmc/file_utils.h index 6e9812d5e83..db65640344c 100644 --- a/include/openmc/file_utils.h +++ b/include/openmc/file_utils.h @@ -5,7 +5,10 @@ namespace openmc { -// TODO: replace with std::filesystem when switch to C++17 is made +// NOTE: This is a thin wrapper over std::filesystem because we +// pass strings around a lot. Objects like settings::path_input +// are extern std::string to play with other libraries and languages + //! Determine if a path is a directory //! \param[in] path Path to check //! \return Whether the path is a directory @@ -18,7 +21,8 @@ bool file_exists(const std::string& filename); //! Determine directory containing given file //! \param[in] filename Path to file -//! \return Name of directory containing file +//! \return Name of directory containing file excluding the final directory +//! separator std::string dir_name(const std::string& filename); // Gets the file extension of whatever string is passed in. This is defined as diff --git a/src/cross_sections.cpp b/src/cross_sections.cpp index 248ae9019d9..b1bfde03d13 100644 --- a/src/cross_sections.cpp +++ b/src/cross_sections.cpp @@ -23,6 +23,7 @@ #include "pugixml.hpp" #include // for getenv +#include #include namespace openmc { @@ -282,15 +283,16 @@ void read_ce_cross_sections(const vector>& nuc_temps, void read_ce_cross_sections_xml() { // Check if cross_sections.xml exists - const auto& filename = settings::path_cross_sections; - if (dir_exists(filename)) { + std::filesystem::path filename(settings::path_cross_sections); + if (!std::filesystem::exists(filename)) { + fatal_error( + "Cross sections XML file '" + filename.string() + "' does not exist."); + } + + if (std::filesystem::is_directory(filename)) { fatal_error("OPENMC_CROSS_SECTIONS is set to a directory. " "It should be set to an XML file."); } - if (!file_exists(filename)) { - // Could not find cross_sections.xml file - fatal_error("Cross sections XML file '" + filename + "' does not exist."); - } write_message("Reading cross sections XML file...", 5); @@ -309,15 +311,10 @@ void read_ce_cross_sections_xml() } else { // If no directory is listed in cross_sections.xml, by default select the // directory in which the cross_sections.xml file resides - - // TODO: Use std::filesystem functionality when C++17 is adopted - auto pos = filename.rfind("/"); - if (pos == std::string::npos) { - // No '\\' found, so the file must be in the same directory as - // materials.xml - directory = settings::path_input; + if (filename.has_parent_path()) { + directory = filename.parent_path().string(); } else { - directory = filename.substr(0, pos); + directory = settings::path_input; } } diff --git a/src/dagmc.cpp b/src/dagmc.cpp index 94c220850f3..52c45f21c28 100644 --- a/src/dagmc.cpp +++ b/src/dagmc.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -56,7 +57,8 @@ DAGUniverse::DAGUniverse(pugi::xml_node node) if (check_for_node(node, "filename")) { filename_ = get_node_value(node, "filename"); if (!starts_with(filename_, "/")) { - filename_ = dir_name(settings::path_input) + filename_; + std::filesystem::path d(dir_name(settings::path_input)); + filename_ = (d / filename_).string(); } } else { fatal_error("Must specify a file for the DAGMC universe"); diff --git a/src/file_utils.cpp b/src/file_utils.cpp index 197c9767cb0..517f82d157e 100644 --- a/src/file_utils.cpp +++ b/src/file_utils.cpp @@ -1,65 +1,42 @@ #include "openmc/file_utils.h" -#include // any_of -#include // for isalpha -#include // for ifstream -#include +#include namespace openmc { bool dir_exists(const std::string& path) { - struct stat s; - if (stat(path.c_str(), &s) != 0) - return false; - - return s.st_mode & S_IFDIR; + std::filesystem::path d(path); + return std::filesystem::is_directory(d); } bool file_exists(const std::string& filename) { - // rule out file being a path to a directory - if (dir_exists(filename)) + std::filesystem::path p(filename); + if (!std::filesystem::exists(p)) { return false; - - std::ifstream s {filename}; - return s.good(); + } + if (std::filesystem::is_directory(p)) { + return false; + } + return true; } std::string dir_name(const std::string& filename) { - size_t pos = filename.find_last_of("\\/"); - return (std::string::npos == pos) ? "" : filename.substr(0, pos + 1); + std::filesystem::path p(filename); + return (p.parent_path()).string(); } std::string get_file_extension(const std::string& filename) { - // try our best to work on windows... -#if defined(_WIN32) || defined(_WIN64) - const char sep_char = '\\'; -#else - const char sep_char = '/'; -#endif - - // check that at least one letter is present - const auto last_period_pos = filename.find_last_of('.'); - const auto last_sep_pos = filename.find_last_of(sep_char); - - // no file extension. In the first case, we are only given - // a file name. In the second, we have been given a file path. - // If that's the case, periods are allowed in directory names, - // but have the interpretation as preceding a file extension - // after the last separator. - if (last_period_pos == std::string::npos || - (last_sep_pos < std::string::npos && last_period_pos < last_sep_pos)) - return ""; - - const std::string ending = filename.substr(last_period_pos + 1); - - // check that at least one character is present. - const bool has_alpha = std::any_of(ending.begin(), ending.end(), - [](char x) { return static_cast(std::isalpha(x)); }); - return has_alpha ? ending : ""; + std::filesystem::path p(filename); + auto ext = p.extension(); + if (!ext.empty()) { + // path::extension includes the period + return ext.string().substr(1); + } + return ""; } } // namespace openmc diff --git a/src/simulation.cpp b/src/simulation.cpp index 09b3764732b..030e447f135 100644 --- a/src/simulation.cpp +++ b/src/simulation.cpp @@ -446,10 +446,9 @@ void finalize_batch() // Write a continously-overwritten source point if requested. if (settings::source_latest) { - // note: correct file extension appended automatically auto filename = settings::path_output + "source"; gsl::span bankspan(simulation::source_bank); - write_source_point(filename.c_str(), bankspan, simulation::work_index, + write_source_point(filename, bankspan, simulation::work_index, settings::source_mcpl_write); } } diff --git a/src/state_point.cpp b/src/state_point.cpp index adc026fa3c2..fcc389df189 100644 --- a/src/state_point.cpp +++ b/src/state_point.cpp @@ -52,9 +52,7 @@ extern "C" int openmc_statepoint_write(const char* filename, bool* write_source) // If a file name was specified, ensure it has .h5 file extension const auto extension = get_file_extension(filename_); - if (extension == "") { - filename_.append(".h5"); - } else if (extension != "h5") { + if (extension != "h5") { warning("openmc_statepoint_write was passed a file extension differing " "from .h5, but an hdf5 file will be written."); } @@ -578,8 +576,10 @@ void write_source_point(std::string filename, gsl::span source_bank, // Dispatch to appropriate function based on file type if (use_mcpl) { + filename.append(".mcpl"); write_mcpl_source_point(filename.c_str(), source_bank, bank_index); } else { + filename.append(".h5"); write_h5_source_point(filename.c_str(), source_bank, bank_index); } } @@ -602,9 +602,7 @@ void write_h5_source_point(const char* filename, std::string filename_(filename); const auto extension = get_file_extension(filename_); - if (extension == "") { - filename_.append(".h5"); - } else if (extension != "h5") { + if (extension != "h5") { warning("write_source_point was passed a file extension differing " "from .h5, but an hdf5 file will be written."); } diff --git a/tests/cpp_unit_tests/test_file_utils.cpp b/tests/cpp_unit_tests/test_file_utils.cpp index 3b7a74346bd..8b0d99d76da 100644 --- a/tests/cpp_unit_tests/test_file_utils.cpp +++ b/tests/cpp_unit_tests/test_file_utils.cpp @@ -30,3 +30,12 @@ TEST_CASE("Test file_exists") // Note: not clear how to portably test where a file should exist. REQUIRE(!file_exists("./should_not_exist/really_do_not_make_this_please")); } + +TEST_CASE("Test dir_name") +{ + REQUIRE(dir_name("") == ""); + REQUIRE(dir_name("/") == "/"); + REQUIRE(dir_name("hello") == ""); + REQUIRE(dir_name("hello/world") == "hello"); + REQUIRE(dir_name("/path/to/dir/") == "/path/to/dir"); +}