Repository: NREL/OpenStudio (language: python)

Issue:
PathHelpers copyDirectory should use overwrite_existing
Issue overview
--------------
Using the copyDirectory, a user expects files to be overwritten if they exist.  But this is not the default behavior of std::filesystem::copy_file.

## Current Behavior
Copying results from Apply Measure Nnow does not update previous results

## Expected Behavior
Copying results from Apply Measure Now should update previous results

## Steps to Reproduce
Create a measure that writes some text to a file under `generated_files`.  Run that in Apply Measure Now in the OS App, save the model.  The text file is found under files in companion dir.  Re-run the measure with updated text.  The text file found under files in companion dir is not updated.

## Possible Solution
This line https://github.com/NREL/OpenStudio/blob/master/src/utilities/core/PathHelpers.cpp#L378 should pass in std::filesystem::copy_options::overwrite_existing

PathHelpers copyDirectory should use overwrite_existing
Issue overview
--------------
Using the copyDirectory, a user expects files to be overwritten if they exist.  But this is not the default behavior of std::filesystem::copy_file.

## Current Behavior
Copying results from Apply Measure Nnow does not update previous results

## Expected Behavior
Copying results from Apply Measure Now should update previous results

## Steps to Reproduce
Create a measure that writes some text to a file under `generated_files`.  Run that in Apply Measure Now in the OS App, save the model.  The text file is found under files in companion dir.  Re-run the measure with updated text.  The text file found under files in companion dir is not updated.

## Possible Solution
This line https://github.com/NREL/OpenStudio/blob/master/src/utilities/core/PathHelpers.cpp#L378 should pass in std::filesystem::copy_options::overwrite_existing

PathHelpers copyDirectory should use overwrite_existing
Issue overview
--------------
Using the copyDirectory, a user expects files to be overwritten if they exist.  But this is not the default behavior of std::filesystem::copy_file.

## Current Behavior
Copying results from Apply Measure Nnow does not update previous results

## Expected Behavior
Copying results from Apply Measure Now should update previous results

## Steps to Reproduce
Create a measure that writes some text to a file under `generated_files`.  Run that in Apply Measure Now in the OS App, save the model.  The text file is found under files in companion dir.  Re-run the measure with updated text.  The text file found under files in companion dir is not updated.

## Possible Solution
This line https://github.com/NREL/OpenStudio/blob/master/src/utilities/core/PathHelpers.cpp#L378 should pass in std::filesystem::copy_options::overwrite_existing



Discussion / hints:
Thanks @tijcolem !
Thanks @tijcolem !
Thanks @tijcolem !

Fix patch:
diff --git a/src/cli/UpdateCommand.cpp b/src/cli/UpdateCommand.cpp
index f96a96f3c49..116abf94cf1 100644
--- a/src/cli/UpdateCommand.cpp
+++ b/src/cli/UpdateCommand.cpp
@@ -66,7 +66,7 @@ namespace cli {
       if (keep) {
         openstudio::path backupPath = osmPath;
         backupPath.replace_extension(openstudio::toPath(".osm.orig"));
-        openstudio::filesystem::copy_file(osmPath, backupPath, openstudio::filesystem::copy_option::overwrite_if_exists);
+        openstudio::filesystem::copy_file(osmPath, backupPath, openstudio::filesystem::copy_options::overwrite_existing);
       }
       if (auto model_ = vt.loadModel(osmPath)) {
         fmt::print("Updated '{}'\n", osmPath.string());
diff --git a/src/energyplus/Test/ResultsTranslator_GTest.cpp b/src/energyplus/Test/ResultsTranslator_GTest.cpp
index 606fd0ef509..01477084c9d 100644
--- a/src/energyplus/Test/ResultsTranslator_GTest.cpp
+++ b/src/energyplus/Test/ResultsTranslator_GTest.cpp
@@ -64,7 +64,7 @@ TEST_F(EnergyPlusFixture,ResultsTranslator_5ZoneAirCooled_NoErrorsNoWarnings)
   openstudio::path epDir = resourcesPath() / openstudio::toPath("energyplus/5ZoneAirCooled/");
   openstudio::path origSqlPath = epDir / openstudio::toPath("eplusout.sql");
   openstudio::path testSqlPath = epDir / openstudio::toPath("eplusout_NoErrorsNoWarnings.sql");
-  openstudio::filesystem::copy_file(origSqlPath,testSqlPath,openstudio::filesystem::copy_option::overwrite_if_exists);
+  openstudio::filesystem::copy_file(origSqlPath,testSqlPath,openstudio::filesystem::copy_options::overwrite_existing);
   openstudio::path errorPath = resourcesPath() / openstudio::toPath("energyplus/ErrorFiles/NoErrorsNoWarnings.err");
 
   SqlFile sqlFile(testSqlPath);
@@ -95,7 +95,7 @@ TEST_F(EnergyPlusFixture,ResultsTranslator_5ZoneAirCooled_WarningsAndSevere)
   openstudio::path epDir = resourcesPath() / openstudio::toPath("energyplus/5ZoneAirCooled/");
   openstudio::path origSqlPath = epDir / openstudio::toPath("eplusout.sql");
   openstudio::path testSqlPath = epDir / openstudio::toPath("eplusout_WarningsAndSevere.sql");
-  openstudio::filesystem::copy_file(origSqlPath, testSqlPath, openstudio::filesystem::copy_option::overwrite_if_exists);
+  openstudio::filesystem::copy_file(origSqlPath, testSqlPath, openstudio::filesystem::copy_options::overwrite_existing);
   openstudio::path errorPath = resourcesPath() / openstudio::toPath("energyplus/ErrorFiles/WarningsAndSevere.err");
 
   SqlFile sqlFile(testSqlPath);
@@ -126,7 +126,7 @@ TEST_F(EnergyPlusFixture,ResultsTranslator_5ZoneAirCooled_WarningsAndCrash)
   openstudio::path epDir = resourcesPath() / openstudio::toPath("energyplus/5ZoneAirCooled/");
   openstudio::path origSqlPath = epDir / openstudio::toPath("eplusout.sql");
   openstudio::path testSqlPath = epDir / openstudio::toPath("eplusout_WarningsAndCrash.sql");
-  openstudio::filesystem::copy_file(origSqlPath,testSqlPath,openstudio::filesystem::copy_option::overwrite_if_exists);
+  openstudio::filesystem::copy_file(origSqlPath,testSqlPath,openstudio::filesystem::copy_options::overwrite_existing);
   openstudio::path errorPath = resourcesPath() / openstudio::toPath("energyplus/ErrorFiles/WarningsAndCrash.err");
 
   SqlFile sqlFile(testSqlPath);
diff --git a/src/model/FileOperations.cpp b/src/model/FileOperations.cpp
index 2facfd922a2..f0aed9f95a4 100644
--- a/src/model/FileOperations.cpp
+++ b/src/model/FileOperations.cpp
@@ -137,16 +137,17 @@ namespace model {
       }
     }
 
-    for (const auto& dirEnt : openstudio::filesystem::recursive_directory_iterator{src}) {
-      const auto& path = dirEnt.path();
-      auto relativePathStr = path.string();
-      boost::replace_first(relativePathStr, src.string(), "");
+    // Not using PathHelper's copyDirectory because we want clear log messages
+    for (const auto& dirEnt : openstudio::filesystem::directory_iterator{src}) {
+      const auto& srcFolderPath = dirEnt.path();
+      const auto& relativeFolderPath = openstudio::filesystem::relative(srcFolderPath, src);
 
       try {
-        openstudio::filesystem::copy(path, dest / relativePathStr);
+        // No need to pass the copy_options::overwrite_existing here, we wiped the directory before
+        openstudio::filesystem::copy(srcFolderPath, dest / relativeFolderPath, openstudio::filesystem::copy_options::recursive);
       } catch (const std::exception& e) {
         LOG_FREE(Error, "replaceDir",
-                 "Error copying from: " << toString(path) << " to: " << toString(dest / relativePathStr) << " Description: " << e.what());
+                 "Error copying from: " << toString(srcFolderPath) << " to: " << toString(dest / relativeFolderPath) << " Description: " << e.what());
         result = false;
       }
     }
@@ -339,7 +340,7 @@ namespace model {
     openstudio::path srcPath = modelTempDir / osmPath.filename();
 
     try {
-      openstudio::filesystem::copy_file(srcPath, osmPath, openstudio::filesystem::copy_option::overwrite_if_exists);
+      openstudio::filesystem::copy_file(srcPath, osmPath, openstudio::filesystem::copy_options::overwrite_existing);
       test = true;
     } catch (const std::exception& e) {
       LOG_FREE(Error, "saveModelTempDir", "Could not copy osm from '" << toString(srcPath) << "' to '" << toString(osmPath) << "' error" << e.what());
diff --git a/src/utilities/core/Filesystem.hpp b/src/utilities/core/Filesystem.hpp
index 33cb67020ae..931bdde3faf 100644
--- a/src/utilities/core/Filesystem.hpp
+++ b/src/utilities/core/Filesystem.hpp
@@ -52,7 +52,7 @@ namespace filesystem {
 
   using boost::filesystem::recursive_directory_iterator;
   using boost::filesystem::filesystem_error;
-  using boost::filesystem::copy_option;
+  using boost::filesystem::copy_options;
   using boost::filesystem::directory_entry;
 
   // functions
diff --git a/src/utilities/core/FilesystemHelpers.cpp b/src/utilities/core/FilesystemHelpers.cpp
index 1302e1baa48..21dd775e9dc 100644
--- a/src/utilities/core/FilesystemHelpers.cpp
+++ b/src/utilities/core/FilesystemHelpers.cpp
@@ -172,7 +172,7 @@ namespace filesystem {
       // concat number to path basename, without adding a new path element
       auto filename = basename;
       filename += openstudio::toPath("-" + std::to_string(std::time(nullptr)) + "-" + std::to_string(count++));
-      const auto full_pathname = temp_dir / filename;
+      auto full_pathname = temp_dir / filename;
       // full_path_name = {temp_path}/{base_name}-{count++}
 
       try {
@@ -193,7 +193,7 @@ namespace filesystem {
     const auto build_path = [](const auto&... elem) {
       auto path_elem = [](auto& missing, const auto& env_var) {
         if (const auto var = std::getenv(env_var); var != nullptr) {
-          const auto path = toPath(var);
+          auto path = toPath(var);
           if (path.empty()) {
             missing = true;
           }
@@ -207,7 +207,7 @@ namespace filesystem {
       bool missing_element = false;
 
       // build up the path from the environment variable names passed in.
-      const auto path = (path_elem(missing_element, elem) / ...).lexically_normal();
+      auto path = (path_elem(missing_element, elem) / ...).lexically_normal();
 
       if (!missing_element && openstudio::filesystem::is_directory(path)) {
         return path;
@@ -217,17 +217,17 @@ namespace filesystem {
     };
 
     // TODO: instead of trace, just comment out?
-    if (const auto home = build_path("USERPROFILE"); !home.empty()) {
+    if (auto home = build_path("USERPROFILE"); !home.empty()) {
       LOG_FREE(Trace, "FilesystemHelpers", "home_path USERPROFILE: " << toString(home));
       return home;
     }
 
-    if (const auto home = build_path("HOMEDRIVE", "HOMEPATH"); !home.empty()) {
+    if (auto home = build_path("HOMEDRIVE", "HOMEPATH"); !home.empty()) {
       LOG_FREE(Trace, "FilesystemHelpers", "home_path HOMEDRIVE/HOMEPATH: " << toString(home));
       return home;
     }
 
-    if (const auto home = build_path("HOME"); !home.empty()) {
+    if (auto home = build_path("HOME"); !home.empty()) {
       LOG_FREE(Trace, "FilesystemHelpers", "home_path HOME: " << toString(home));
       return home;
     }
diff --git a/src/utilities/core/Path.i b/src/utilities/core/Path.i
index 2a72aecc0ed..cf5133ea07c 100644
--- a/src/utilities/core/Path.i
+++ b/src/utilities/core/Path.i
@@ -52,7 +52,7 @@
 
     // The contents and attributes of the file from_fp resolves to are copied to the file to_fp resolves to.
     void copy_file(const path& from_path, const path& to_path){
-      boost::filesystem::copy_file(from_path, to_path, boost::filesystem::copy_option::overwrite_if_exists);
+      boost::filesystem::copy_file(from_path, to_path, boost::filesystem::copy_options::overwrite_existing);
     }
 
     // complete the path and return an absolute path, behavior is portable
diff --git a/src/utilities/core/PathHelpers.cpp b/src/utilities/core/PathHelpers.cpp
index 5fe786497cd..07bb4832417 100644
--- a/src/utilities/core/PathHelpers.cpp
+++ b/src/utilities/core/PathHelpers.cpp
@@ -173,6 +173,8 @@ bool makeParentFolder(const path& p, const path& base, bool recursive) {
 }
 
 path relativePath(const path& p, const path& base) {
+  // Note: could just use openstudio::filesystem::relative(p, base);
+
   path wp = p;
   path wBase = base;
 
@@ -372,10 +374,14 @@ bool copyDirectory(const path& source, const path& destination) {
   // note : we are not using openstudio::filesystem::copy to copy recursively
   // because that copies the entire directory into the destination, not just the
   // contents of the directory
-  for (const auto& file : openstudio::filesystem::recursive_directory_files(source)) {
+  for (const auto& dirEnt : openstudio::filesystem::directory_iterator{source}) {
+
+    const auto& srcFolderPath = dirEnt.path();
+    const auto& relativeFolderPath = openstudio::filesystem::relative(srcFolderPath, source);
+
     try {
-      openstudio::filesystem::create_directories((destination / file).parent_path());
-      openstudio::filesystem::copy_file(source / file, destination / file, openstudio::filesystem::copy_option::overwrite_if_exists);
+      openstudio::filesystem::copy(srcFolderPath, destination / relativeFolderPath,
+                                   openstudio::filesystem::copy_options::recursive | openstudio::filesystem::copy_options::overwrite_existing);
     } catch (const std::exception&) {
       return false;
     }
diff --git a/src/utilities/sql/Test/IlluminanceMap_GTest.cpp b/src/utilities/sql/Test/IlluminanceMap_GTest.cpp
index b418503deb6..1bc84a212cd 100644
--- a/src/utilities/sql/Test/IlluminanceMap_GTest.cpp
+++ b/src/utilities/sql/Test/IlluminanceMap_GTest.cpp
@@ -56,7 +56,7 @@ class IlluminanceMapFixture : public ::testing::Test
     std::string currentTestName = ::testing::UnitTest::GetInstance()->current_test_info()->name();
     currentSqlPath = resourcesPath() / toPath("utilities/SqlFile") / toPath(currentTestName + ".sql");
     // Copy the original SqlFile to a unique one
-    openstudio::filesystem::copy_file(oriSqlPath, currentSqlPath, openstudio::filesystem::copy_option::overwrite_if_exists);
+    openstudio::filesystem::copy_file(oriSqlPath, currentSqlPath, openstudio::filesystem::copy_options::overwrite_existing);
 
     sqlFile = SqlFile(currentSqlPath);
     ASSERT_TRUE(sqlFile.connectionOpen());


Test patch:
diff --git a/src/epjson/test/epJSONFixture.cpp b/src/epjson/test/epJSONFixture.cpp
index d3899a3b17b..9273e07d537 100644
--- a/src/epjson/test/epJSONFixture.cpp
+++ b/src/epjson/test/epJSONFixture.cpp
@@ -151,7 +151,7 @@ openstudio::path setupIdftoEPJSONTest(const openstudio::path& location) {
   const auto working_directory = openstudio::filesystem::complete(openstudio::toPath("epjson_tests") / basename);
   auto idf_path = working_directory / openstudio::toPath("eplus.idf");
   openstudio::filesystem::create_directories(working_directory);
-  openstudio::filesystem::copy_file(location, idf_path, openstudio::filesystem::copy_option::overwrite_if_exists);
+  openstudio::filesystem::copy_file(location, idf_path, openstudio::filesystem::copy_options::overwrite_existing);
   return idf_path;
 }