facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2025-01-08 14:46:47 +03:00
Code Issues Projects Releases Wiki Activity

setParentCommits after checkout while holding parentsLock_

Browse Source 
Summary: was looking in this area and saw this TODO. moves the `setParentCommits()` call into `CheckoutConflict::finish()` so it is called while we are still holding the parents lock.

Reviewed By: chadaustin

Differential Revision: D18668329

fbshipit-source-id: 8415c792193e5b89737f15141f33c5a9799f527b
This commit is contained in:
Genevieve Helsel 2019-12-10 18:11:21 -08:00 committed by Facebook Github Bot
parent 20c87ae2db
commit 8ff8d6217e
3 changed files with 24 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
eden/fs/inodes/CheckoutContext.cpp
View File

@ -9,6 +9,7 @@
#include <folly/logging/xlog.h>
#include "eden/fs/config/CheckoutConfig.h"
#include "eden/fs/inodes/EdenMount.h"
#include "eden/fs/inodes/InodePtr.h"
#include "eden/fs/inodes/TreeInode.h"
@ -36,8 +37,15 @@ void CheckoutContext::start(RenameLock&& renameLock) {
Future<vector<CheckoutConflict>> CheckoutContext::finish(Hash newSnapshot) {
// Only update the parents if it is not a dry run.
if (!isDryRun()) {
auto oldParents = parentsLock_->parents;
// Update the in-memory snapshot ID
parentsLock_->parents.setParents(newSnapshot);
auto config = mount_->getConfig();
// Save the new snapshot hash to the config
config->setParentCommits(newSnapshot);
XLOG(DBG1) << "updated snapshot for " << config->getMountPath() << " from "
<< oldParents << " to " << newSnapshot;
}
// Release the rename lock.

7
eden/fs/inodes/EdenMount.cpp
View File

@ -852,13 +852,6 @@ folly::Future<CheckoutResult> EdenMount::checkout(
return result;
}
// Save the new snapshot hash to the config
// TODO: This should probably be done by CheckoutConflict::finish()
// while still holding the parents lock.
this->config_->setParentCommits(snapshotHash);
XLOG(DBG1) << "updated snapshot for " << this->getPath() << " from "
<< oldParents << " to " << snapshotHash;
// Write a journal entry
//
// Note that we do not call journalDiffCallback->performDiff() a

16
eden/fs/inodes/test/CheckoutTest.cpp
View File

@ -13,6 +13,7 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include "eden/fs/config/CheckoutConfig.h"
#include "eden/fs/inodes/EdenMount.h"
#include "eden/fs/inodes/FileInode.h"
#include "eden/fs/inodes/InodeMap.h"
@ -563,16 +564,31 @@ void testModifyConflict(
EXPECT_EQ(path, result.conflicts[0].path);
EXPECT_EQ(ConflictType::MODIFIED_MODIFIED, result.conflicts[0].type);
const auto currentParent =
testMount.getEdenMount()->getParentCommits().parent1();
const auto configParent =
testMount.getEdenMount()->getConfig()->getParentCommits().parent1();
// Make sure both the mount parent and the config parent information was
// updated
EXPECT_EQ(currentParent, configParent);
auto postInode = testMount.getFileInode(path);
switch (checkoutMode) {
case CheckoutMode::FORCE:
// Make sure the path is updated as expected
EXPECT_FILE_INODE(postInode, contents2, perms2);
// Make sure the parent information has been updated
EXPECT_EQ(currentParent, makeTestHash("b"));
break;
case CheckoutMode::DRY_RUN:
// make sure the currentParent is still commit1
EXPECT_EQ(currentParent, makeTestHash("a"));
break;
case CheckoutMode::NORMAL:
// Make sure the path has not been changed
EXPECT_FILE_INODE(postInode, currentContents, currentPerms);
// Make sure the parent information has been updated
EXPECT_EQ(currentParent, makeTestHash("b"));
break;
}