From d3273504bdbc19466fd5ca4254d3080510c75df6 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 1 Mar 2023 20:12:19 -0600 Subject: [PATCH] fix: 5053 old torrent files keep appearing (#5117) --- libtransmission/resume.cc | 16 ++++------------ libtransmission/resume.h | 2 +- libtransmission/torrent-metainfo.cc | 20 +++++++++----------- libtransmission/torrent.cc | 10 ++-------- tests/libtransmission/rename-test.cc | 4 ++-- 5 files changed, 18 insertions(+), 34 deletions(-) diff --git a/libtransmission/resume.cc b/libtransmission/resume.cc index 9716ae876..7370e06df 100644 --- a/libtransmission/resume.cc +++ b/libtransmission/resume.cc @@ -621,22 +621,14 @@ auto loadProgress(tr_variant* dict, tr_torrent* tor) // --- -auto loadFromFile(tr_torrent* tor, tr_resume::fields_t fields_to_load, bool* did_migrate_filename) +auto loadFromFile(tr_torrent* tor, tr_resume::fields_t fields_to_load) { auto fields_loaded = tr_resume::fields_t{}; TR_ASSERT(tr_isTorrent(tor)); auto const was_dirty = tor->isDirty; - auto const migrated = tr_torrent_metainfo::migrateFile( - tor->session->resumeDir(), - tor->name(), - tor->infoHashString(), - ".resume"sv); - if (did_migrate_filename != nullptr) - { - *did_migrate_filename = migrated; - } + tr_torrent_metainfo::migrateFile(tor->session->resumeDir(), tor->name(), tor->infoHashString(), ".resume"sv); auto const filename = tor->resumeFile(); if (!tr_sys_path_exists(filename)) @@ -865,7 +857,7 @@ auto useFallbackFields(tr_torrent* tor, tr_resume::fields_t fields, tr_ctor cons } } // namespace -fields_t load(tr_torrent* tor, fields_t fields_to_load, tr_ctor const* ctor, bool* did_rename_to_hash_only_name) +fields_t load(tr_torrent* tor, fields_t fields_to_load, tr_ctor const* ctor) { TR_ASSERT(tr_isTorrent(tor)); @@ -873,7 +865,7 @@ fields_t load(tr_torrent* tor, fields_t fields_to_load, tr_ctor const* ctor, boo ret |= useMandatoryFields(tor, fields_to_load, ctor); fields_to_load &= ~ret; - ret |= loadFromFile(tor, fields_to_load, did_rename_to_hash_only_name); + ret |= loadFromFile(tor, fields_to_load); fields_to_load &= ~ret; ret |= useFallbackFields(tor, fields_to_load, ctor); diff --git a/libtransmission/resume.h b/libtransmission/resume.h index 60c2162b9..7690d5a2a 100644 --- a/libtransmission/resume.h +++ b/libtransmission/resume.h @@ -46,7 +46,7 @@ auto inline constexpr Group = fields_t{ 1 << 23 }; auto inline constexpr All = ~fields_t{ 0 }; -fields_t load(tr_torrent* tor, fields_t fields_to_load, tr_ctor const* ctor, bool* did_rename_to_hash_only_name); +fields_t load(tr_torrent* tor, fields_t fields_to_load, tr_ctor const* ctor); void save(tr_torrent* tor); diff --git a/libtransmission/torrent-metainfo.cc b/libtransmission/torrent-metainfo.cc index 9edf34382..19209b160 100644 --- a/libtransmission/torrent-metainfo.cc +++ b/libtransmission/torrent-metainfo.cc @@ -679,22 +679,20 @@ bool tr_torrent_metainfo::migrateFile( std::string_view suffix) { auto const old_filename = makeFilename(dirname, name, info_hash_string, BasenameFormat::NameAndPartialHash, suffix); - auto const old_filename_exists = tr_sys_path_exists(old_filename); - auto const new_filename = makeFilename(dirname, name, info_hash_string, BasenameFormat::Hash, suffix); - auto const new_filename_exists = tr_sys_path_exists(new_filename); + if (!tr_sys_path_exists(old_filename)) + { + return false; + } - if (old_filename_exists && new_filename_exists) + auto const new_filename = makeFilename(dirname, name, info_hash_string, BasenameFormat::Hash, suffix); + if (tr_sys_path_exists(new_filename)) { tr_sys_path_remove(old_filename); return false; } - if (new_filename_exists) - { - return false; - } - - if (old_filename_exists && tr_sys_path_rename(old_filename, new_filename)) + auto const renamed = tr_sys_path_rename(old_filename, new_filename); + if (!renamed) { tr_logAddError( fmt::format( @@ -705,7 +703,7 @@ bool tr_torrent_metainfo::migrateFile( return true; } - return false; // neither file exists + return renamed; } void tr_torrent_metainfo::removeFile( diff --git a/libtransmission/torrent.cc b/libtransmission/torrent.cc index 8e298a60e..9610362bd 100644 --- a/libtransmission/torrent.cc +++ b/libtransmission/torrent.cc @@ -1105,15 +1105,9 @@ void torrentInit(tr_torrent* tor, tr_ctor const* ctor) // the same ones that would be saved back again, so don't let them // affect the 'is dirty' flag. auto const was_dirty = tor->isDirty; - - bool resume_file_was_migrated = false; - loaded = tr_resume::load(tor, tr_resume::All, ctor, &resume_file_was_migrated); + loaded = tr_resume::load(tor, tr_resume::All, ctor); tor->isDirty = was_dirty; - - if (resume_file_was_migrated) - { - tr_torrent_metainfo::migrateFile(session->torrentDir(), tor->name(), tor->infoHashString(), ".torrent"sv); - } + tr_torrent_metainfo::migrateFile(session->torrentDir(), tor->name(), tor->infoHashString(), ".torrent"sv); } tor->completeness = tor->completion.status(); diff --git a/tests/libtransmission/rename-test.cc b/tests/libtransmission/rename-test.cc index 65c1f047c..d038506e5 100644 --- a/tests/libtransmission/rename-test.cc +++ b/tests/libtransmission/rename-test.cc @@ -192,7 +192,7 @@ TEST_F(RenameTest, singleFilenameTorrent) // (while it's renamed: confirm that the .resume file remembers the changes) tr_resume::save(tor); sync(); - auto const loaded = tr_resume::load(tor, tr_resume::All, ctor, nullptr); + auto const loaded = tr_resume::load(tor, tr_resume::All, ctor); EXPECT_STREQ("foobar", tr_torrentName(tor)); EXPECT_NE(decltype(loaded){ 0 }, (loaded & tr_resume::Name)); @@ -303,7 +303,7 @@ TEST_F(RenameTest, multifileTorrent) tr_resume::save(tor); // this is a bit dodgy code-wise, but let's make sure the .resume file got the name tor->setFileSubpath(1, "gabba gabba hey"sv); - auto const loaded = tr_resume::load(tor, tr_resume::All, ctor, nullptr); + auto const loaded = tr_resume::load(tor, tr_resume::All, ctor); EXPECT_NE(decltype(loaded){ 0 }, (loaded & tr_resume::Filenames)); EXPECT_EQ(ExpectedFiles[0], tr_torrentFile(tor, 0).name); EXPECT_STREQ("Felidae/Felinae/Felis/placeholder/Kyphi", tr_torrentFile(tor, 1).name);