From f9d0a0044c2f80a0dd90862a1ad16b8aea1c8115 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Fri, 16 Jun 2017 20:45:10 +0100 Subject: [PATCH] Fix masterlist update handling of diverged histories Fixes loot/loot#790. --- .travis.yml | 3 +++ appveyor.yml | 3 +++ src/api/helpers/git_helper.cpp | 2 +- src/api/masterlist.cpp | 4 ++-- src/tests/api/internals/masterlist_test.h | 28 ++++++++++++++++------- 5 files changed, 29 insertions(+), 11 deletions(-) diff --git a/.travis.yml b/.travis.yml index edfa7075..0520e433 100644 --- a/.travis.yml +++ b/.travis.yml @@ -45,6 +45,9 @@ before_script: script: - make all + # Some tests call Git, so set a user + - git config --global user.email "travis@ci" + - git config --global user.name "Travis" - ./loot_api_internals_tests - ./loot_api_tests diff --git a/appveyor.yml b/appveyor.yml index c426951b..9fb1bbd4 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -47,6 +47,9 @@ build: test_script: - cd %APPVEYOR_BUILD_FOLDER%\build\%CONFIGURATION% + # Some tests call Git, so set a user + - git config --global user.email "appveyor@ci" + - git config --global user.name "AppVeyor" - loot_api_internals_tests.exe --gtest_output=xml:loot_api_internals_tests.xml - loot_api_tests.exe --gtest_output=xml:loot_api_tests.xml diff --git a/src/api/helpers/git_helper.cpp b/src/api/helpers/git_helper.cpp index 71c1d27a..b12bb9c4 100644 --- a/src/api/helpers/git_helper.cpp +++ b/src/api/helpers/git_helper.cpp @@ -216,7 +216,7 @@ void GitHelper::CheckoutNewBranch(const std::string& remote, const std::string& // Create a branch. BOOST_LOG_TRIVIAL(trace) << "Creating the new branch."; Call(git_commit_lookup(&data_.commit, data_.repo, commit_id)); - Call(git_branch_create(&data_.reference, data_.repo, branch.c_str(), data_.commit, 0)); + Call(git_branch_create(&data_.reference, data_.repo, branch.c_str(), data_.commit, 1)); // Set upstream. BOOST_LOG_TRIVIAL(trace) << "Setting the upstream for the new branch."; diff --git a/src/api/masterlist.cpp b/src/api/masterlist.cpp index 90969434..d8f88283 100644 --- a/src/api/masterlist.cpp +++ b/src/api/masterlist.cpp @@ -181,8 +181,8 @@ bool Masterlist::Update(const boost::filesystem::path& path, const std::string& // The local branch can't be easily merged. Best just to delete and recreate it. BOOST_LOG_TRIVIAL(trace) << "Local branch cannot be easily merged with remote branch."; - BOOST_LOG_TRIVIAL(trace) << "Deleting the local branch."; - git.Call(git_branch_delete(git.GetData().reference)); + BOOST_LOG_TRIVIAL(trace) << "Detaching HEAD so that the branch can be recreated."; + git.Call(git_repository_detach_head(git.GetData().repo)); // Need to free ref before calling git.CheckoutNewBranch() git_reference_free(git.GetData().reference); diff --git a/src/tests/api/internals/masterlist_test.h b/src/tests/api/internals/masterlist_test.h index 373489f0..63aa8059 100644 --- a/src/tests/api/internals/masterlist_test.h +++ b/src/tests/api/internals/masterlist_test.h @@ -71,19 +71,19 @@ INSTANTIATE_TEST_CASE_P(, GameType::fo4, GameType::tes5se)); -TEST_P(MasterlistTest, updateWithSeparateParametersShouldThrowIfAnInvalidPathIsGiven) { +TEST_P(MasterlistTest, updateShouldThrowIfAnInvalidPathIsGiven) { Masterlist masterlist; EXPECT_THROW(masterlist.Update(";//\?", repoUrl, repoBranch), boost::filesystem::filesystem_error); } -TEST_P(MasterlistTest, updateWithSeparateParametersShouldThrowIfABlankPathIsGiven) { +TEST_P(MasterlistTest, updateShouldThrowIfABlankPathIsGiven) { Masterlist masterlist; EXPECT_THROW(masterlist.Update("", repoUrl, repoBranch), boost::filesystem::filesystem_error); } -TEST_P(MasterlistTest, updateWithSeparateParametersShouldThrowIfABranchThatDoesNotExistIsGiven) { +TEST_P(MasterlistTest, updateShouldThrowIfABranchThatDoesNotExistIsGiven) { Masterlist masterlist; EXPECT_THROW(masterlist.Update(masterlistPath, @@ -91,13 +91,13 @@ TEST_P(MasterlistTest, updateWithSeparateParametersShouldThrowIfABranchThatDoesN "missing-branch"), std::system_error); } -TEST_P(MasterlistTest, updateWithSeparateParametersShouldThrowIfABlankBranchIsGiven) { +TEST_P(MasterlistTest, updateShouldThrowIfABlankBranchIsGiven) { Masterlist masterlist; EXPECT_THROW(masterlist.Update(masterlistPath, repoUrl, ""), std::invalid_argument); } -TEST_P(MasterlistTest, updateWithSeparateParametersShouldThrowIfAUrlThatDoesNotExistIsGiven) { +TEST_P(MasterlistTest, updateShouldThrowIfAUrlThatDoesNotExistIsGiven) { Masterlist masterlist; EXPECT_THROW(masterlist.Update(masterlistPath, @@ -105,19 +105,19 @@ TEST_P(MasterlistTest, updateWithSeparateParametersShouldThrowIfAUrlThatDoesNotE repoBranch), std::system_error); } -TEST_P(MasterlistTest, updateWithSeparateParametersShouldThrowIfABlankUrlIsGiven) { +TEST_P(MasterlistTest, updateShouldThrowIfABlankUrlIsGiven) { Masterlist masterlist; EXPECT_THROW(masterlist.Update(masterlistPath, "", repoBranch), std::invalid_argument); } -TEST_P(MasterlistTest, updateWithSeparateParametersShouldReturnTrueIfNoMasterlistExists) { +TEST_P(MasterlistTest, updateShouldReturnTrueIfNoMasterlistExists) { Masterlist masterlist; EXPECT_TRUE(masterlist.Update(masterlistPath, repoUrl, repoBranch)); } -TEST_P(MasterlistTest, updateWithSeparateParametersShouldReturnFalseIfAnUpToDateMasterlistExists) { +TEST_P(MasterlistTest, updateShouldReturnFalseIfAnUpToDateMasterlistExists) { Masterlist masterlist; EXPECT_TRUE(masterlist.Update(masterlistPath, @@ -129,6 +129,18 @@ TEST_P(MasterlistTest, updateWithSeparateParametersShouldReturnFalseIfAnUpToDate repoBranch)); } +TEST_P(MasterlistTest, updateShouldDiscardLocalHistoryIfRemoteHistoryIsDifferent) { + Masterlist masterlist; + ASSERT_TRUE(masterlist.Update(masterlistPath, repoUrl, repoBranch)); + + auto testPath = boost::filesystem::current_path(); + boost::filesystem::current_path(masterlistPath.parent_path()); + system("git commit --amend -m \"changing local history\""); + boost::filesystem::current_path(testPath); + + EXPECT_TRUE(masterlist.Update(masterlistPath, repoUrl, repoBranch)); +} + TEST_P(MasterlistTest, getInfoShouldThrowIfNoMasterlistExistsAtTheGivenPath) { Masterlist masterlist; EXPECT_THROW(masterlist.GetInfo(masterlistPath, false), FileAccessError);