diff --git a/ChangeLog b/ChangeLog index 7d7dbbd36..8008b89a8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,6 @@ 1.1.1 release + * move files one-by-one when moving storage for a torrent * fix bug in enum_net() for BSD and Mac * fix bug in python binding of announce_entry * fixed bug related to flag_merge_resume_http_seeds flag in add_torrent_params diff --git a/include/libtorrent/file.hpp b/include/libtorrent/file.hpp index 736e94752..ff48618e8 100644 --- a/include/libtorrent/file.hpp +++ b/include/libtorrent/file.hpp @@ -135,6 +135,8 @@ namespace libtorrent , std::string const& new_path, error_code& ec); TORRENT_EXTRA_EXPORT void copy_file(std::string const& f , std::string const& newf, error_code& ec); + TORRENT_EXTRA_EXPORT void move_file(std::string const& f + , std::string const& newf, error_code& ec); // file is expected to exist, link will be created to point to it. If hard // links are not supported by the filesystem or OS, the file will be copied. diff --git a/src/file.cpp b/src/file.cpp index e49b453c8..6557de4b0 100644 --- a/src/file.cpp +++ b/src/file.cpp @@ -684,6 +684,44 @@ namespace libtorrent #endif // TORRENT_WINDOWS } + void move_file(std::string const& inf, std::string const& newf, error_code& ec) + { + ec.clear(); + + file_status s; + stat_file(inf, &s, ec); + if (ec) return; + + if (has_parent_path(newf)) + { + create_directories(parent_path(newf), ec); + if (ec) return; + } + + rename(inf, newf, ec); + + // on OSX, the error when trying to rename a file across different + // volumes is EXDEV, which will make it fall back to copying. + + if (ec) + { + if (ec != boost::system::errc::no_such_file_or_directory + && ec != boost::system::errc::invalid_argument + && ec != boost::system::errc::permission_denied) + { + ec.clear(); + copy_file(inf, newf, ec); + + if (!ec) + { + // ignore errors when removing + error_code ignore; + remove(inf, ignore); + } + } + } + } + std::string split_path(std::string const& f) { if (f.empty()) return f; @@ -879,7 +917,7 @@ namespace libtorrent ++len; } return std::string(first, len); - + } return std::string(sep + 1); } @@ -1017,7 +1055,7 @@ namespace libtorrent ret.resize(write_cur - &ret[0]); return ret; } -#endif +#endif boost::int64_t file_size(std::string const& f) { diff --git a/src/storage.cpp b/src/storage.cpp index 97b00b3a1..2abea7490 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -1158,19 +1158,6 @@ namespace libtorrent } } - // collect all directories in to_move. This is because we - // try to move entire directories by default (instead of - // files independently). - std::map to_move; - for (int i = 0; i < f.num_files(); ++i) - { - // files moved out to absolute paths are not moved - if (is_complete(f.file_path(i))) continue; - - std::string split = split_path(f.file_path(i)); - to_move.insert(to_move.begin(), std::make_pair(split, i)); - } - e.clear(); stat_file(save_path, &s, e); if (e == boost::system::errc::no_such_file_or_directory) @@ -1198,85 +1185,102 @@ namespace libtorrent print_open_files("release files", m_files.name().c_str()); #endif - for (std::map::const_iterator i = to_move.begin() - , end(to_move.end()); i != end; ++i) + int i; + for (i = 0; i < f.num_files(); ++i) { - std::string old_path = combine_path(m_save_path, i->first); - std::string new_path = combine_path(save_path, i->first); + // files moved out to absolute paths are not moved + if (is_complete(f.file_path(i))) continue; + + std::string old_path = combine_path(m_save_path, f.file_path(i)); + std::string new_path = combine_path(save_path, f.file_path(i)); + + if (flags == dont_replace && exists(new_path)) + { + if (ret == piece_manager::no_error) ret = piece_manager::need_full_check; + continue; + } e.clear(); - rename(old_path, new_path, e); + move_file(old_path, new_path, e); // if the source file doesn't exist. That's not a problem // we just ignore that file if (e == boost::system::errc::no_such_file_or_directory) e.clear(); - // on OSX, the error when trying to rename a file across different - // volumes is EXDEV, which will make it fall back to copying. - if (e) { - if (flags == dont_replace && e == boost::system::errc::file_exists) - { - if (ret == piece_manager::no_error) ret = piece_manager::need_full_check; - continue; - } - - if (e == boost::system::errc::invalid_argument - || e == boost::system::errc::permission_denied) - { - ec.ec = e; - ec.file = i->second; - ec.operation = storage_error::rename; - break; - } - - if (e != boost::system::errc::no_such_file_or_directory) - { - e.clear(); - recursive_copy(old_path, new_path, ec.ec); - if (ec.ec == boost::system::errc::no_such_file_or_directory) - { - // it's a bit weird that rename() would not return - // ENOENT, but the file still wouldn't exist. But, - // in case it does, we're done. - ec.ec.clear(); - break; - } - if (ec) - { - ec.file = i->second; - ec.operation = storage_error::copy; - } - else - { - // ignore errors when removing - error_code ignore; - remove_all(old_path, ignore); - } - break; - } + ec.ec = e; + ec.file = i; + ec.operation = storage_error::rename; + break; } } - if (!ec) + if (!e && m_part_file) { - if (m_part_file) + m_part_file->move_partfile(save_path, e); + if (e) { - // TODO: if everything moves OK, except for the partfile - // we currently won't update the save path, which breaks things. - // it would probably make more sense to give up on the partfile - m_part_file->move_partfile(save_path, ec.ec); - if (ec) + ec.ec = e; + ec.file = -1; + ec.operation = storage_error::partfile_move; + } + } + + if (e) + { + // rollback + while (--i >= 0) + { + // files moved out to absolute paths are not moved + if (is_complete(f.file_path(i))) continue; + + std::string old_path = combine_path(m_save_path, f.file_path(i)); + std::string new_path = combine_path(save_path, f.file_path(i)); + + if (!exists(old_path)) { - ec.file = -1; - ec.operation = storage_error::partfile_move; - return piece_manager::fatal_disk_error; + // ignore errors when rolling back + error_code ignore; + move_file(new_path, old_path, ignore); } } - m_save_path = save_path; + return piece_manager::fatal_disk_error; } + + m_save_path.swap(save_path); + // now we have old save path in save_path + + std::set subdirs; + for (i = 0; i < f.num_files(); ++i) + { + // files moved out to absolute paths are not moved + if (is_complete(f.file_path(i))) continue; + + if (has_parent_path(f.file_path(i))) + subdirs.insert(parent_path(f.file_path(i))); + std::string old_path = combine_path(save_path, f.file_path(i)); + + // we may still have some files in old save_path + // eg. if (flags == dont_replace && exists(new_path)) + // ignore errors when removing + error_code ignore; + remove(old_path, ignore); + } + + for (std::set::iterator it(subdirs.begin()) + , end(subdirs.end()); it != end; ++it) + { + e.clear(); + std::string subdir(*it); + while (!subdir.empty() && !e) + { + remove(combine_path(save_path, subdir), e); + subdir = parent_path(subdir); + } + } + return ret; } diff --git a/test/test_storage.cpp b/test/test_storage.cpp index 3c975b0e4..519996b9a 100644 --- a/test/test_storage.cpp +++ b/test/test_storage.cpp @@ -1253,27 +1253,107 @@ TORRENT_TEST(readwritev_zero_size_files) TORRENT_TEST(move_storage_into_self) { + error_code ec; + std::string save_path = current_working_directory(); + remove_all(combine_path(save_path, "temp_storage"), ec); + if (ec && ec != boost::system::errc::no_such_file_or_directory) + std::cerr << "remove_all '" << combine_path(save_path, "temp_storage") + << "': " << ec.message() << std::endl; + TEST_CHECK(!exists(combine_path(save_path, "temp_storage"))); + aux::session_settings set; file_storage fs; std::vector buf; file_pool fp; io_service ios; disk_buffer_pool dp(16 * 1024, ios, boost::bind(&nop)); - boost::shared_ptr s = setup_torrent(fs, fp, buf - , current_working_directory() - , set); + boost::shared_ptr s = setup_torrent(fs, fp, buf, save_path, set); file::iovec_t b = {&buf[0], 4}; storage_error se; s->writev(&b, 1, 2, 0, 0, se); - s->move_storage(combine_path("temp_storage", "folder1"), 0, se); + std::string test_path = combine_path(save_path, combine_path("temp_storage", "folder1")); + s->move_storage(test_path, 0, se); + TEST_EQUAL(se.ec, boost::system::errc::success); - printf("move error: %s\n", se.ec.message().c_str()); -#ifdef _WIN32 - TEST_EQUAL(se.ec, boost::system::errc::permission_denied); -#else - TEST_EQUAL(se.ec, boost::system::errc::invalid_argument); -#endif + TEST_CHECK(exists(combine_path(test_path, combine_path("temp_storage" + , combine_path("folder1", "test2.tmp"))))); + + // these directories and files are created up-front because they are empty files + TEST_CHECK(exists(combine_path(test_path, combine_path("temp_storage" + , combine_path("folder2", "test3.tmp"))))); + TEST_CHECK(exists(combine_path(test_path, combine_path("temp_storage" + , combine_path("_folder3", "test4.tmp"))))); +} + +TORRENT_TEST(dont_move_intermingled_files) +{ + error_code ec; + + std::string save_path = combine_path(current_working_directory(), "save_path_1"); + remove_all(combine_path(save_path, "temp_storage"), ec); + if (ec && ec != boost::system::errc::no_such_file_or_directory) + std::cerr << "remove_all '" << combine_path(save_path, "temp_storage") + << "': " << ec.message() << std::endl; + TEST_CHECK(!exists(combine_path(save_path, "temp_storage"))); + + std::string test_path = combine_path(current_working_directory(), "save_path_2"); + remove_all(combine_path(test_path, "temp_storage"), ec); + if (ec && ec != boost::system::errc::no_such_file_or_directory) + std::cerr << "remove_all '" << combine_path(test_path, "temp_storage") + << "': " << ec.message() << std::endl; + TEST_CHECK(!exists(combine_path(test_path, "temp_storage"))); + + aux::session_settings set; + file_storage fs; + std::vector buf; + file_pool fp; + io_service ios; + disk_buffer_pool dp(16 * 1024, ios, boost::bind(&nop)); + boost::shared_ptr s = setup_torrent(fs, fp, buf, save_path, set); + + file::iovec_t b = {&buf[0], 4}; + storage_error se; + s->writev(&b, 1, 2, 0, 0, se); + + create_directory(combine_path(save_path, combine_path("temp_storage" + , combine_path("_folder3", "alien_folder1"))), ec); + TEST_EQUAL(ec, boost::system::errc::success); + file f; + f.open(combine_path(save_path, combine_path("temp_storage", "alien1.tmp")) + , file::write_only, ec); + f.close(); + TEST_EQUAL(ec, boost::system::errc::success); + f.open(combine_path(save_path, combine_path("temp_storage" + , combine_path("folder1", "alien2.tmp"))), file::write_only, ec); + f.close(); + TEST_EQUAL(ec, boost::system::errc::success); + + s->move_storage(test_path, 0, se); + TEST_EQUAL(se.ec, boost::system::errc::success); + + // torrent files moved to new place + TEST_CHECK(exists(combine_path(test_path, combine_path("temp_storage" + , combine_path("folder1", "test2.tmp"))))); + // these directories and files are created up-front because they are empty files + TEST_CHECK(exists(combine_path(test_path, combine_path("temp_storage" + , combine_path("folder2", "test3.tmp"))))); + TEST_CHECK(exists(combine_path(test_path, combine_path("temp_storage" + , combine_path("_folder3", "test4.tmp"))))); + + // intermingled files and directories are still in old place + TEST_CHECK(exists(combine_path(save_path, combine_path("temp_storage" + , "alien1.tmp")))); + TEST_CHECK(!exists(combine_path(test_path, combine_path("temp_storage" + , "alien1.tmp")))); + TEST_CHECK(exists(combine_path(save_path, combine_path("temp_storage" + , combine_path("folder1", "alien2.tmp"))))); + TEST_CHECK(!exists(combine_path(test_path, combine_path("temp_storage" + , combine_path("folder1", "alien2.tmp"))))); + TEST_CHECK(exists(combine_path(save_path, combine_path("temp_storage" + , combine_path("_folder3", "alien_folder1"))))); + TEST_CHECK(!exists(combine_path(test_path, combine_path("temp_storage" + , combine_path("_folder3", "alien_folder1"))))); }