Summary:
Move the code for the command-line tool up one directory, out of eden/fs.
This better separates the code so that eden/fs contains code for the edenfs
daemon, while eden/cli contains code for the command line tool.
Reviewed By: bolinfest
Differential Revision: D4888633
fbshipit-source-id: 5041e292c5353d05122eefe5db3257289e31239a
Summary:
Fix a subtle crash during checkout when handling newly added entries that
already exist in the working directory: CheckoutAction passed the entry name to
checkoutUpdateEntry() as a PathComponentPiece. However, this
PathComponentPiece could refer to the entry name owned by newScmEntry_, and it
also passed newScmEntry_ into checkoutUpdateEntry() as an rvalue reference.
As a result, if the string data was stored invalidated by the move the name
would no longer be valid when checkoutUpdateEntry() tried to use it.
This bug is triggered by doing an "hg update --clean", where a file added in
the destination commit already exists on disk, and has an entry name of 23
characters or less. (The 23 character limit is fbstring's upper bound on
small string optimizations, where it will store the string data inline in the
object, causing it to be invalidated on move.)
This also fixes a crash in a VLOG() statement when the verbose log level for
TreeInode.cpp was set to 4 or greater.
Reviewed By: bolinfest
Differential Revision: D4882544
fbshipit-source-id: 917ede6eeae2224aaa0724b8b30324f3c3a5c924
Summary:
This is a bit too magical -- it's not clear that the thing produces an Options object. If you do know that you can chain further option setters off this thing, it's nice, but otherwise, the first impression is "what just happened?".
So, let's have one good way for doing things.
Reviewed By: yfeldblum
Differential Revision: D4863947
fbshipit-source-id: 3dfe83cfc077d47f604f47dcb21149fbaa2d2243
Summary:
Logical ids are so 2014. We have been thinking
about emitting stats using normalized names for some time.
Approach wise, I picked the following
- If port does not have a name - default to using port ID
- In agent if a port name get set or updated. Re init BCMPort stats. Basically
this means that the moment config gets applied, we would start emitting
normalized interface stats.
Two additional pieces of work here are
i) Update interface based alerts to also look at these new normalized
interface name stats - t17052196
ii) Update cubism to look at these new names - t17052255
I won't land till at least i) is fixed
Differential Revision: D4815561
fbshipit-source-id: c088b5a8981ea40429fedc8570652c376a374be3
Summary:
Update the hg extension to implement dirstate.rebuild(). This is necessary for
the `hg reset` command. This also now implements dirstate.setparents() for
cases when there is only one parent.
Reviewed By: wez
Differential Revision: D4823780
fbshipit-source-id: 802de006e03860995095dc3af17acb2eb05f4e8b
Summary:
Update the checkOutRevision() thrift API to accept the commit ID as a
BinaryHash rather than as a hexadecimal ASCII string, to make it more
consistent with our other thrift APIs.
Reviewed By: wez
Differential Revision: D4823779
fbshipit-source-id: 6226f926533ef86d60f8b1d659bc5d912e2855e6
Summary:
The intent is to provide a way to locate the SNAPSHOT file
for tools that want to have a very fast way to figure out the commit
id without making any RPCs or subprocess invocations.
Reviewed By: simpkins
Differential Revision: D4824176
fbshipit-source-id: 5adca225d9984146852dad1e83de0d903848c1e5
Summary:
This simplifies the InodePtrImpl class by removing support for pointer-to-const
Inode objects. We don't currently use pointer-to-const Inode objects, and I
don't really anticipate that we will ever need this.
The primary motivating factor behind this diff is a recent change in clang that
breaks the existing code: clang PR31606 prevents children classes from
inheriting template constructor instantiations from their parent which take a
reference to the parent class type as the only argument.
While this could have been fixed by explicitly defining the necessary
constructors in the child class (or simply by overriding newPtrLocked() and
newPtrFromExisting() to return the subclass type), dropping support for
pointer-to-const allows us to simplify the code and resolve this issue.
Reviewed By: wez
Differential Revision: D4825526
fbshipit-source-id: 999de352788b13818b7f23788bb34686e193cd5d
Summary:
previously, the importer would read the entire manifest
and emit data to the store as it resolved complete directory entries.
The entire manifest data would be buffered and sent out to the store.
In the scenario where one subtree has been modified and a commit has
been made, only the parents of the subdirectory need to be hashed
and stored, but we would compute and try to store everything anyway.
While this diff can't avoid having to compute hashes for everything (we need
tree manifest data for that), by breaking the import into two passes we can
potentially avoid interrogating the LocalStore about every tree in the entire
manifest during an import; we only need to store the trees that are missing and
can simply cut out sub trees that are already present.
This saves us IO at the expense of buffering the manifest tree in memory
for the duration of the first pass; it is an acceptable trade off.
Reviewed By: simpkins
Differential Revision: D4832166
fbshipit-source-id: 0a40cb851c65393b407a8161db05c4b1795fb11a
Summary:
This avoids hard coding knowledge of the FB-specific `local` dir.
This is only suitable for use on already-mounted eden instances,
so the function accepts either the mounted path or the eden
configuration dir.
Reviewed By: bolinfest
Differential Revision: D4829166
fbshipit-source-id: df9daed6024cdec6222be90f5cb624dd9ca0da0b
Summary:
Folly is dropping support for GCC 4.8 now that the branch for last HHVM release that needs to support it has been cut.
This codemod's away all the trivially identifiable uses of `folly::make_unique` in fbcode outside of folly, mcrouter and hphp.
A future diff will kill it in those, and a diff after that will kill it entirely.
Reviewed By: djwatson
Differential Revision: D4498157
fbshipit-source-id: 5a2c05ffd29e5358298acb53dab189a699b022cc
Summary:
This diff fixes FileData::stat() so that we report reasonable timestamp values
on non-materialized files, rather than always leaving them as 0. We set the
timestamps to the time that we created the FileInode. This ensures that
timestamps are updated correctly when files are modified by a checkout
operation.
Note that for materialized files the code reports the timestamp of the overlay
file. This diff does not modify that behavior. However, this behavior is
incorrect, as the overlay file timestamps are not updated by a FUSE client
opening, modifying, then closing a file (since we keep the underyling overlay
file handle open, and don't close it).
In the future we'll need to implement our own tracking of atime, mtime, and
ctime values. We should probably store these in a header inside the overlay
file. For now, this diff is a stop-gap measure that ensures we at least update
non-materialized file timestamps correctly on checkouts.
Reviewed By: bolinfest
Differential Revision: D4765632
fbshipit-source-id: 478da6441e213cdfe830f1c5129212182ce4eeb0
Summary:
This diff adds a new "eden debug" command, with a few subcommands:
- "eden debug inode": Report information about currently loaded inodes
- "eden debug tree": Report data about source control Tree objects
- "eden debug blobmeta": Report metadata about source control Blob objects
- "eden debug blob": Report the contents of source control Blob objects
This diff also includes the thrift APIs in edenfs to support these commands.
Reviewed By: bolinfest
Differential Revision: D4760528
fbshipit-source-id: 7dc2bd6e0e952ba6fb61702c672fb9417d603ffa
Summary:
Previously EdenServer::getMount() returned a null pointer if the path did not
refer to a known mount point. However, most of the thrift APIs using this
method never checked if the return value was null, which could allow any thrift
client to crash edenfs.
This updates getMount() to throw if the path is not found, and never return
null.
Reviewed By: bolinfest
Differential Revision: D4760527
fbshipit-source-id: 6cd9a1e3bc0a0a220d7f5fa518d497d37f245eff
Summary:
This updates all of the references to gtest and gmock with googletest.
The change is mechanilcal, generated with the following one-liner:
```lang=bash
hg grep -lwE '(gtest|gmock)' 'glob:**/TARGETS' | grep -v '^third-party-buck' | xargs perl -pi -e '
$gt=qr!(["'"'"'])gtest\g1!;
(
s!$gt(\s*,\s*(.any.|None))(\s*,\s*)?\),?!\1googletest\1\2, \1gtest\1\),!g or
s!$gt((\s*,\s*(.any.|None)[^\)]+))\),?!\1googletest\1\2\),!g or
s!\(\s*$gt,?\s*\),?!\(\1googletest\1, None, \1gtest\1\),!g or
s!$gt,?!\(\1googletest\1, None, \1gtest\1\),!g
) unless /(name|type) *=/;
$gm=qr!(["'"'"'])gmock\g1!;
(
s!$gm(\s*,\s*(.any.|None))(\s*,\s*)?\),?!\1googletest\1\2, \1gmock\1\),!g or
s!$gm((\s*,\s*(.any.|None)[^\)]+))\),?!\1googletest\1\2\),!g or
s!\(\s*$gm,?\s*\),?!\(\1googletest\1, None, \1gmock\1\),!g or
s!$gm,?!\(\1googletest\1, None, \1gmock\1\),!g
) unless /(name|type) *=/;
'
```
Reviewed By: meyering
Differential Revision: D4643237
fbshipit-source-id: fda7f41760c7e44254231df87634631c343e6355
Summary:
Update the integration test code to allow TestCase classes to control the
--vmodule settings used when starting the edenfs daemon.
D3851805 initially set the vlog level for RequestData to 5 to help debug
issues. However, this log level doesn't really make sense for most of the
mercurial integration tests: most mercurial commands checks for the existence
of lots of files, causing the logs to be filled with useless ENOENT messages
when the RequestData log level is this high.
Reviewed By: wez
Differential Revision: D4814402
fbshipit-source-id: 1127c0a25b656ea1a710ca54a59a9407d66a5659
Summary:
Add an assert_status() method to the hg integration tests that runs "hg
status", parses the output, then compares it to expected results.
Reviewed By: wez
Differential Revision: D4814422
fbshipit-source-id: 24ebdc2e0239c4833953c31e5786cc320bcd9d62
Summary:
Update HgRepository.hg() to catch subprocess.CalledProcessError exceptions and
translate them into subclass that returns a more helpful message.
The CalledProcessError includes the command stderr as a member variable, but
unfortunately does not include this in the output from `__str__()`. The stderr
output is usually important for helping debug test failures.
Reviewed By: wez
Differential Revision: D4814401
fbshipit-source-id: 4f010bb3ce33833c55d18768997d2d1bb6a001b8
Summary:
The hg_import_helper script that eden uses to import data from mercurial keeps
a long-lived repository object open. This caches some data about the
repository, and if new commits are added after it was created, it can fail to
see them.
This updates hg_import_helper.py to catch errors that occur when trying to use
the repository objects. The code will invalidate the repository object and
then retry the operation once, in the hopes that it will now succeed after
invalidation.
Reviewed By: bolinfest
Differential Revision: D4752659
fbshipit-source-id: 1c75c84766d6bbda0710882a338eaa09e0cb0030
Summary:
The kernel can return ENOENT in response to invalidation notification if we
have never told the kernel about the inode in question. This resulted in
spurious errors during checkout when updating files that were loaded internally
by edenfs rather than via FUSE call. For instance, this was commonly triggered
by .gitignore files, which eden loads on its own to perform ignore processing.
Reviewed By: bolinfest
Differential Revision: D4752630
fbshipit-source-id: d4e092643a8d33cf33709f7e3664289f167ac093
Summary:
I found it rather awkward in HgExtensionTestBase that self.repo is not actually
the repository being tested. It was instead the repostiory used as the backing
store for the mercurial data, and self.repo_for_mount was the repository being
tested.
This diff renames the two repository classes, so that self.backing_repo is now
the backing store repository, and self.repo is the repository being tested.
In order to do this I changed HgExtensionTestBase to derive directly from
EdenTestCase. Previously it derived from EdenHgTest, and was letting
EdenHgTest set up self.repo. It seemed more understandable to avoid deriving
EdenHgTest now since self.repo is not the repository that needs to be set up
initially.
Reviewed By: bolinfest
Differential Revision: D4752631
fbshipit-source-id: d8b542b0ecead66b965af1a582085345e28b2908
Summary:
Previously the eden hg extension short-circuited the checkout operation if the
destination commit was the same as the one currently checked out. This was
incorrect if --clean was specified, since we do need to reset the working
directory state in this case.
This updates the extension code to always make the thrift checkout() call when
doing a force checkout.
This also avoids calling applyupdates() to resolve conflicts when force=True.
When doing a force checkout, eden reports files with conflicts that it
overwrote, but these do not need to be resolved by mercurial.
Additionally, this also updates a couple other APIs that have recently been
changed in upstream mercurial: merge.update() now takes an updatecheck
argument, and repo.join() should now be written as repo.vfs.join().
Reviewed By: bolinfest
Differential Revision: D4752510
fbshipit-source-id: e1ee92d086315e35a1378f674e668876a667c0ce
Summary:
Tweaks the stats stuff so that we can name the histograms and export them with p50, p90, p99 percentiles.
This is made a bit ugly because the fb303 stats code isn't yet open source, so
there's a moderately ugly preprocessor directive that we assume to be true for
our internal build. We'll need to force that to be 0 for the open source build
until we open the stats code and make it available.
Reviewed By: bolinfest
Differential Revision: D4801868
fbshipit-source-id: 643909e63bd4a74b2cfa580be131f65c5673bc94
Summary:
This explicitly uses a WriteBatch during importing. This results in a
5x perf gain for import operations.
I've also added in a read-before-write check to make sure that we're not
attempting to overwrite a key that already exists. Since this is a
content-addressed store, a given key should always have the same
value; it should be immutable. Overwriting an existing key is an
opportunity to change data that should never change and is also wasteful
of IO bandwidth.
Ideally we'd be able to exploit this to avoid descending into a directory;
I'd like to hook that up to the treemanifest stuff in mercurial in a
follow on diff.
Reviewed By: bolinfest
Differential Revision: D4783598
fbshipit-source-id: 24f95495d09a852859b346559fdf83d064de2b51
Summary:
this is the bare minimum to support creating unix domain sockets.
We only support using mknod to create a unix socket; other uses will yield an error.
I've added an rdev field as a sibling of the existing mode field that we track,
as that is the additional parameter that we need to track as part of the
special file node.
Special file nodes are tracked in the overlay as empty files.
Reviewed By: bolinfest
Differential Revision: D4774099
fbshipit-source-id: 0824b7e509063faa8bede7aff82a7c51930c4f83
Summary:
This is an initial pass at implementing "hg status" using the new EdenMount and
TreeInode diff() code.
More work still needs to be done to clean things up here, but this makes
"hg status" more correct in the face of modified files and directories that are
not modified, and vice-versa.
In particular, the following still needs to be cleaned up in future diffs:
- Update the "hg status" code to more correctly process user directives for
paths that did not appear in the diff output. This will probably be
simplified some by correctly updating the user directive list on checkout and
resetCommit() operations. It may also make things simpler if we store the
user directives in a hierarchical path map, rather than in a flat list.
- Update the addAll() code to also use the new diff logic, rather than the
getStatusForExistingDirectory() function.
- Clean up the locking some. I think it may be worth changing the code to use
a single lock to manage both the current commit ID for the mount and the
dirstate user directives. We probably should hold this lock for the duration
of a diff operation. The diff operation should also return the commit ID
that it is based against.
Reviewed By: wez
Differential Revision: D4752281
fbshipit-source-id: 6a6d7e2815321b09c75e95aa7460b0da41931dc0
Summary:
This is a utility that is intended to be consumed by our
fbagent monitoring infrastructure.
The typical pattern at FB is that fbagent is configured via monitoring config
to pull data from the hosts that belong to a thrift tier and then push that
into ODS.
Since our model is a bit different we don't fit that pattern; eden runs as a
process per user on a host, and is accessible only via unix domain socket.
That means that we need to use a CommandCollector to sample data from our eden
servers, and that's where this tool comes in.
The intent is to use configerator to set up the command collector to run the tool
using a command line that looks something like this:
```
eden-fb303-collector /data/users/*/.eden/socket --tierName %%TIER_NAME%% --counterRegexes '[".*mount.*.rate.60"]'
```
Note that we rely on the shell to expand the glob to find plausible eden sockets.
The `counterRegexes` parameter is a JSON encoded array listing the pcre regexes
that should be used to select the counters that we want to place into ODS.
I chose to pass these as JSON encoded parameters because it will allow the list
of counters to be configured easily in configerator (python) code alongside the
CommandCollector definition and pass that down to the collector. That makes it
a no-brainer to `json.dumps()` and pass the list down.
This collector emits three entities for each key:
* user@hostname - allows us to distinguish different users on the same host.
* hostname - aggregated by fbagent for all instances on the same host.
* TIER - aggregated by fbagent for all instances on the same tier.
I could add more levels of aggregation to this; it is common to track by region
and cluster, but I'm not sure how many of these we really want; any strong preferences?
The intent is that we'll deploy this alongside the edenfs binary that we
package with packman and then have a separate configerator diff to add the
CommandCollector to enable collection for the dev tiers.
D4797021 and D4797101 are the related diffs to hook this up in the right places
Reviewed By: bolinfest
Differential Revision: D4789717
fbshipit-source-id: 4a1d58c50847a8ca99043542cd2f0415da783ad7
Summary:
Leaving these as nullptr causes the kernel to handle advisory locking for us.
There's some plumbing in libfuse that handles negotiating this for us.
Reviewed By: simpkins
Differential Revision: D4774540
fbshipit-source-id: 5495963b980d95f06ededa72f4d483cc74fd8ec3
Summary:
It's not really magic because we don't have a virtual directory
inode base any more. Instead, we mkdir and populate it at mount time.
What is slightly magical about it is that we give it some special powers:
* We know the inode number of the eden dir and prevent unlink operations
on it or inside it.
* The .eden dir is present in the contents of the root inode and will
show up when that directory is `readdir`'d
* When resolving a child of a TreeInode by name, we know to return the
magic `.eden` inode number. This means that it is possible to `stat`
and consume the `.eden` directory from any directory inside the eden
mount, even though it won't show up in `readdir` for those child dirs.
The contents of the `.eden` dir are:
* `socket` - a symlink back to the unix domain socket that our thrift
server is listening on. This means that it is a simple
`readlink(".eden/socket")` operation to discover both whether a directory
is part of an eden mount and how to talk to the server.
* `root` - a symlink back to the root of this eden mount. This allows
using `readlink(".eden/root")` as a simple 1-step operation to find
the root of an eden mount, and avoids needing to walk up directory
by directory as is the common pattern for locating `.hg` or `.git`
dirs.
Reviewed By: simpkins
Differential Revision: D4637285
fbshipit-source-id: 0eabf98b29144acccef5c83bd367493399dc55bb
Summary:
Update the EdenMount::diff() code to completely skip .hg and .eden directories.
This was implemented through the GitIgnore code, and adding a new HIDDEN status
The .hg and .eden directories are similar to ignored directories, but we never
want to report any information for them even when listIgnored is true.
This also changes the GitIgnore code to so that GitIgnoreStack computes the
basename for each path once, and passes it down into the underlying match code,
so we don't have to re-compute it for each pattern that needs the basename.
Reviewed By: wez
Differential Revision: D4751917
fbshipit-source-id: ec22d62f31a3ce9ef998a8ccb4413f72f1d7a487
Summary:
This updates the diff() logic to process gitignore rules.
The bulk of the existing TreeInode::diff() was moved into a
TreeInode::computeDiff() function. TreeInode::diff() now loads the gitignore
rules for this directory before calling computeDiff(). The argument list for
computeDiff() was now getting slightly unwieldy, so I created a new DiffContext
class to hold some of the arguments that are the same for the duration of the
entire diff operation. This resulted in some rather mechanical changes through
the computeDiff() and DeferredDiffEntry code to update the argument lists.
Reviewed By: wez
Differential Revision: D4744690
fbshipit-source-id: c4981d9f49878b522e0b3faf35de837739066f3c
Summary:
Add suffixes() and rsuffixes() methods to the RelativePath and AbsolutePath
classes.
I am planning to use this functionality when performing gitignore matching.
(Each .gitignore file matches against relative path suffixes from the directory
containing the .gitignore file.)
Reviewed By: wez
Differential Revision: D4744612
fbshipit-source-id: 1fd3bdb4a66193fbd73aa514442c51b5024a30e3
Summary:
This tweaks the definition of the subscribe method in the
streamingeden thrift file and implements the server side of the
thrift service, and the client side to consume it in watchman.
The definition is now a bit simpler than it was previously; we're
now just sending a stream of the updated JournalPosition objects
to the client rather than computing and sending FileDelta objects.
This is cheaper for Eden to deal with because it is very cheap to take
the current journal position and pass that over the wire. This is
important because a burst of mutations will likely queue up a bunch
of these in quick succession.
In a future diff I'm considering adding a latency parameter for
the subscription so that we can constrain the number of updates
over a certain time period (likely in the 10's of milliseconds range).
For now I just want to prove that the concept works.
On the watchman side we just need to pull these off the wire as they are sent
by eden, then wait for the subscription stream to settle before internally
broadcasting to any connected subscribers.
Reviewed By: bolinfest
Differential Revision: D4647259
fbshipit-source-id: 03aa16e59a43195a2505a8d03bce1ccf88a8d42f
Summary:
Update the edenfs main function to check the exit code of the privhelper
process, and to exit with EX_SOFTWARE rather than EX_OK if the privhelper did
not exit with status 0.
This will ensure that our integration tests always catch crashes in the
privhelper process. Previously we could catch errors that happened before the
privhelper socket was closed, but if the privhelper server crashed during
cleanup we never noticed the error.
Reviewed By: bolinfest
Differential Revision: D4728006
fbshipit-source-id: 2c478612e5fabec90438b28b9c76a3343f561655
Summary:
Add a unit test that modifies a file and then does a forced checkout to revert
back to the original commit state.
This also includes some pretty-printers to make the tests print
CheckoutConflict objects nicely if any test assertions fail.
Reviewed By: bolinfest
Differential Revision: D4727779
fbshipit-source-id: b31dc90be28d17ed0f2a4076c96a08ba82588190
Summary:
This begins adding an EdenMount::diff() method, which walks through the inode
tree and reports differences from the current source control tree state.
My intent is to eventually update the Dirstate code to use this diff()
function, and to remove the existing getModifiedDirectories() logic. The
getModifiedDirectories() code is currently incorrect, as it reports
materialized directories, which is not the same as directories that are
modified from the current source control state.
The ignore processing logic is not currently implemented in EdenMount::diff(),
but it should be relatively straightforward to add in a subsequent diff.
Reviewed By: bolinfest
Differential Revision: D4726048
fbshipit-source-id: ad0bb3b5d72bb3830f60fc2b2e56a81217c35353
Summary:
The privhelper code was erasing elements from a std::unordered_map before it
was done using the iterator pointing to that element. This causes memory
corruption issues.
Between this and some of my other recent unmount fixes (D4548030, D4547938)
this makes the bind-mount-related integration tests work.
Reviewed By: bolinfest
Differential Revision: D4727850
fbshipit-source-id: 6d1fda3f89cb91c89d0020921b1805fc10e65785
Summary:
Update the TreeInode and CheckoutAction to correctly be able to remove
directories during checkout operations. This handles both removing directories
that are newly empty as a result of the checkout, and directories that are
replaced with a file or symlink.
This also refactors the rmdir/unlink code a bit so we can share some of its
logic for doing directory removal during the checkout operation.
Reviewed By: bolinfest
Differential Revision: D4708607
fbshipit-source-id: 3b9dd9cc3536845dad0d9278e3c5d3ac1ed04570
Summary:
Synchronize access to the CheckoutContext conflicts_ list when recording
conflicts or errors during a checkout operation.
In theory it is possible for the checkout operation to be proceeding
simultaneously in multiple threads. At the moment this does not happen because
the BackingStore implementations currently block, and always return
immediately-ready Future objects in the same thread. However, once they are
updated to return Future objects that actually complete asynchronously in other
threads the checkout work will continue in these other threads.
Reviewed By: bolinfest
Differential Revision: D4708605
fbshipit-source-id: 885b0114d97dadcacf64652f795a7d3846412f11
Summary:
We are using `BinaryAddress` heavily within fboss codebase to represent a
nexthop as well as an IPAddres in wire format. Nexthops will need to have
a scoping identifier (iface name) for link-local address inorder to resolve
it properly.
We do this in OpenR and well tested on Arista. This seems bit hacky but it
saves a lot of time. In future we would like to have separate network util for
fboss and openr and we can get rid of this hack at that time.
Differential Revision: D4710768
fbshipit-source-id: 72092935f3738b3fb88a422462d1c995cc0acf18
Summary:
This adds a common/ directory with stub files required for building
eden. These stubs are the same ones as used in the fboss repository:
https://github.com/facebook/fboss/
Our goal is to eventually create an opensource fb303 repository with
complete implementations of these files, but for now these basic stubs
allow building eden. (Most of the underlying stats code that supports
fb303 is already available in folly/stats/ in the folly repository.)
Test Plan:
Tested building eden on Ubuntu 16.04
Summary:
Add a test that exercises a checkout operation that creates a brand new
subdirectory.
Reviewed By: bolinfest
Differential Revision: D4693994
fbshipit-source-id: 70f43f67c2cdb10bd854f51d5ba2e61c45976975
Summary:
Unfortunately it looks like gcc won't support the [[nodiscard]] attribute until
the gcc-7.x release. (The functionality is available in gcc-4.8 and later, but
using the name [[gnu::warn_unused_result]] instead.)
This changes the code to use the FOLLY_WARN_UNUSED_RESULT macro from
folly/Portability.h instead. (This expands to older non-standard __attribute__
syntax for gcc and clang, and _Check_return_ on Windows.)
Reviewed By: bolinfest
Differential Revision: D4704184
fbshipit-source-id: d5c13630ea611a2f43080a501add42ce9fda6789
Summary:
Add new ASSIGN_PARENT_INODE and ASSIGN_INODE types to LoadBehavior, to exercise
additional code paths during checkout operations. This also cleans up the
overall handling of the LoadBehavior enum.
Reviewed By: bolinfest
Differential Revision: D4694548
fbshipit-source-id: e8359cfdadb1fabb1a0d07bc57a1a1610ed9ba57
Summary:
Refactor FileInode and FileData so that they no longer store the
TreeInode::Entry that refers to this file. The TreeInode::Entry is owned by
the parent TreeInode and is not safe to access without holding the TreeInode
contents_ lock, which the FileInode code never did. Additionally, once a
FileInode is unlocked the TreeInode::Entry is destroyed. Previously the
FileInode code would retain its pointer to this now invalid data, and could
still dereference it.
In the future I think it might be worth refactoring the FileInode/FileData
split a bit more, but this at least addresses the issues around invalid
accesses to entry_.
Reviewed By: bolinfest
Differential Revision: D4688058
fbshipit-source-id: 17d5d0c4e756afc6e3c4bb279cb1bb5231f3134f
Summary:
Remove the TreeInode::entry_ member variable. This was a pointer to the
TreeInode::Entry object representing this inode in the parent TreeInode.
However, it isn't safe for a child Inode to access the TreeInode::Entry owned
by its parent. This Entry object is supposed to be protected by the parent
TreeInode's contents_ lock, and the child inode never held this when accessing
the Entry.
This updates the TreeInode code to never use entry_ any more. I will work on
removing FileInode::entry_ in a subsequent diff.
This did require changing the overlay materialization code some. Previously
the checkout code directly modified entry_ to inform the parent TreeInode if
the child was materialized or not. Now we have to more explicitly tell the
parent TreeInode about changes to each child's materialization state. With
this change we now keep the overlay data on disk more consistent during the
checkout process. We should now always have overlay data for children inodes
on disk whenever the parent thinks they are materialized. We may end up
writing out TreeInode overlay data frequently during the checkout process,
however. We might be able to improve this in the future. For now it seemed
simpler and safer to just update the data frequently to make sure it stays
consistent.
Reviewed By: bolinfest
Differential Revision: D4688055
fbshipit-source-id: d08a14b9304ac49826f0897a0b53281177d9d9f4
Summary:
Update the code so that we no longer immediately remove overlay data about a
file or directory when the inode is unlinked. The file can continue being used
after it was unlinked if there are open file handles or other internal
references to it.
This updates the code to instead wait to remove the overlay data until the
InodeBase object is unloaded.
This does not fully fix all the outstanding issues with using inodes after they
have been unlinked, because the FileInode and TreeInode still use the entry_
pointer that now points to a bogus TreeInode::Entry.
Reviewed By: bolinfest
Differential Revision: D4688056
fbshipit-source-id: a9e1c1a9562590b8b81237057bfc4ef77795807f
Summary:
This changes a couple aspects of the materialization process:
- Write out child files in the overlay before their parent. If we throw an
exception or crash partway through this process, it is better to have the
child not marked materialized in its parent, rather than to have the parent
indicate that the child is materialized but to not actually have any overlay
data for the child. In the former case we will still load the correct child
data the next time we need to, but in the latter case we would fail to load
the child correctly.
- Hold the rename lock while materializing our parent directories, to ensure
that we materialize the correct parent, and it cannot change while we are
trying to perform the materialization.
- Make the parent responsible for modifying the child's TreeInode::Entry. This
data is owned by the parent and is protected by the parent's contents_ lock.
The child really shouldn't ever be directly accessing this data without
holding the parent's contents_ lock.
Reviewed By: bolinfest
Differential Revision: D4688057
fbshipit-source-id: 2662f79cb7d7febb086f4e0888a3d96a580c4bfa
Summary:
This fixes the checkout code to always recurse into directories with conflicts,
and accurately list all individual files with conflicts.
Previously the checkout code always avoided recursing into unloaded directory
inodes that were affected by the checkout. Since the inode was not loaded we
can easily replace it without recursing into it. Unfortunately this resulted
in an inaccurate conflict list--we would report the directory itself as a
conflict. This is insufficient for our mercurial extension, which needs to
know the specific list of files with conflicts, so it can ask mercurial to
perform conflict resolution on them.
Reviewed By: bolinfest
Differential Revision: D4672373
fbshipit-source-id: ff400a4a6e25b8b0754c66cb067a69d553a5c98b
