Summary:
In case there are many CommandDefinitions, we don't need all their flag
definitions until we decided to execute one of the commands.
Reviewed By: sfilipco
Differential Revision: D16796398
fbshipit-source-id: af205f59efd77fd7ff9eb4655d1f9167e2c350da
Summary: Convert global flags to `HgGlobalOpts` struct to make code shorter.
Reviewed By: sfilipco
Differential Revision: D16796407
fbshipit-source-id: b9d4c3dbec68c81908d439da4c353249347ca74a
Summary:
The tests are
- Mostly testing about configurations.
- Mostly depends on private functions.
ConfigSet already has a good test coverage, the tests do not provide much
value (aside from testing config override ordering, which is also covered
by hg tests). I'm going to change / remove the private functions. Remove the
tests to make changes easier.
Reviewed By: sfilipco
Differential Revision: D16796402
fbshipit-source-id: 56a8d55a0d1b0438bd2fcde5d3379d76f51dcd9d
Summary: This makes it possible to simplify the code reading global flags.
Reviewed By: sfilipco
Differential Revision: D16796405
fbshipit-source-id: eb604470d052ef84b748d1e60270cacd410fc5da
Summary:
The `Dispatcher` provides lots of features but its internal state only contains
the command table. Replace it with `CommandTable` and make the methods free
functions.
This makes function dependencies more cleaner, for example things like "locating
a repo", "getting the args" etc. won't require a `Dispatcher`.
A side effect of this change is the non-utf8 command line arguments are no longer
supported. This is already broken since our hg wrapper (accidentally) enforced
utf-8 command line. Therefore related tests are removed.
Reviewed By: sfilipco
Differential Revision: D16796395
fbshipit-source-id: a793ce7b8befe7caf62405c582fc932eb3daa099
Summary:
The dispatch logic couples with too much stuff - repo, parsing, command,
config. The command / registration part can be cleanly seprated.
This diff moves the core logic of CommandTable to command.rs.
Reviewed By: sfilipco
Differential Revision: D16796406
fbshipit-source-id: 5a33d6b6452537f07895bd9944122a3b3149d925
Summary:
Change `root` to normalize the repo path so it strips `\\?\` UNC prefix that
might break some (ex. Python) scripts.
Reviewed By: DurhamG
Differential Revision: D16928880
fbshipit-source-id: 9691b712f1ba0a07815d025e083d0cbd90b7d6a3
Summary: This makes the code reusable.
Reviewed By: DurhamG
Differential Revision: D16928881
fbshipit-source-id: cced0ecd6099e86c4a3f2f843e4dcfa9c6748e27
Summary:
Previously, the command table state in `Dispatcher` is confusing:
command_table: BTreeMap<String, CommandFunc>,
commands: BTreeMap<String, CommandDefinition>,
Question: In what case do these BTreeMaps have different keys?
It does not make much sense. Therefore merge `CommandFunc` into
`CommandDefinition`, and remove the `command_table` field.
This affects the `register` API:
fn register(&mut self, command: CommandDefinition, f: FN)
`f` is part of `CommandFunc`, which duplicates with `CommandDefinition`.
`CommandDefinition` contains 3 things: name, doc, and flags.
In the new `define_flags!` world, `flags` can be inferred from the type
of the function, so only `name` and `doc` are needed. Therefore change
the register function to:
fn register(&mut self, f: FN, name: &str, doc: &str)
Update `hgcommands` to use the newer APIs. Commands can now be registered
without going through `CommandDefinition` explicitly.
Reviewed By: sfilipco
Differential Revision: D16733275
fbshipit-source-id: 68e404a5b271b72aad52f640123b1c083f31d76c
Summary:
The only actual use of the function is to get Python command names (not
functions) so it can do prefix matching.
Just change the prefix matching logic to load the config and drop concepts
about "python" in CommandDefinition.
The Rust command table now only contains Rust commands.
Reviewed By: sfilipco
Differential Revision: D16733265
fbshipit-source-id: 7c680ef77874e9a136befc286cd26663ba82b09f
Summary: The enum owns the function of the command. Rename to make it clear.
Reviewed By: sfilipco
Differential Revision: D16733262
fbshipit-source-id: 6fdc7e999b0863b2a7b35203ac704928f8f92cd2
Summary:
Use `define_flags!` to auto generate conversion from `ParseOutput`.
A side effect is `args` is moved to the struct, and function signature becomes simpler.
Currently, `args` will also include the command name itself. This might be
useful when multiple commands share a similar implementation (ex. `next`, `prev`).
Reviewed By: sfilipco
Differential Revision: D16733269
fbshipit-source-id: fe32e41fe48a97d2d2f5a122522a17fa3c5f5f82
Summary:
This makes it possible to use one structure instead of a structure + another
`args` list.
The current version only supports a `Vec<String>` field and there is no verification
about how many arguments that `Vec<String>` can have. In the future we can add:
- Verification about arguments. Potentially change `From<ParseOutput>` to `TryFrom<ParseOutput>`.
- Individual named arguments as individual fields.
Reviewed By: sfilipco
Differential Revision: D16733272
fbshipit-source-id: 2bb407fff6cd1790cf33e8ce5527bb5e44255215
Summary: This can make code shorter.
Reviewed By: sfilipco
Differential Revision: D16733264
fbshipit-source-id: 27c0299c6e59bc30cfa14aa9ce122e2a542fd9c1
Summary:
Alias starting with `!` are considered shell commands. The entire command
string should be passed roughly as-is to the shell.
The current alias handling uses shlex::split to split the alias into arguments,
then replaces things like `$1` in arguments. The problem is, escaping
shlex::split a complex shell alias, then unesape (shlex::quote) it is not
loseless.
To maintain compatibility for existing complex shell alias configuration,
add a new code path that imitates the Python code behavior.
Reviewed By: sfilipco
Differential Revision: D16814144
fbshipit-source-id: 0e5e95f99bf8b8b16bd8d0cbcadd6760f7f77ebb
Summary: New degbugstore command prints contents of blob in store give filenname and hash.
Reviewed By: xavierd
Differential Revision: D16791780
fbshipit-source-id: d4529f3f368677b4f65a5772f82a1655552fefa5
Summary:
The indexes are lagging by design to reduce space overhead. For immutable Logs
(esp. used by RotateLog), the indexes no longer need to be lagging.
Make it so to reduce overhead opening RotateLog.
As we're here, also fix an issue where `rotate_assume_locked` was not really
protected by a lock.
Reviewed By: sfilipco
Differential Revision: D16587181
fbshipit-source-id: 3cf81864e90c875ee661dbd994bcd3ebc4b55322
Summary: `--no-foo` should only work if `foo` is a boolean flag.
Reviewed By: farnz
Differential Revision: D16715454
fbshipit-source-id: 9a8183586aa50fb55f30e19276bd60ebcc23a5fb
Summary: The helper function is no longer necessary.
Reviewed By: sfilipco
Differential Revision: D16715455
fbshipit-source-id: 3bf528d3c9f18418724793edebf298425a3bba87
Summary:
Previously, `get` returns an `Option<T>`. Almost all callers use `unwrap`
immediately, since they know what flag names are available. Let's just
move the `unwrap` inside the function. `get` in the Rust (and Python)
eco-system is considered nullable and panic-free. Rename it to `pick`
to make the difference more obvious.
I considered using the `Index` trait, but it has to return a reference,
which does not fit into this use-case.
Reviewed By: sfilipco
Differential Revision: D16715453
fbshipit-source-id: f754d208c4a1b0adeddaee82c7db82c790d6436c
Summary:
The default value of a flag is already provided with the flag. Therefore
`get_or_default` requiring another `default` does not make much sense.
Remove it.
Reviewed By: farnz
Differential Revision: D16713531
fbshipit-source-id: 95a55289077b7308083b6685f013d1058419a675
Summary:
This simplifies a lot of code. For example:
- let config_path: Vec<String> = result.get("config").unwrap().clone().try_into().unwrap();
+ let config_path: Vec<String> = result.get("config").unwrap();
Also, the use of `TryInto` does not make sense - the code only implements
`Into`, and `TryInto` will panic. Therefore remove `TryInto`.
Reviewed By: farnz
Differential Revision: D16713534
fbshipit-source-id: 29dc71881dd844fa87e086543fb0a3bb5d8837a1
Summary:
Replace `_` in field names to `-` as long flag name. That is consistent with
the current hg behavior.
Reviewed By: sfilipco, farnz
Differential Revision: D16713530
fbshipit-source-id: ce36caebc6b3131cb418d86ca0fdc507d2d8f17f
Summary:
Change the macro to have state so we can parse fields one by one with different
syntax.
Reviewed By: sfilipco
Differential Revision: D16713536
fbshipit-source-id: 7f8cb63cfa0ea000c2c2a431454c2d88d49b9187
Summary:
This macro provides `Vec<Flag>` and implements `From<ParseOutput>` for a
struct intended to be used as command options.
The core macro is just 21 lines. Both structopt-derive and gumdrop_derive have
1000+ lines.
Of course, it lacks of features. Some of those features are:
- No way to provide a "short flag".
- Default values have to be explicitly written.
- No way to compose structs (ex. reuse DiffOpts)
- Should "args" be part of the structure?
- Should "command help" be part of the structure?
Reviewed By: sfilipco
Differential Revision: D16713535
fbshipit-source-id: e8137e5f110ebb280fc40f84dace7b1d3a346c82
Summary:
This updates Mononoke to support LFS metadata when serving data over getpackv2.
However, in doing so, I've also refactored the various ways in which we currently access file data to serve it to clients or to process client uploads (when we need to compute deltas). The motivation to do that is that we've had several issues recently where some protocols knew about some functionality, and others didn't. Notably, redaction and LFS were supported in getfiles, but neither of them were supported in getpack or eden_get_data.
This patch refactors all those callsites away from blobrepo and instead through repo_client/remotefilelog, which provides an internal common method to fetch a filenode and return its metadata and bytes (prepare_blob), and separate protocol specific implementations for getpackv1 (includes metadata + file content -- this is basically the existing fetch_raw_filenode_bytes function), getpackv2 (includes metadata + file contents + getpackv2 metadata), getfiles (includes just file content, and ties file history into its response) and eden_get_data (which uses getpackv1).
Here are a few notable changes here that are worth noting as you review this:
- The getfiles method used to get its filenode from get_maybe_draft_filenode, but all it needed was the copy info. However, the updated method gets its filenode from the envelope (which also has this data). This should be equivalent.
- I haven't been able to remove fetch_raw_filenode_bytes yet because there's a callsite that still uses it and it's not entirely clear to me whether this is used and why. I'll look into it, but for now I left it unchanged.
- I've used the Mercurial implementation of getpack metadata here. This feels like the better approach so we can reuse some of the code, but historically I don't think we've depended on many Mercurial crates. Let me know if there's a reason not to do that.
Finally, there are a couple things to be aware of as you review:
- I removed some more `Arc<BlobRepo>` in places where it made it more difficult to call the new remotefilelog methods.
- I updated the implementation to get copy metadata out of a file envelope to not require copying the metadata into a mercurial::file::File only to immediately discard it.
- I cleaned up an LFS integration test a little bit. There are few functional changes there, but it makes tests a little easier to work with.
Reviewed By: farnz
Differential Revision: D16784413
fbshipit-source-id: 5c045d001472fb338a009044ede1e22ccd34dc55
Summary:
We have seen a case where the blackbox log file contains chunks of zeros where
it shouldn't. Right now, 2 zero bytes are a valid log entry (the first byte
indicates "no checksum", the second indicates the entry has length 0). That
means corruptions in the log file produces valid zero-sized entries. That in
turn can cause issues - some index functions assume the log entries to have at
least some bytes, or they will panic.
Given the fact that:
- `ChecksumType::None` is not actually used.
- The cost of calculating the checksum is negligible comparing to index
lookups.
Let's just remove the `ChecksumType::None` feature so every entry is enforced
checksum verified.
Regarding on blackbox, it already has proper error handling to deal with
corrupted indexedlogs.
Reviewed By: DurhamG
Differential Revision: D16773741
fbshipit-source-id: f0c55b34c3c92d55baa3b54560c2ac5549987a51
Summary:
In some cases, the file we are trying to fetch from the EdenAPI can be
redacted. We need to handle those cases. The good way to do so would be to
support this on a protocol level, but that is a *lot* of work. For now,
we are using the "tombstone approach", in which the ApiServer reacts to
redaction errors by injecting a special "tomstone" content, which the client
needs to recognize as not an error, but as a special case. In particular,
it needs to not fail the validation.
Reviewed By: markbt
Differential Revision: D16752747
fbshipit-source-id: 1bf126d50ecd6b862954284461b7db8f51e7e891
Summary:
Previously, `ancestors` only takes a single id, `gca_one` and `gca_all` only
take 2 ids. Extend them to accept a set. This is more consistent with the rest
of the APIs, ex. `heads` and `parents`.
Reviewed By: markbt, sfilipco
Differential Revision: D16672417
fbshipit-source-id: 98297486cb01215479e0e16f2189cf3053d60a1d
Summary: As the title. It uses segments to speed up the calculation.
Reviewed By: markbt, sfilipco
Differential Revision: D16672422
fbshipit-source-id: 4c66b4db031a4f1da6ce3f3ac802efa093c65f3a
Summary: Make them reusable for different tests.
Reviewed By: sfilipco
Differential Revision: D16672419
fbshipit-source-id: fc0bfbde833928620c13ba910c0bb2f2259e8714
Summary:
- from_span: Some create SpanSet using a single span, handy.
- push_set: Faster than `union` for certain cases.
- as_spans: provide fast paths for certain cases, ex. `set(id-1 for id in oldset)`.
- into: later the ancestor-related API will migrate to `impl Into<SpanSet>`,
this allows syntax like `gca_one((a, b))`.
Reviewed By: sfilipco
Differential Revision: D16672423
fbshipit-source-id: 5b0e46d18c9bdb7a68e5edf1552e1699a5bf672a
Summary:
Previously, `SpanSet::contains` can only test one `Id`. Change it to be able to
test a `Span`. This will be used in `Dag::parents` implementation.
Reviewed By: sfilipco
Differential Revision: D16672425
fbshipit-source-id: 769196b53b61c05b8a15537292cc1c24cd86013c
Summary:
Move the debug logic from test code to segment.rs so it can be used in the
Python bindings for debugging purposes.
Reviewed By: sfilipco
Differential Revision: D16672418
fbshipit-source-id: 9d2fef58ecc77167bdc44167ad14b58cb8eebc18
Summary:
The existing hg APIs support things like getting all the gcas, and testing
"isancestor" without calcuating the gca. Implement them in the dag crate to
match those features.
I picked `gca` to make names shorter. Here is a list of how those features
get named in two codebases:
Name in hg codebase | Name in this crate
-----------------------------------------
ancestor | gca_one
commonancestorsheads | gca_all
isancestor | is_ancestor
ancestors | ancestors
Reviewed By: markbt, sfilipco
Differential Revision: D16672420
fbshipit-source-id: d205d7970623e992e656ae300218239cddbd26c8
Summary:
This makes it possible to detect cases where it's no longer necessary to build
higher level segments.
Reviewed By: markbt
Differential Revision: D16683579
fbshipit-source-id: 77a0c2c1339616ee47778e4d0ab5ababb94e6bed
Summary:
Use the bindag format to store the DAG from mozilla-central. The bindag file
was generated by:
hg clone https://hg.mozilla.org/mozilla-central/
cd mozilla-central
hg debugbindag -o mozilla-central.bindag -r '::48797c5119a4d7fd1ff533501b6f80009cd3498c'
The file will be used to benchmark some parameters of the segment DAG.
Reviewed By: sfilipco
Differential Revision: D16660076
fbshipit-source-id: 4218e3d973e0b2847504733ae850d060b62bf729
Summary: This is useful where the user wants to define their own messages.
Reviewed By: markbt
Differential Revision: D16683580
fbshipit-source-id: 029e3b7720af7e53cb2b5b5efb1ebdd8ac940b67
Summary:
Previously, the function has the type `Fn`, which is less flexible.
Change it to `FnMut` so the function can have some side effects to
its environment.
Reviewed By: sfilipco
Differential Revision: D16660079
fbshipit-source-id: ef6b3e1688d265538df4fc67dccf5217125d0d15
Summary: Previously, `name` is `&str`. Change it to `impl ToString` to be more flexible.
Reviewed By: sfilipco
Differential Revision: D16660075
fbshipit-source-id: c5c88f6ffd7788e0a2cef95713b7e48c127c7a3d
Summary:
In case there are too many merges, the function will stack overflow.
Avoid it by emulating the stack. The heuristic to optimize certain
cases is removed to simplify the code.
Reviewed By: sfilipco
Differential Revision: D16660077
fbshipit-source-id: 6491227e76374548b25a0804fd71a630d392adb9
Summary: We never used this, and loosefiles are pretty much gone, let's remove the code.
Reviewed By: quark-zju
Differential Revision: D16726160
fbshipit-source-id: f810356a0a9cd980d8e5306bc967c3f25475afa6
