Commit Graph

21 Commits

Author SHA1 Message Date
Adam Simpkins
6cec539d3f fix dirstate.normallookup() behavior
Summary:
Fix `dirstate.normallookup()` to behave like `dirstate.normal()`, since our
`dirstate.status()` function does not handle "unsure" files that need to be
checked on disk.

This was previously causing problems after running `hg commit -i`.

We ideally should probably have some additional changes here to avoid making an
filesystem `lstat()` call even in the `dirstate.normal()`, but that can wait
until later.

Reviewed By: bolinfest

Differential Revision: D6746814

fbshipit-source-id: c1623415fc3a923b0b7878be651db3ad8fbb2aff
2018-01-19 15:16:09 -08:00
Michael Bolin
5e2afa735f Change how the UNTRACKED_ADDED conflict and merges are handled.
Summary:
Previously, we used the Mercurial code `g` when faced with an `UNTRACKED_ADDED`
file conflict, but that was allowing merges to silently succeed that should not
have. This revision changes our logic to use the code `m` for merge, which
unearthed that we were not honoring the user's `update.check` setting properly.

Because we use `update.check=noconflict` internally at Facebook, we changed the
Eden integration tests to default to verifying Hg running with this setting. To
support it properly, we had to port this code from `update.py` in Mercurial to
our own `_determine_actions_for_conflicts()` function:

```
if updatecheck == 'noconflict':
    for f, (m, args, msg) in actionbyfile.iteritems():
        if m not in ('g', 'k', 'e', 'r', 'pr'):
            msg = _("conflicting changes")
            hint = _("commit or update --clean to discard changes")
            raise error.Abort(msg, hint=hint)
```

However, this introduced an interesting issue where the `checkOutRevision()`
Thrift call from Hg would update the `SNAPSHOT` file on the server, but
`.hg/dirstate` would not get updated with the new parents until the update
completed on the client. With the new call to `raise error.Abort` on the client,
we could get in a state where the `SNAPSHOT` file had the hash of the commit
assuming the update succeeded, but `.hg/dirstate` reflected the reality where it
failed.

To that end, we changed `checkOutRevision()` to take a new parameter,
`checkoutMode`, which can take on one of three values: `NORMAL`, `DRY_RUN`, and
`FORCE`. Now if the user tries to do an ordinary `hg update` with
`update.check=noconflict`, we first do a `DRY_RUN` and examine the potential
conflicts. Only if the conflicts should not block the update do we proceed with
a call to `checkOutRevision()` in `NORMAL` mode.

To make this work, we had to make a number of changes to `CheckoutAction`,
`CheckoutContext`, `EdenMount`, and `TreeInode` to keep track of the
`checkoutMode` and ensure that no changes are made to the working copy when a
`DRY_RUN` is in effect.

One minor issue (for which there is a `TODO`) is that a `DRY_RUN` will not
report any `DIRECTORY_NOT_EMPTY` conflicts that may exist. As `TreeInode` is
implemented today, it is a bit messy to report this type of conflict without
modifying the working copy along the way.

Finally, any `UNTRACKED_ADDED` conflict should cause an update to
abort to match the behavior in stock Mercurial if the user has the following
config setting:

```
[commands]
update.check = noconflict
```

Though the original name for this setting was:

```
[experimental]
updatecheck = noconflict
```

Although I am on Mercurial 4.4.1, the `update.check` setting does not seem to
take effect when I run the integration tests, but the `updatecheck` setting
does, so for now, I set both in `hg_extension_test_base.py` with a `TODO` to
remove `updatecheck` once I can get `update.check` to do its job.

Reviewed By: simpkins

Differential Revision: D6366007

fbshipit-source-id: bb3ecb1270e77d59d7d9e7baa36ada61971bbc49
2017-11-29 21:50:34 -08:00
Adam Simpkins
a3aa8d11e7 add python type information to more integration test code
Summary: Annotate more integration test functions with type information.

Reviewed By: bolinfest

Differential Revision: D6434358

fbshipit-source-id: b88351eebee58561465752378c6771b7b1f9554e
2017-11-29 14:36:39 -08:00
Michael Bolin
c729a17a11 New condition for test_update_with_untracked_file_that_is_tracked_in_destination.
Summary:
We now run two versions of this test: one where the file that exists in the
destination commit is untracked before the update and one in which it is added
before the update.

Reviewed By: simpkins

Differential Revision: D6334002

fbshipit-source-id: ef6bffa27bc18171b5e21dc284c7a21aa6e35da4
2017-11-20 15:56:35 -08:00
Michael Bolin
ade30224d9 Add assert_dirstate_empty to test_update_clean_removes_added_and_removed_statuses.
Summary: Addresses outstanding TODO now that D6322052 has landed.

Reviewed By: simpkins

Differential Revision: D6368884

fbshipit-source-id: 497c42466e05af0f1690bc6401b1d271de691e58
2017-11-20 14:07:41 -08:00
Michael Bolin
a9d8e55931 Verify that hg update --clean clears the dirstate.
Summary:
Users often run `hg update --clean .` to get out of a bad state, but this was
not clearing the "added" or "removed" state in the dirstate as it should in
Eden.

Reviewed By: wez

Differential Revision: D6331858

fbshipit-source-id: 616f187930587a1af40a1f151e3a424d50dd8da3
2017-11-14 22:26:08 -08:00
Michael Bolin
be4ff47192 New actions that resulted from applyupdates() in merge.py were not getting applied.
Summary:
In the course of verifying a fix for `hg update --merge` in D6270272, I
discovered a new bug in our merge logic in the Python code. As expained in the
test plan, there was a case where a file was listed as "untracked" instead of
"added" after a merge with `--tool :local`.

I traced through what happens in stock Mercurial. After the call to
`applyupdates()` in `update()` in `merge.py`, there is this code:

```
stats = applyupdates(repo, actions, wc, p2, overwrite, labels=labels)
wc.flushall()

if not partial:
    with repo.dirstate.parentchange():
        repo.setparents(fp1, fp2)
        recordupdates(repo, actions, branchmerge)
        # update completed, clear state
        util.unlink(repo.vfs.join('updatestate'))

        if not branchmerge:
            repo.dirstate.setbranch(p2.branch())
```

It turns out that `applyupdates()` can have the side-effect of adding new
entries to the `actions` dict. In this case, we have a `'cd'` action for which
an `'am'` action is generated. Our `merge_update()` function in
`eden/hg/eden/__init__.py` did not have the `recordupdates()` call that the
stock implementation of Mercurial does, so the `'am'` (for "add/merge") was not
getting applied.

It seems likely that introducing this `recordupdates()` call may fix other
subtle bugs in Eden's Mercurial extension for which we do not yet have
integration tests.

Reviewed By: wez

Differential Revision: D6279971

fbshipit-source-id: 901c1bc563a7a3910dde18cf2f0d8b8ff9cd6fbe
2017-11-09 16:29:56 -08:00
Michael Bolin
c6f59d25b8 Fix a crash that could occur when doing hg update .^ --merge.
Summary:
The underlying issue is that we were reporting a `MODIFIED_REMOVED`
conflict as a `MODIFIED_MODIFIED` conflict. This put us in a state where
Mercurial expected to find a file in the new manifest, but failed because the
file was not present in that revision, so no such file could be found.

Somewhat surprisingly, the appropriate handler for a `MODIFIED_REMOVED`
conflict already existed in our Mercurial extension, but there was no logic on
the server that would generate a `MODIFIED_REMOVED` conflict previous to
this change.

Like D6204916, this was an issue I ran into when trying to create a repro case
for the issue that was fixed in D6199215.

Reviewed By: wez

Differential Revision: D6270272

fbshipit-source-id: 6604eea00b0794cd44b01d2ba6b9ea10db32d556
2017-11-09 16:29:55 -08:00
Adam Simpkins
c3ebc91fdc update hg integration test inheritance to allow type checking
Summary:
Python 3 type checking currently complains about most of our integration
testing since the tests use an `hg_test` decorator to inherit from the base
test class.  This prevents the type checker from being able to figure out this
inheritance.

This updates all of the test cases to explicitly derive from the test case base
class, rather than using the decorator to do so.  I also renamed the base test
case class to `EdenHgTestCase` to be slightly more succinct and to follow the
common pattern of calling `unittest.TestCase` subclasses `FooTestCase`

Reviewed By: bolinfest

Differential Revision: D6268258

fbshipit-source-id: 09eef2f8217932a6516f78d17dddcd35c83b73da
2017-11-07 19:04:20 -08:00
Michael Bolin
4c24e5bd9f hg prev should leave untracked files alone if no conflicts, even if in a directory.
Summary:
This fixes an issue where the `DIRECTORY_NOT_EMPTY` conflict type reported by
the server was not handled by the client. Somewhat ironically, the fix appears
to be to explicitly "do nothing," though the important part of this revision is
the new integration test.

As this is only one test, I'm not convinced this covers all possible corner
cases, but it's certainly better than blowing up, which is what we did before.

Reviewed By: wez

Differential Revision: D6264069

fbshipit-source-id: a7c45a43776a903a4d6b6cdfb0ce75db9549c380
2017-11-07 19:04:20 -08:00
Michael Bolin
a43eb5afaf Add proper support for the UNTRACKED_ADDED conflict type.
Summary:
If you have an untracked file and you `hg update` to a commit that has
that file in the tracked state, then the contents of the untracked version
should be ignored, as they are replaced with the contents of the file in the
commit you are updating to. The untracked version should be backed up
as specified by `ui.origbackuppath`.

Previously, our code in `eden/hg/eden/__init__.py` mapped this to a merge action
named `c`, but we did not include that in our set of `actions`, so we were
getting a `KeyError` if you exercised this code path.

I discovered this while trying to reproduce the issue that I fixed in D6199215.

Reviewed By: simpkins

Differential Revision: D6204916

fbshipit-source-id: b70153428291bda9a8853a37c0955ad7cb3bd89d
2017-11-07 17:50:52 -08:00
Michael Bolin
ed155d84cc Add p and pr to the list of potential merge actions when dealing with conflicts.
Summary:
Upstream, some new merge actions were added:

* `p` https://phab.mercurial-scm.org/D776
* `pr` https://phab.mercurial-scm.org/D777

We must include entries for these in the list of `actions` that we build up in
`eden/hg/eden/__init__.py` because the `actions` dict gets passed through to
Mercurial's own `applyupdates()` function in `merge.py` that contains this line:

```
for f, args, msg in actions['p']:
```

Therefore, without an entry for `p` in `actions` here, we get a `KeyError`.

Reviewed By: markbt

Differential Revision: D6199215

fbshipit-source-id: a7408e5ef84a659f37e7771a7c15f6a4b14ae0f9
2017-10-31 12:24:16 -07:00
Adam Simpkins
227d851a9b update integration tests to support multiple hg configs
Summary:
Update the integration test framework so that we can run the hg integration
tests with several different hg config settings, using different sets of
mercurial extensions.

This adds code to test using flat manifest, treemanifest in hybrid mode, and
treemanifest in tree only mode.  However, the treeonly configuration is
disabled at the moment due to some bugs in treeonly behavior preventing it from
being able to create test repositories in treeonly mode.

Reviewed By: bolinfest

Differential Revision: D5685880

fbshipit-source-id: 081ead4e77cd14a7feb03381783395bd5a8fef4f
2017-08-23 18:49:33 -07:00
Adam Simpkins
b0dde83ff2 update logging flags in integration tests
Summary:
Update integration tests to pass logging settings in via the new `--logging`
argument instead of using `--vmodule`.  Now that eden log messages have all
been switched from glog to folly logging, the `--logging` flag needs to be
used.

Reviewed By: wez

Differential Revision: D5367618

fbshipit-source-id: 299f73a8376863e46b7fffe59c539e54c78caed4
2017-07-07 15:43:42 -07:00
Adam Simpkins
4bb5948640 fix an invalid memory access in the checkout code
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
2017-04-13 17:34:38 -07:00
Adam Simpkins
8fcaefe6b2 implement hg reset
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
2017-04-06 17:50:43 -07:00
Adam Simpkins
879fabdcc8 add an assert_status() method to the hg integration tests
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
2017-03-31 18:21:44 -07:00
Adam Simpkins
82cc79b694 update hg_import_helper.py to invalidate the repo if an error occurs
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
2017-03-31 14:07:14 -07:00
Adam Simpkins
080134ea84 ignore ENOENT errors when invalidating FUSE inode entries
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
2017-03-31 14:07:14 -07:00
Adam Simpkins
a18e042e1c tweak the hg integration test base class
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
2017-03-31 11:39:48 -07:00
Adam Simpkins
49adc9c96b fix the behavior of "hg update --clean ."
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
2017-03-31 11:39:48 -07:00