Summary:
Context: in pushrebase replay job we run on Mononoke we also want to test our
hooks i.e. replay pushrebase requests that were denied on mercurial because of
a hook failure. The problem is that at the moment the only error message we have
is `prepushrebase.driver hook failed` (see xdb table `xdb.mononoke_production`,
query `select * from pushrebaserecording where pushrebase_errmsg like '%hook%'
limit 10;`). This error message tells us nothing, not even what hook failed.
To change it I suggest to make it possible for python hooks to raise HookAbort
and provide more information in the `hook_failure_reason` field.
Reviewed By: quark-zju
Differential Revision: D14131359
fbshipit-source-id: 69a2b51b30c78efadf3ba0d3332f46a777022568
Summary: In chef there is currently an override to 50, so if someone got more due to some reason, it is nice to add a message.
Reviewed By: quark-zju
Differential Revision: D14170749
fbshipit-source-id: 3f3c79e4e6103523c14c8d9c1600f104e3d5d3d8
Summary:
Rust tells us that Rng::choose and Rng::shuffle should be replaced by
SliceRandom::choose and SliceRandom::shuffle, so let's do it.
Reviewed By: singhsrb
Differential Revision: D14178565
fbshipit-source-id: 586eb2891f1c2cab0a3435c1b4ae8f870e7a3c25
Summary:
Tell serde to use an [untagged representation](https://serde.rs/enum-representations.html#untagged) of this enum. This means that `Parents::None` will map to a null value, `Parents::One` will map to an array representing a `Node`, and a `Parents::Two` will map to an array of 2 arrays representing `Node`.
Using CBOR serialization, this means that these variants are 1 byte, 21 bytes, and 43 bytes respectively.
Differential Revision: D14174309
fbshipit-source-id: 0217a23c4ee5409ab293525d7b6e5ae969b5504d
Summary: Add implementations for `FromIterator` and `IntoIterator` for the `Parents` type to make it more ergonomic to use.
Reviewed By: quark-zju
Differential Revision: D14172511
fbshipit-source-id: 5ba848c1dfbb8cc23ed19c9dc816616f5ed7af5f
Summary: Add a convenience function to `MutableHistoryPack` to add an entry from a `PackHistoryEntry` struct.
Differential Revision: D14162781
fbshipit-source-id: a0e07f34b9231011a339ce63adcef8ab55a0555e
Summary:
This changes the way we represent history entries for the Eden API by splitting them into two types and putting them into a new `historyentry` module.
- `PackHistoryEntry` is the same as the old `HistoryEntry`, containing the fields required to add this entry to a `MutableHistoryPack` (namely a `Key` and a `NodeInfo`).
- `LooseHistoryEntry` is a history entry containing the information that would normally be present in a line of the history text in the remotefilelog loose file format.
There are several reasons why it makes sense to have both of these types:
- The existing remotefilelog code in Mononoke uses a type very similar to `LooseHistoryEntry` internally, and as such having a similar type for API calls simplifies code on the server side.
- `PackHistoryEntry` contains redundant information (in particular, the file path may be duplicated up to 3 times). While it's structure is ideal for `revisionstore`'s in-memory data structures, for transmitting data, this redundancy is undesirable, especially since the client already has the file path (it is required to make the request in the first place).
- Conversions between these two representations include some subtle details that are tricky to get right. By putting the conversion in one canonical place, we can avoid having to duplicate this conversion logic in multiple places.
Differential Revision: D14162783
fbshipit-source-id: 63e0a060709916f21613442b75370f4d34a04f04
Summary:
Add a type representing a node's parents. Mercurial nodes can have zero, one, or two parents; this is normally represented as an array of two node hashes, with any absent parents denoted with a null hash. This representation is not particularly Rustic because it allows for invalid combinations (such as a null p1 and non-null p2) to be represented. By using an enum, invalid values are unrepresentable, making code using this type simpler (due to not requiring validation logic).
This type will be used later in the stack to represent parents in history entries.
Reviewed By: quark-zju
Differential Revision: D14162782
fbshipit-source-id: dfff3ecc76dea114e0044839216d080b7f34a506
Summary: `pastry` is the modern replacement for `arc paste`. Let's use it in `hg rage` instead of `arc paste`.
Reviewed By: singhsrb
Differential Revision: D14172108
fbshipit-source-id: 6586b9a8d8b90bac55d91baed852edbc7c1d9db9
Summary:
`Durable` nodes are inner nodes that come from storage. Their contents can be shared between multiple instances of Tree. They are lazily evaluated. Their children list will be read from storage only when it is accessed.
The inner structure of a durable link poses an interesting question related to handling failures: what do we do when we have a failure when reading from storage?
We can cache the failure or we don't cache it. Caching it is mostly fine if we had an error reading from local storage or when deserializing. It is not the best option if our storage is remote and we hit a network blip. On the other hand we would not want to always retry when there is a failure on remote storage, we'd want to have a least an exponential backoff on retries. Long story short is that caching the failure is a reasonable place to start from.
The lazy-init crate is useful for modeling the lazy initialization model that we have for durable node links. See docs at https://docs.rs/lazy-init/0.3.0/lazy_init/
Reviewed By: quark-zju
Differential Revision: D14142928
fbshipit-source-id: 077f708b38e2ace772f30b3392445326ce17f47c
Summary:
The store abstracts the data that we need from the store for TreeManifest. This is not a long term abstraction, mostly something that we can code against until we write the real parsing logic from the real data store.
Adding a TestStore implementation that we can use in tests and debug.
Reviewed By: quark-zju
Differential Revision: D14142930
fbshipit-source-id: 7f2d4f05a7b7e63758db9247cdbcd51541c88ec0
Summary: Preparing to add the store abstraction for the tree manifest. This store implementations are going to be tied to the tree implementation and should be made private from the rest of the crate. To do this we move the tree to a module.
Reviewed By: quark-zju
Differential Revision: D14142929
fbshipit-source-id: 588d597e1248fc2e632c9efe03f08ba3d491e8cd
Summary: manifest::tree::Link has several convenience functions. In the grand scheme of things this is something that we will want because they are common operations. The problems is that the node structure is currently changing rapidly and the additional layer of abstraction is hindering iteration. I don't know the exact shape that the nodes will have and trying different things out is easier when the Tree functions use the nodes directly.
Reviewed By: quark-zju
Differential Revision: D14142926
fbshipit-source-id: 5e4e8f4d3e1b19fd14dcc290a3dea4271b502d97
Summary:
Currently, stackpush requires specifying the original node for each
new node to be created. This inhibits the creation of new commits which are not
replacing any commit via stackpush. This commit changes the behaviour such that
specifying the original node is optional.
Reviewed By: quark-zju
Differential Revision: D14153674
fbshipit-source-id: b3d150a8a8044ac1740937f2dc058ce542ee13e4
Summary:
First stab at a job that will keep hg in sync with Mononoke when Mononoke
becomes a source of truth.
Reviewed By: ikostia
Differential Revision: D14018269
fbshipit-source-id: f88c5eba8bf5482f2f162b7807ca8e41a3b4291d
Summary: There was a typo, the timezone is `[1]` and not at `[0]`
Reviewed By: ikostia
Differential Revision: D14147329
fbshipit-source-id: 8ad4bff810ed949a9f8e86d03ef62bc63aaf11bd
Summary:
Alias documentation can now include multiple lines, however for the output
of `hg help commands`, only the first line should be used.
Reviewed By: quark-zju
Differential Revision: D14149607
fbshipit-source-id: 84992078c9bc4659532350b47694c562f4dc1983
Summary:
Remove:
- log --only-branch
- pull --branch
- push --branch
- record --close-branch
- incoming --branch
- outgoing --branch
Currently, `log --branch` is kept since some arcanist code still uses it.
We can probably remove it after the next arcanist release.
Differential Revision: D14116320
fbshipit-source-id: 50960ae8700200b322f615c4defd9c05353c2435
Summary:
CommandError happens if there is an unknown command flag, or a required
argument is missing. The old behavior is to print an error message to
stderr, then start the pager with the command help printed to stdout.
There are 2 problems with that approach:
1. When using mosh, a long help text might flush the actual error to out of the
screen. The error message will be missed.
2. When captured in shell scripts, the help text printed to stdout would be
captured, which is almost always undesirable.
The actual motivation of this change is for 2. Zsh themes like bullet-train [1]
uses `hg id -b 2>/dev/null` and we'd like to remove `id -b` support. After that,
the command should not polluate stdout with help text.
[1]: bd88ade263/bullet-train.zsh-theme (L102)
Differential Revision: D14151200
fbshipit-source-id: edd38e91115f96929438379aa2e40edfba560b41
Summary:
It's possible that the non-mergedriver code path decides to create ("get") a
file, because it only exists in one side of the merge parents, while
mergedriver decides to remove that file. That makes the file exist in both
"g" (get, aka. create) and "r" (remove) lists and result in incorrect
dirstate state.
Fix it by detecting the case and remove the conflicted files from the "g" list.
Reviewed By: DurhamG
Differential Revision: D14145990
fbshipit-source-id: f4e5d9787dca5cda6cae270f8a84296ecddf553c
Summary:
vipannalla encountered an issue that his mergedriver code does not know the
complete list of files to "driver-resolve" at the "preprocess" step.
When doing "queueremove" files from one-side in the "conclude" step, the
dirstate would have "!" state afterwards.
Add a test to show that.
Reviewed By: DurhamG
Differential Revision: D14145989
fbshipit-source-id: e316aedea3359260dfbf7578a332f87b99fb37f9
Summary: the push-path will be used for Mononoke write path roll out.
Reviewed By: markbt
Differential Revision: D14153207
fbshipit-source-id: 158e6e08dfa017c98e668fdfc37138dd7b7d76c7
Summary:
We can't run in parallel at the moment as the log file and the lock file are
shared.
Every path maintains independent backup state (the previous diff).
The secondary backup state doesn't affect smartlog (only the main one)
The issue with this approach is that we maintain backup lock a bit longer.
Unfortunately, the progress in smartlog doesn't show anything about the second backup.
I added 'finished', it makes it easier to compare in the logs.
Reviewed By: markbt
Differential Revision: D14149399
fbshipit-source-id: f90e8aac6cb8dee53d5c7468bd6adba067e13362
Summary:
We are going to support 2 different backends of Commit Cloud: Mercurial and
Mononoke.
Each of them should maintain local backup state separately.
Output of some tests have been slightly changes, this is because a separate backup
state, the same error appears earlier when we are trying the backup stacks.
The idea is to have separate backup states for different remote paths, but
there will be only one cloud sync state for the current source of
truth. We could include there the remote path and then validate that cloud sync
state is correct if the remote path has been changed.
However, for backup states it is much easier to have them separately (and we
will backup in 2 places)
Reviewed By: markbt
Differential Revision: D14138496
fbshipit-source-id: 0a7a763a395be5456cbd724bff7ebc069f03fb0e
Summary: Add conversions between the `HgFileHistoryEntry` type produced by Mononoke and the `edenapi_types::HistoryEntry` type shared between Mercurial and Mononoke. The latter can be serialized and sent from the API server to the Mercurial client, and will be used for HTTP history fetching.
Reviewed By: quark-zju
Differential Revision: D14079338
fbshipit-source-id: 2123c88310f633f87d8c92405382d5046874dfee
Summary: The `hg update` command has short flags for `--merge` and `--clean`, namely `-m` and `-C`. Let's add the short versions to `hg next` and `hg prev` as well.
Reviewed By: quark-zju
Differential Revision: D14155218
fbshipit-source-id: d51f5f658b525809e4c512ffa300dea4b4cabcdf
Summary:
We're encountering an issue that I think is caused by invalid data
coming back from memcache, possibly due to our recently introduced connection
reusing. Let's add some checksums to verify that the data we put in is identical
to the data we get out for a given key.
Reviewed By: kulshrax
Differential Revision: D14141683
fbshipit-source-id: 206b51b862db7d54def02f5310b90f473d5a0d03
Summary: We weren't passing the `--merge` flag from `hg prev` and `hg next` to the underlying invocation of `hg update`. This diff fixes the problem so that `--merge` now works as expected.
Reviewed By: DurhamG
Differential Revision: D14154244
fbshipit-source-id: 4a2412cca714ec8f269eb5f2989e39821642fbb3
Summary:
Backs out D13944829 that add preloading the manifest to pushrebase. In
a situation where you receive tree commits, but the repo is hybrid, this causes
it to try to lookup the bundled tree manifests in the flat manifest revlog.
Let's just back this out for now until we can come up with an appropriate fix,
or move all our repos to treeonly.
Reviewed By: quark-zju
Differential Revision: D14151876
fbshipit-source-id: 0f7419d5b9bcd9d7ce363f4349e9e2e4a86cf713
Summary: Add the current clock value to the output of `hg debugstatus`
Reviewed By: quark-zju
Differential Revision: D14150641
fbshipit-source-id: 917ac3095bc933c042c0f057d0dbda38ef710844
Summary:
If treestate is in use, fsmonitor state is stored in the treestate, rather than
in a separate fsmonitor.state file. Update rage to understand this.
Reviewed By: quark-zju
Differential Revision: D14131131
fbshipit-source-id: d80d766625d7915b6a76f66f33e763eed23677e9
Summary: `RepoPath` and `PathComponent` were recently added as path abstractions for source control internals. This diff makes use of these abstractions in the manifest crate.
Reviewed By: quark-zju
Differential Revision: D14142925
fbshipit-source-id: 1e0033f4f2aeb5c1d63899adbce198302086bfda
Summary:
While working on TreeManifest I found that the code does not allow creating an empty `RepoPath`. I did not realize that splitting an empty string always results in an empty string. This is an edge case and we need to handle it appropriately.
Currently adding a hack where the components iterator function will skip empty values. We are going to add more iterators in a future revision and we should revisit this function then.
Reviewed By: quark-zju
Differential Revision: D14142927
fbshipit-source-id: 1be43d61913138e3fa50e02976e81f9ca38a74a7
Summary:
With the Store trait, we can de-duplicate code between the datapack repack, and
the historypack repack.
Reviewed By: quark-zju
Differential Revision: D14091894
fbshipit-source-id: 5bf335414df2420b42ec45cce7097f3a97a49796
Summary:
This is a perf optimization. `unbundlereplay.respondlightly` instructs the
server to not produce the pushback parts regardless of what `replycaps` part
of the incoming bundle says. This is important, since the mononoke-hg sync will
send all the bundles in a searialized way, so we want to optimize time where
possible.
Reviewed By: StanislavGlebik
Differential Revision: D14131575
fbshipit-source-id: afec15347d43fa52b1ec64b4ac8ece5b227ccf7d
Summary:
Add a `pager.stderr` config option that when set to false, disables the
redirection of stderr to the pager.
This means progress bars will be rendered, and other information will appear
synchronously, rather than buffered by the pager, but also that Mercurial will
start outputting stderr over the top of any output that the pager produces,
potentially corrupting the pager display.
Reviewed By: simpkins
Differential Revision: D14130965
fbshipit-source-id: eba8e31fdad05c935eae13e1900c5f7e5af7ecbc
Summary:
This test has been extremely flaky in the last few weeks, causing builds to not
being released, or just sandcastle jobs failing randomly. The reason for the
flakyness was due to the mtime stored in the dirstate being -1 instead of the
real mtime of the file.
The reason for the mtime to be -1 is to fix a race where a file is modified in
the same second the dirstate is updated. This can happen when a transaction
spans less than a second. By forcing a transaction to always last at least a
second, we can avoid this issue and make the test stable.
Reviewed By: quark-zju
Differential Revision: D14141924
fbshipit-source-id: ce151cd65d930f8c745633f7575ea006748af9b9
Summary: I am making this a new diff because the I am documenting failures which only make sense when validation was already added.
Differential Revision: D14097490
fbshipit-source-id: 7eb2c22ebbbc4365f8203d68bc29134176f326f6
Summary:
When introducing `Component` we said that it will not have any slashes (`/`). This diff adds enforcement for that along with enforcing that `Component` is not empty.
`Path` and by extension `Component` also have a set of invalid bytes: `\0`, `\1` and `\n`. I got these checks from the mononoke codebase (mononoke-types/path.rs)
Reviewed By: DurhamG
Differential Revision: D14001106
fbshipit-source-id: 5acbdf66214e54f1034fd89d90e88c3e904d7f9b
Summary:
`Component` and `ComponentBuf` are specializations of `RelativePath` and `RelativePathBuf` that do not have any separators: `/`. The main operation that is done on paths is iterating over its components. The components are generally directory names and occasionally file names.
For the path: `foo/bar/baz.txt` we have 3 components: `foo`, `bar` and `baz.txt`.
A lot of algorithms used in source control management operate on directories so having an abstraction for individual components is going to increase readability in the long run. A clear example for where we may want to use `ComponentBuf` is in the treemanifest logic where all indexing is done using components. The index in those cases must be able to own component. Writing it in terms of `RelativePathBuf` would probably be less readable that writing it in terms of `String`.
This diff also adds the `RelativePath::components` which acts as an iterator over `RelativePath` producing `Component`.
The name `Component` is inspired from `std::path::Component`.
Reviewed By: DurhamG
Differential Revision: D14001108
fbshipit-source-id: 30916d2b78fa89537fc5b30420b3b7c12d1f82c7
Summary:
`RelativePath` and `RelativePathBuf` are types for working with paths specialized for source control internals. They are akin to `str` and `String` in high level behavior. `RelativePath` is an unsized type wrapping a `str` so it can't be instantiated directly. `RelativePathBuf` represents the owned version of a `RelativePath` and wraps a `String`.
RelativePaths are going to be used in all contexts for SCM so flexible and efficient abstractions for dealing with common needs is important.
The inspiration for `RelativePath` and `RelativePathBuf` comes from the `std::path` crate however we know that the internal representation of a path is consistently an array of bytes where directories are delimited by `/` thus our types can have a simpler representation. It is because of the same reason that we can't use the abstractions in `std::path` for internal uses where we need to apply the same algorithm for blobs we get from the server across all systems.
This diff adds a new crate called `vfs`. The recommended use of the types in this crate are `use ...::vfs` followed by `vfs::RelativePath` and `vfs::RelativePathBuf`.
Alternatives for representing the path:
* We could use `String` and `&str` directly however these types are inexpressive and have few guarantees. Rust has a strong type system so we can leverage it to provide more safety.
* Mononoke use `PathElement(Vec<u8>)` and `MPath(Vec<PathElement>)`. One issue is that this type requires more allocations. Having read a buffer it is more difficult to provide views into it as &MRelativePath.
`RelativePath` is Dynamically Sized Type (DST). That means that `mem::transmute` is the practical way of constructing it in Rust right now:
* https://doc.rust-lang.org/nomicon/exotic-sizes.html
* https://github.com/rust-lang/rust/pull/39594#issuecomment-279471476
Basing these types on `String` and `&str` means that the paths that we deal with need to have UTF-8 encodings. In the grand scheme of things a lot of things are simplified when we can make this kind of assumption.
The code right now doesn't have much documentation. I'll add that if this direction makes sense to other people.
Reviewed By: DurhamG
Differential Revision: D14001109
fbshipit-source-id: 1ca399827b1284f32cd83219e7e090d8b2487cee
Summary:
A lot of code is duplicated between data stores, and history stores, and one
reason for it is the absence of common trait between these 2. By adding a new
Store trait it will make it easier to write generic code that works accross
data and history store.
Reviewed By: quark-zju
Differential Revision: D14091899
fbshipit-source-id: deef1d43a7d300cb3607c67554ad54f20c870e23
Summary:
Instead of manually implementing DataStore/HistoryStore for Box, Rc, Arc, and
future smart pointers, we can simply implement the trait for all the types that
can be deref into a DataStore/HistoryStore.
Reviewed By: quark-zju
Differential Revision: D14078072
fbshipit-source-id: 47a80ab0179b84aa08836b6e7c5c3c5f9c1a08ff
Summary:
As we are moving to smooth switching to Mononoke and backwards, we can start to deprecate
Mercurial specific optimizations to simplify the code.
Reviewed By: markbt
Differential Revision: D14131594
fbshipit-source-id: fa927011890ecdf0874a3a74b4910412b3c84b70
Summary:
Prepare the code to use 'known' wireprotocol command to check what is backed up
on the server instead of slower 'lookup'. We don't need to reestablish ssh
connection, and we can test all the nodes in one go.
Also test the 'known' request with Mononoke is working (all commands have been
tested with isbackedupnodes => isbackedupnodes2)
Reviewed By: markbt
Differential Revision: D14131383
fbshipit-source-id: 5a150b64d0e84a02357c2367879b2da8493d9167
Summary: Add the actual verification of the server-side repo after `unbundlereplay`
Reviewed By: StanislavGlebik
Differential Revision: D14131577
fbshipit-source-id: 18828479b07a110f26fd8adf108eb94a21d817c6
Summary:
Use the same way to check if the commits have been backed up in Mononoke and Mercurial.
The only common way to check is 'lookup' command because Mercurial doesn't support discovery for commit cloud commits, the command 'known' is also not supported.
Also we have to go one by one because lookup doesn't got any better API.
It is still much faster than backup commits that are already there.
Introduce such check for pushbackup as well.
Remove hacky way to check it from cloud sync.
For commit cloud in Mononoke we will have backfill, so the server side check will be heavily used when you go to Mononoke at the first time.
Unfortunately connection pool module in mercurial is not good enough in detecting closed connections and can easily return a broken connection on the next call.
Reviewed By: markbt
Differential Revision: D14085849
fbshipit-source-id: d76d9a71f9efdbdfec4de3198cd428b6b693418d
Summary:
This is to be used from Mononoke->hg sync.
Currently expects only `pushrebase` bundles, e.g. one head and one book to
move.
Reviewed By: StanislavGlebik
Differential Revision: D14116130
fbshipit-source-id: 959a6e51f51e21da5592c84188e294a33057ffaa
