Summary:
Add blackbox logging of syncing to or from the commit cloud workspace,
including which changes are being submitted.
Also log when obsmarker fixup happens in blackbox.
Reviewed By: quark-zju
Differential Revision: D16961281
fbshipit-source-id: 0d0f675d77ab3446198703b31eea940dae3bdd85
Summary:
Add a debug log for when a repository is tracking visibility through visible
heads. It's not enough to just check if `visibility.enabled` is true, as the
repo needs to be migrated to visibility tracking, which might not have happened
yet.
Reviewed By: quark-zju
Differential Revision: D16961282
fbshipit-source-id: 8dec04dcc07e72e09f61399f9be04bdee339e262
Summary: This makes `config|debugconfig` as a debug command. Previously it was not.
Reviewed By: singhsrb
Differential Revision: D16866460
fbshipit-source-id: f1fcf0777d2fe6c1426f557ae1999710dd722109
Summary:
It is unclear where we generate basemfnodes that get to be equal to mfnodes
in our requests to the server. This logging allows us to track the codepath
that is problematic.
Reviewed By: quark-zju
Differential Revision: D16969888
fbshipit-source-id: b2d0aa7b573180664ad5cf8d6909451f089ac4a5
Summary:
Similar to the previous diff, opt out the revnum detection for the `repo.revs`
call used internally by directaccess to calculate revs.
Change smartlog to disable the revnum detection for its internal calculation.
Carefully choose the scope so `smartlog(1+2)` will still be warned.
Reviewed By: singhsrb
Differential Revision: D16954996
fbshipit-source-id: 2612055a8a9bb2fc9bdd1946c4bc565f967b22ca
Summary:
If `smartlog()` is the user input, the revnum detection is enabled for the
whole scope of smartlog revset implemenation. The use of rev numbers triggers
the warning. But the user didn't use any rev number.
Change smartlog to disable the revnum detection for its internal calculation.
Carefully choose the scope so `smartlog(1+2)` will still be warned.
Reviewed By: singhsrb
Differential Revision: D16954997
fbshipit-source-id: 1a3d32c1c2bcba08bfac908623b6416cf1cd63a8
Summary:
Instead of `debugsnapshot` and `debugcheckoutsnapshot` we will use
`snapshot create|checkout`.
Reviewed By: markbt
Differential Revision: D16961037
fbshipit-source-id: 142d072672376c2ce6a248a4dceffdd94d56a4df
Summary:
Aliases with `:doc` in name are not real commands. Do not treat them as
commands.
The upstream patch https://phab.mercurial-scm.org/D5087 added other metadata
including `:help` and `:category`. We might end up having some in the future
so I blacklisted names with `:` in them, not just `:doc`.
Reviewed By: chadaustin
Differential Revision: D16955596
fbshipit-source-id: b6f3e1129b632e0ba43c800e7e6fdd2b52d3b40c
Summary:
The type of `args` has changed. Tests did not capture the issue, since the code
block was not compiled using the default setup.
Reviewed By: sfilipco
Differential Revision: D16953736
fbshipit-source-id: a678f1354511f2d2ceb5faa7a4de0c0c6b03e84d
Summary:
`fs::canonicalize` has unwanted side effect - resolving symlinks.
Use `util::path::absolute` instead. This resolves a side effect where
Note on Linux, `chdir` resolves symlinks and `getcwd` (aka.
`std::env::current_dir`) will return the resolved path. On Windows
`std::env::current_dir` does not return the resolved path, at least
for the mapped drive created via the `subst` command.
Differential Revision: D16952484
fbshipit-source-id: 6969a1844020ff6b82de46416d8950ec40394159
Summary:
On Windows, some people use the "map drive" feature to map a long path (ex.
`C:\long\path\to\repo`) to a short path (ex. `Z:\`) so their tooling can
handle some long paths.
In that case, resolving symlinks by `hg root` is undesirable.
Unfortunately, the Rust stdlib does not have a Python `os.path.abspath`
equivalent. There were some attempts (ex. https://github.com/rust-lang/rust/pull/47363)
but the corner cases (ex. symlinks) have made the problem much more
complicated.
There are some 3rd-party crates. But they are not a good fit:
- https://github.com/danreeves/path-clean/ (last commit fb84930) follows the golang plan9 idea. It does not have proper support for Windows paths.
- https://github.com/vitiral/path_abs/ (latest commit 8370838) reinvents many path-related types, which is an overkill for this usecase.
This diff implements the feature "reasonably" for both Windows and Linux, with
nasty corner cases (symlink) ignored.
Differential Revision: D16952485
fbshipit-source-id: ba91f4975c2e018362e2530119765a380f103e19
Summary:
It does not seem to get used anywhere. Its usage of `scmutil.revrange` can
potentailly trigger a false positive "legacy revision number used" warning.
Reviewed By: chadaustin
Differential Revision: D16910635
fbshipit-source-id: 0b84d4b6a85678c581ccf1754bfbefbc97d8c055
Summary: The configparser crate is a better place for logic to load system and user config.
Reviewed By: sfilipco
Differential Revision: D16796396
fbshipit-source-id: c1ae5f85aa9839d89f5026279d699135dc1b442b
Summary:
Move `repo.shared_path` handling to repo initialization time and store it in
the repo structure.
This makes `repo.shared_path()` cheap if it gets used frequently.
Reviewed By: sfilipco
Differential Revision: D16796401
fbshipit-source-id: e19f3381cc87b55500ea1d27fd918ccb16a71972
Summary:
Some functions in `dispatch.rs` are about the "repo". Move them to a better
place - `repo.rs`.
The repo and config logic is coupled. A new enum `OptionalRepo` was added, to
make code easier to write - `Some(repo)` means the repo exists, and `repo` owns
the config. `None(config)` means the repo is missing, but the config is still
available.
Reviewed By: sfilipco
Differential Revision: D16796403
fbshipit-source-id: 2f4017a52296b629e990f85437b2cfdd7263b9e6
Summary:
"infer" means "try to get the repo path from command line arguments like a full path".
The enum variant is really "repo is optional". Rename to clarify.
Reviewed By: sfilipco
Differential Revision: D16796399
fbshipit-source-id: 505d2a406a83e0006200ece63d360b119548d2dd
Summary:
Change error type in clidispatch from `DispatchError` to `failure::Error`.
Pros:
- `failure` will attach a backtrace for free. (otherwise, backtrace handling is
manual)
- Wrapping other errors (ex. `io::Error`, `cliparser::Error`) is optional.
(otherwise, wrapping other errors is mandatory, and needs to be careful to
not lose information)
Cons:
- No longer able to enumerate *all* possible error types. (but can still
downcast to specific errors)
This seems to be a good tradeoff especially because of the backtrace handling - I
ran into a few issues where the location where the error happened really helped
debugging.
Since we can no longer enumerate all possible error types, the enum was changed
to individual structs to make the code shorter (ex. the struct can be downcasted
directly, instead of down-casting to the enum, then matching its variant).
The `HighLevelError` handling was simplified and moved to `hgmain`.
The new code path falls back to Python less aggressively, therefore some behaviors
were tweaked (ex. `-R` takes a bundle path).
Reviewed By: sfilipco
Differential Revision: D16796400
fbshipit-source-id: 1b17eb8d62503644b118c6b799778182bf222538
Summary:
This is more conistent with Mercurial style. And make them usable directly in
Rust code.
Reviewed By: sfilipco
Differential Revision: D16796397
fbshipit-source-id: 9016ea2b09fdf96b2b54138f5c8405caf96390f7
Summary:
This makes the "content" of the error stable. It is used in a later diff where
AmbiguousCommand gets handled by Rust directly instead of falling back to
Python dispatch.py.
Reviewed By: sfilipco
Differential Revision: D16796404
fbshipit-source-id: c439db14ec83c76c4762d3c627bfce1ea44bccf4
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:
At first, that aim was to create a good and consistent test plan of snapshots:
```
# Snapshot test plan:
# 1) Empty snapshot (no changes);
# 2) Snapshot with an empty manifest (changes only in tracked files);
# 3) Snapshot with a manifest (merge state + mixed changes);
# 4) Same as 3 but test the --clean flag on creation;
# 5) Same as 3 but test the --force flag on restore.
```
Then I discovered some bugs while writing the tests.
1) In case of an empty manifest the `snapshotmanifestid` extra should be an empty string, not `None`.
This is because of the extra serialization rules.
2) The initial approach for doing the checkout did some unnecessary operations with dirstate,
which required it to rollback later.
Now the overhead is gone, and the strategy of checking out is as follows:
```
1. Regular hg update to the (1st) parent of the snapshot;
2. Apply all the changes via mergemod.update without pushing it to the dirstate;
3. Register these changes in the dirstate;
4. Tie the second snapshot parent to the working context, if it exists.
Reviewed By: markbt
Differential Revision: D16929417
fbshipit-source-id: 0319ac0e81f80956cdef5b2a696b642cc782f48d
Summary: Snapshots have invisible commits underneath them, so that flag is redundant.
Reviewed By: markbt
Differential Revision: D16916268
fbshipit-source-id: 294f34af5aa7942be0bb2b58f0dc65979d593157
Summary: Due to a bug the missing files from the snapshot manifest were marked as removed after checkout.
Reviewed By: markbt
Differential Revision: D16831796
fbshipit-source-id: 200ccf34b16392a73d38294f1fcb43bfa9a94690
Summary:
D16910636 and D16898856 both add a `hex` call to a `node`. That results
in a 80-char commit id, which breaks 20+ tests. Back out D16910636 to fix
tests.
Reviewed By: simpkins
Differential Revision: D16931869
fbshipit-source-id: 5794cd6a2d1d1eda87c525231ef15b621f2074c5
Summary:
Log the full command-line arguments, and the traceback to a dedicated table if
revision numbers are used to resolve commits.
This should give us clues about why revision numbers get used in some subtle
code paths.
Reviewed By: singhsrb
Differential Revision: D16910639
fbshipit-source-id: 2cc1f6436b6100720fad65ea672dc4bfe2caa245
Summary:
`legacy.revnum:real` is set by `scmutil.rev*` to scope the revision number
detection to user-provided input. We shouldn't log it if the user does not
provide revision numbers.
This diff skips logging if the code runs outside the `scmutil.rev*` context.
This should remove most false positives.
Reviewed By: singhsrb
Differential Revision: D16921155
fbshipit-source-id: cc1402df8806a65f45079ef076bd81e1ed8ce0af
Summary:
A revset optimization pass rewrites revsets like 1+2+3 to use the `_list`
revset function that has a fast path to bypass the `stringset` function.
Change `_list` to detect revision numbers too.
Reviewed By: singhsrb
Differential Revision: D16910791
fbshipit-source-id: 51036b7d6f16b5388bbaf996864946df91a9f093
Summary:
There are some commands like `hg log --limit 50` that show up in the
"revision number used" cases but it's unclear how they acutally use
revision number.
To figure it out, I plan to also log tracebacks to a separate table.
To make the traceback more useful, I plan to do a "best effort"
printing the arguments passed to functions. That will probably help
figuring out what revset expression is passed to some layer.
Reviewed By: singhsrb
Differential Revision: D16910638
fbshipit-source-id: dbc493cd6ae571e3256305487f12842494c92a8a
Summary: This removes a false positive detection of "revision number used".
Reviewed By: singhsrb
Differential Revision: D16910636
fbshipit-source-id: d30d04f1a793598b75eab6adb07d4f9c18cb85a1
Summary:
`scmutil.rev*` should not be used for non-user-provided input. Change them to
`repo.anyrevs` or `repo[]` instead. This should reduce false positives of
revision number usage detection.
Reviewed By: singhsrb
Differential Revision: D16910637
fbshipit-source-id: b53bc9ba4ee52cb37c86f819d70898c14a6c569f
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:
Use of revision number is being deprecated. Change the moverelative commands
(`next`, `previous`, ...) to use nodes internally.
Reviewed By: mitrandir77
Differential Revision: D16898856
fbshipit-source-id: 71f6fc32776e2912795f086d11897d36b942b608
Summary:
We do not want to use censorship/blacklisting, as those names are ambiguous
and redaction has exactly the meaning we need.
Reviewed By: quark-zju
Differential Revision: D16890423
fbshipit-source-id: 1a6dcc61ca3fe77b7a938a37fb1ec9250ae9e15a
