( )⚙ D10296 rhg: read [paths] for `--repository` value

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10296

rhg: read [paths] for `--repository` value
ClosedPublic

Authored by pulkit on Apr 2 2021, 6:46 AM.

Details

Reviewers
Alphare
SimonSapin
Group Reviewers
hg-reviewers
Commits
rHGebdef6283798: rhg: read [paths] for `--repository` value
Summary

hg parses -R and --repository CLI arguments "early" in order to know which
local repository to load config from. (Config can then affect whether or how to
fall back.)

The value of of those arguments can be not only a filesystem path, but also an
alias configured in the [paths] section. This part was missing in rhg and
this patch implements that.

The current patch still lacks functionality to read config of current repository
if we are not at root of repo. That will be fixed in upcoming patches.

A new crate home is added to get path of home directory.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Apr 2 2021, 6:46 AM
Alphare requested changes to this revision.Apr 3 2021, 12:02 PM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
rust/rhg/src/main.rs
175

The nested ifs can be replaced by:

match config_val {
    Ok(Some(val)) if val.len() > 0 => Some(get_path_from_bytes(val.as_bytes())),
    _ => Some(get_path_from_bytes(&repo_arg)),
}

Also, you're using get_str, when you should probably just be using get, which already returns bytes (and does not unnecessarily force UTF8). That would remove the need for as_bytes() in the above suggested code.

This revision now requires changes to proceed.Apr 3 2021, 12:02 PM
pulkit updated this revision to Diff 26659.Apr 5 2021, 4:33 PM
pulkit marked an inline comment as done.Apr 5 2021, 4:34 PM
Alphare accepted this revision.Apr 6 2021, 4:59 AM
SimonSapin added a subscriber: SimonSapin.Apr 6 2021, 5:22 AM

Looks good, but there seems to be yet another edge case within the edge case… *sigh*. See inline comment.

tests/test-globalopts.t
73–80

This tests suggests that relative paths in [paths] are resolved relative to the user’s $HOME directory, not relative to the the config file nor the processes current directory. To be honest I find this really weird and unexpected, but it does appear to be the case when I change HOME=/somethingelse in the test and I do see root = os.path.expanduser(b'~') in mercurial/ui.py before _applyconfig calls fixconfig which looks at [paths] items and makes them absolute.

When run with --rhg this test ends up passing but not quite for the right reason. rhg resolves path a relative to its current directory which is the repository c. That repository happens to contains a file named a, so Repo::find returns HgError::unsupported("bundle repository") which causes fallback to Python. Fallback is what we want, but only because the identify command is not implemented (yet).

It looks like os.path.expanduser returns its input unchanged when getting the home directory fails, so the code above should be something like Some(home::home_dir().unwrap_or_else(|| PathBuf::new("~")).join(get_path_from_bytes(val)))

pulkit updated this revision to Diff 26679.Apr 7 2021, 6:21 PM
pulkit added inline comments.Apr 7 2021, 6:25 PM
tests/test-globalopts.t
73–80

Ugh!

Btw, how did you figured out that we are falling back with HgError:unsupported("bundle ...")? I mean is there some config or something which shows these debug prints or is there any other way?

I updated the patch after fighting with borrow checker a bit, does seems bit ugly to me, kindly have a look.

SimonSapin accepted this revision.Apr 8 2021, 4:06 PM

This version of the patch looks good, thanks!

Re debugging, there isn’t an easy way unfortunately. --config rhg.on-unsupported=abort prints the string passed to HgError::unsupported, but also disables fallback. We could add a fourth mode that keeps fallback but also has a debug log line printed to stderr.

What I did was add a use of the dbg! macro in main.rs to print the error value: https://doc.rust-lang.org/std/macro.dbg.html Since the value contains a byte string, the derived implementation of the Debug trait prints an array of integers. I copy-pasted that into a Python REPL and passed it to bytes() decode it as ASCII. Really not a good debugging experience.

Alphare added a comment.Apr 9 2021, 3:00 AM
In D10296#157162, @SimonSapin wrote:

Re debugging, there isn’t an easy way unfortunately. --config rhg.on-unsupported=abort prints the string passed to HgError::unsupported, but also disables fallback. We could add a fourth mode that keeps fallback but also has a debug log line printed to stderr.

Why don't we add a simple log::debug (with a possible rhg target in case we have a log consumer that allows for filtering by targets)? I've done this (albeit with a log::trace in rust/hg-cpython/src/dirstate/status.rs:89, and it's handy enough to use RUST_LOG=debug to get the debug entries printed.

pulkit retitled this revision from rhg: try read [paths] for `--repository` value to rhg: read [paths] for `--repository` value.Apr 10 2021, 5:15 PM
pulkit edited the summary of this revision. (Show Details)
pulkit updated this revision to Diff 26787.
pulkit added a comment.Apr 10 2021, 5:18 PM

@SimonSapin I updated this patch to also include reading .hg/hgrc and .hg/hgrc-not-shared of current dir. Kindly have a re-look. Thanks!

pulkit added a comment.Apr 10 2021, 5:20 PM
In D10296#157162, @SimonSapin wrote:

This version of the patch looks good, thanks!
Re debugging, there isn’t an easy way unfortunately. --config rhg.on-unsupported=abort prints the string passed to HgError::unsupported, but also disables fallback. We could add a fourth mode that keeps fallback but also has a debug log line printed to stderr.

+1 for fourth mode. Will help a lot in case of testing new code.

marmoute added a subscriber: marmoute.Apr 20 2021, 6:23 AM

Note: I suspect we are lacking a test coverage for --repository FOO, where FOO is defined as a path://.

SimonSapin added a comment.Apr 20 2021, 6:43 AM

That’s quite possible, but we have code a few lines above this in main.rs that falls back to Python for any --repository value that looks like a URL. So I’d expect that case to be handled correctly (if not quickly).

marmoute added a comment.Apr 20 2021, 6:44 AM
In D10296#159296, @SimonSapin wrote:

That’s quite possible, but we have code a few lines above this in main.rs that falls back to Python for any --repository value that looks like a URL. So I’d expect that case to be handled correctly (if not quickly).

ha ! "great" then.

Alphare requested changes to this revision.May 20 2021, 9:34 AM

Sorry, this seems to have been forgotten somehow. @SimonSapin please also take a look since you requested changes.

rust/rhg/src/main.rs
172

Use repo_arg.is_empty() instead

178

if current_dir.is_err() is simpler. However, you're using current_dir.unwrap() right after, so in the end you could simply do:

if let Ok(current_dir_path) = current_dir {
    ...
} else {
    None
}
192

You could use config.ok() here.

211

Why do we unwrap here?

230

Same question as above

236

Could use config_val.or(Some(get_path_from_bytes(&repo_arg).to_path_buf()))

This revision now requires changes to proceed.May 20 2021, 9:34 AM
pulkit added a comment.May 20 2021, 10:29 AM

Sorry for being silent for long, catching up now. Will update this patch.

pulkit marked 3 inline comments as done.May 24 2021, 6:59 AM
pulkit edited the summary of this revision. (Show Details)
pulkit added a child revision: D10767: rhg: look for repository in ancestors also instead of cwd only.
pulkit updated this revision to Diff 28192.
pulkit added inline comments.May 24 2021, 7:01 AM
rust/rhg/src/main.rs
192

You mean if config.ok()?

211

.canonicalize() returns a Result.

Alphare added inline comments.May 25 2021, 4:28 AM
rust/rhg/src/main.rs
192

No, I'm referencing https://doc.rust-lang.org/std/result/enum.Result.html#method.ok, which could replace the entire if expression

211

Well sure, but what happens if it's an Err? Mercurial will just panic and throw a nasty traceback in Python context, and a Rust panic message in Rust ones.

pulkit updated this revision to Diff 28201.May 25 2021, 6:01 PM
Alphare added inline comments.May 26 2021, 4:06 AM
rust/rhg/Cargo.toml
15

I forgot when first reading this patch, but we usually give a reason for adding a new dependency in the commit message.

rust/rhg/src/main.rs
203

I'm assuming this means that we silently ignore this error in the Python implementation as well?

221

This is canpath.unwrap_or(non_repo_config_val)

pulkit added inline comments.May 26 2021, 4:04 PM
rust/rhg/Cargo.toml
15

Will add.

rust/rhg/src/main.rs
203

Not sure what exact part in python will mean same as this one. But I see python code will raise IOError if it fails to read config from a file path passed.

221

but we are puting canpath.unwrap in Some() again, will it work?

baymax edited the summary of this revision. (Show Details)May 26 2021, 4:48 PM
baymax updated this revision to Diff 28206.

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare added inline comments.May 27 2021, 3:30 AM
rust/rhg/src/main.rs
203

This should probably do the same thing then.

221

Ah sorry, I misread. This is canpath.ok().or(non_repo_config_val). But you can use a match expression if you find it clearer also. unwrap raises an immediate alarm of "this can panic if not handled correctly or if the code changes in the future", so we try to avoid doing it in favor of letting the type system check all code paths for us.

baymax updated this revision to Diff 28504.Jun 7 2021, 7:15 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

SimonSapin added inline comments.Jun 7 2021, 10:59 AM
rust/rhg/src/main.rs
187

.ok() here together with the handling of None later silently ignore config parse errors, where Python-based hg aborts. However:

  • This is a rather niche corner case
  • Handling such errors in this particular function is tricky because we can’t just use the ? operator
  • This patch still improves things already, and has been open long enough

So please add a // TODO: handle errors from load_from_explicit_sources` comment and let’s follow up later.

Similarly please add a TODO comment above for handling errors from current_dir

pulkit updated this revision to Diff 28513.Jun 9 2021, 3:51 AM
Alphare accepted this revision.Jun 9 2021, 5:10 AM
This revision is now accepted and ready to land.Jun 9 2021, 5:10 AM
pulkit added a commit: rHGebdef6283798: rhg: read [paths] for `--repository` value.Jun 9 2021, 5:37 AM
Closed by commit rHGebdef6283798: rhg: read [paths] for `--repository` value (authored by pulkit). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 28515

rust/Cargo.lock

version = "0.1.0" version = "0.1.0"
dependencies = [ dependencies = [
"chrono", "chrono",
"clap", "clap",
"derive_more", "derive_more",
"env_logger", "env_logger",
"format-bytes", "format-bytes",
"hg-core", "hg-core",
"home",
"lazy_static", "lazy_static",
"log", "log",
"micro-timer", "micro-timer",
"regex", "regex",
"users", "users",
] ]
[[package]] [[package]]

rust/hg-core/src/config.rs

// config.rs // config.rs
// //
// Copyright 2020 // Copyright 2020
// Valentin Gatien-Baron, // Valentin Gatien-Baron,
// Raphaël Gomès <rgomes@octobus.net> // Raphaël Gomès <rgomes@octobus.net>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
//! Mercurial config parsing and interfaces. //! Mercurial config parsing and interfaces.
mod config; mod config;
mod layer; mod layer;
mod values; mod values;
pub use config::{Config, ConfigValueParseError}; pub use config::{Config, ConfigSource, ConfigValueParseError};
pub use layer::{ConfigError, ConfigParseError}; pub use layer::{ConfigError, ConfigParseError};

rust/hg-core/src/repo.rs

/// ancestors. /// ancestors.
/// ///
/// A method with two very different "modes" like this usually a code smell /// A method with two very different "modes" like this usually a code smell
/// to make two methods instead, but in this case an `Option` is what rhg /// to make two methods instead, but in this case an `Option` is what rhg
/// sub-commands get from Clap for the `-R` / `--repository` CLI argument. /// sub-commands get from Clap for the `-R` / `--repository` CLI argument.
/// Having two methods would just move that `if` to almost all callers. /// Having two methods would just move that `if` to almost all callers.
pub fn find( pub fn find(
config: &Config, config: &Config,
explicit_path: Option<&Path>, explicit_path: Option<PathBuf>,
) -> Result<Self, RepoError> { ) -> Result<Self, RepoError> {
if let Some(root) = explicit_path { if let Some(root) = explicit_path {
if root.join(".hg").is_dir() { if root.join(".hg").is_dir() {
Self::new_at_path(root.to_owned(), config) Self::new_at_path(root.to_owned(), config)
} else if root.is_file() { } else if root.is_file() {
Err(HgError::unsupported("bundle repository").into()) Err(HgError::unsupported("bundle repository").into())
} else { } else {
Err(RepoError::NotFound { Err(RepoError::NotFound {

rust/rhg/Cargo.toml

[package] [package]
name = "rhg" name = "rhg"
version = "0.1.0" version = "0.1.0"
authors = [ authors = [
"Antoine Cezar <antoine.cezar@octobus.net>", "Antoine Cezar <antoine.cezar@octobus.net>",
"Raphaël Gomès <raphael.gomes@octobus.net>", "Raphaël Gomès <raphael.gomes@octobus.net>",
] ]
edition = "2018" edition = "2018"
[dependencies] [dependencies]
hg-core = { path = "../hg-core"} hg-core = { path = "../hg-core"}
chrono = "0.4.19" chrono = "0.4.19"
clap = "2.33.1" clap = "2.33.1"
derive_more = "0.99" derive_more = "0.99"
home = "0.5.3"
AlphareUnsubmitted
Not Done

I forgot when first reading this patch, but we usually give a reason for adding a new dependency in the commit message.

Alphare: I forgot when first reading this patch, but we usually give a reason for adding a new…
pulkitAuthorUnsubmitted
Done

Will add.

pulkit: Will add.
lazy_static = "1.4.0" lazy_static = "1.4.0"
log = "0.4.11" log = "0.4.11"
micro-timer = "0.3.1" micro-timer = "0.3.1"
regex = "1.3.9" regex = "1.3.9"
env_logger = "0.7.1" env_logger = "0.7.1"
format-bytes = "0.2.1" format-bytes = "0.2.1"
users = "0.11.0" users = "0.11.0"

rust/rhg/src/main.rs

extern crate log; extern crate log;
use crate::ui::Ui; use crate::ui::Ui;
use clap::App; use clap::App;
use clap::AppSettings; use clap::AppSettings;
use clap::Arg; use clap::Arg;
use clap::ArgMatches; use clap::ArgMatches;
use format_bytes::{format_bytes, join}; use format_bytes::{format_bytes, join};
use hg::config::Config; use hg::config::{Config, ConfigSource};
use hg::repo::{Repo, RepoError}; use hg::repo::{Repo, RepoError};
use hg::utils::files::{get_bytes_from_os_str, get_path_from_bytes}; use hg::utils::files::{get_bytes_from_os_str, get_path_from_bytes};
use hg::utils::SliceExt; use hg::utils::SliceExt;
use std::ffi::OsString; use std::ffi::OsString;
use std::path::PathBuf; use std::path::PathBuf;
use std::process::Command; use std::process::Command;
mod blackbox; mod blackbox;
// TODO: show a warning or combine with original error if // TODO: show a warning or combine with original error if
// `get_bool` returns an error // `get_bool` returns an error
non_repo_config non_repo_config
.get_bool(b"ui", b"detailed-exit-code") .get_bool(b"ui", b"detailed-exit-code")
.unwrap_or(false), .unwrap_or(false),
) )
} }
} }
let repo_path = early_args.repo.as_deref().map(get_path_from_bytes); let repo_arg = early_args.repo.unwrap_or(Vec::new());
let repo_result = match Repo::find(&non_repo_config, repo_path) { let repo_path: Option<PathBuf> = {
if repo_arg.is_empty() {
AlphareUnsubmitted
Done

Use repo_arg.is_empty() instead

Alphare: Use `repo_arg.is_empty()` instead
None
} else {
let local_config = {
AlphareUnsubmitted
Done

The nested ifs can be replaced by:

match config_val {
    Ok(Some(val)) if val.len() > 0 => Some(get_path_from_bytes(val.as_bytes())),
    _ => Some(get_path_from_bytes(&repo_arg)),
}

Also, you're using get_str, when you should probably just be using get, which already returns bytes (and does not unnecessarily force UTF8). That would remove the need for as_bytes() in the above suggested code.

Alphare: The nested ifs can be replaced by: ``` match config_val { Ok(Some(val)) if val.len() > 0 =>…
if std::env::var_os("HGRCSKIPREPO").is_none() {
let current_dir = hg::utils::current_dir();
// TODO: handle errors from current_dir
AlphareUnsubmitted
Done

if current_dir.is_err() is simpler. However, you're using current_dir.unwrap() right after, so in the end you could simply do:

if let Ok(current_dir_path) = current_dir {
    ...
} else {
    None
}
Alphare: `if current_dir.is_err()` is simpler. However, you're using `current_dir.unwrap()` right after…
if let Ok(current_dir_path) = current_dir {
let config_files = vec![
ConfigSource::AbsPath(
current_dir_path.join(".hg/hgrc"),
),
ConfigSource::AbsPath(
current_dir_path.join(".hg/hgrc-not-shared"),
),
];
SimonSapinUnsubmitted
Not Done

.ok() here together with the handling of None later silently ignore config parse errors, where Python-based hg aborts. However:

  • This is a rather niche corner case
  • Handling such errors in this particular function is tricky because we can’t just use the ? operator
  • This patch still improves things already, and has been open long enough

So please add a // TODO: handle errors from load_from_explicit_sources` comment and let’s follow up later.

Similarly please add a TODO comment above for handling errors from current_dir

SimonSapin: `.ok()` here together with the handling of `None` later silently ignore config parse errors…
// TODO: handle errors from
// `load_from_explicit_sources`
Config::load_from_explicit_sources(config_files).ok()
} else {
None
AlphareUnsubmitted
Not Done

You could use config.ok() here.

Alphare: You could use `config.ok()` here.
pulkitAuthorUnsubmitted
Done

You mean if config.ok()?

pulkit: You mean `if config.ok()`?
AlphareUnsubmitted
Not Done

No, I'm referencing https://doc.rust-lang.org/std/result/enum.Result.html#method.ok, which could replace the entire if expression

Alphare: No, I'm referencing https://doc.rust-lang.org/std/result/enum.Result.html#method.ok, which…
}
} else {
None
}
};
let non_repo_config_val = {
let non_repo_val = non_repo_config.get(b"paths", &repo_arg);
match &non_repo_val {
Some(val) if val.len() > 0 => home::home_dir()
.unwrap_or_else(|| PathBuf::from("~"))
AlphareUnsubmitted
Not Done

I'm assuming this means that we silently ignore this error in the Python implementation as well?

Alphare: I'm assuming this means that we silently ignore this error in the Python implementation as well?
pulkitAuthorUnsubmitted
Done

Not sure what exact part in python will mean same as this one. But I see python code will raise IOError if it fails to read config from a file path passed.

pulkit: Not sure what exact part in python will mean same as this one. But I see python code will raise…
AlphareUnsubmitted
Not Done

This should probably do the same thing then.

Alphare: This should probably do the same thing then.
.join(get_path_from_bytes(val))
.canonicalize()
// TODO: handle error and make it similar to python
// implementation maybe?
.ok(),
_ => None,
}
};
AlphareUnsubmitted
Not Done

Why do we unwrap here?

Alphare: Why do we unwrap here?
pulkitAuthorUnsubmitted
Done

.canonicalize() returns a Result.

pulkit: `.canonicalize()` returns a `Result`.
AlphareUnsubmitted
Not Done

Well sure, but what happens if it's an Err? Mercurial will just panic and throw a nasty traceback in Python context, and a Rust panic message in Rust ones.

Alphare: Well sure, but what happens if it's an `Err`? Mercurial will just panic and throw a nasty…
let config_val = match &local_config {
None => non_repo_config_val,
Some(val) => {
let local_config_val = val.get(b"paths", &repo_arg);
match &local_config_val {
Some(val) if val.len() > 0 => {
// presence of a local_config assures that
// current_dir
// wont result in an Error
AlphareUnsubmitted
Not Done

This is canpath.unwrap_or(non_repo_config_val)

Alphare: This is `canpath.unwrap_or(non_repo_config_val)`
pulkitAuthorUnsubmitted
Done

but we are puting canpath.unwrap in Some() again, will it work?

pulkit: but we are puting `canpath.unwrap` in `Some()` again, will it work?
AlphareUnsubmitted
Not Done

Ah sorry, I misread. This is canpath.ok().or(non_repo_config_val). But you can use a match expression if you find it clearer also. unwrap raises an immediate alarm of "this can panic if not handled correctly or if the code changes in the future", so we try to avoid doing it in favor of letting the type system check all code paths for us.

Alphare: Ah sorry, I misread. This is `canpath.ok().or(non_repo_config_val)`. But you can use a `match`…
let canpath = hg::utils::current_dir()
.unwrap()
.join(get_path_from_bytes(val))
.canonicalize();
canpath.ok().or(non_repo_config_val)
}
_ => non_repo_config_val,
}
}
AlphareUnsubmitted
Not Done

Same question as above

Alphare: Same question as above
};
config_val.or(Some(get_path_from_bytes(&repo_arg).to_path_buf()))
}
};
let repo_result = match Repo::find(&non_repo_config, repo_path.to_owned())
AlphareUnsubmitted
Done

Could use config_val.or(Some(get_path_from_bytes(&repo_arg).to_path_buf()))

Alphare: Could use `config_val.or(Some(get_path_from_bytes(&repo_arg).to_path_buf()))`
{
Ok(repo) => Ok(repo), Ok(repo) => Ok(repo),
Err(RepoError::NotFound { at }) if repo_path.is_none() => { Err(RepoError::NotFound { at }) if repo_path.is_none() => {
// Not finding a repo is not fatal yet, if `-R` was not given // Not finding a repo is not fatal yet, if `-R` was not given
Err(NoRepoInCwdError { cwd: at }) Err(NoRepoInCwdError { cwd: at })
} }
Err(error) => exit( Err(error) => exit(
&initial_current_dir, &initial_current_dir,
&ui, &ui,

tests/test-globalopts.t

$ hg -R file:a identify $ hg -R file:a identify
8580ff50825a tip 8580ff50825a tip
$ hg -R file://localhost/`pwd`/a/ identify $ hg -R file://localhost/`pwd`/a/ identify
8580ff50825a tip 8580ff50825a tip
-R with path aliases: -R with path aliases:
TODO: add rhg support for path aliases
#if no-rhg
$ cd c $ cd c
$ hg -R default identify $ hg -R default identify
8580ff50825a tip 8580ff50825a tip
$ hg -R relative identify $ hg -R relative identify
8580ff50825a tip 8580ff50825a tip
$ echo '[paths]' >> $HGRCPATH $ echo '[paths]' >> $HGRCPATH
$ echo 'relativetohome = a' >> $HGRCPATH $ echo 'relativetohome = a' >> $HGRCPATH
$ hg path | grep relativetohome $ hg path | grep relativetohome
relativetohome = $TESTTMP/a relativetohome = $TESTTMP/a
$ HOME=`pwd`/../ hg path | grep relativetohome $ HOME=`pwd`/../ hg path | grep relativetohome
relativetohome = $TESTTMP/a relativetohome = $TESTTMP/a
$ HOME=`pwd`/../ hg -R relativetohome identify $ HOME=`pwd`/../ hg -R relativetohome identify
8580ff50825a tip 8580ff50825a tip
SimonSapinUnsubmitted
Not Done

This tests suggests that relative paths in [paths] are resolved relative to the user’s $HOME directory, not relative to the the config file nor the processes current directory. To be honest I find this really weird and unexpected, but it does appear to be the case when I change HOME=/somethingelse in the test and I do see root = os.path.expanduser(b'~') in mercurial/ui.py before _applyconfig calls fixconfig which looks at [paths] items and makes them absolute.

When run with --rhg this test ends up passing but not quite for the right reason. rhg resolves path a relative to its current directory which is the repository c. That repository happens to contains a file named a, so Repo::find returns HgError::unsupported("bundle repository") which causes fallback to Python. Fallback is what we want, but only because the identify command is not implemented (yet).

It looks like os.path.expanduser returns its input unchanged when getting the home directory fails, so the code above should be something like Some(home::home_dir().unwrap_or_else(|| PathBuf::new("~")).join(get_path_from_bytes(val)))

SimonSapin: This tests suggests that relative paths in `[paths]` are resolved relative to the user’s…
pulkitAuthorUnsubmitted
Done

Ugh!

Btw, how did you figured out that we are falling back with HgError:unsupported("bundle ...")? I mean is there some config or something which shows these debug prints or is there any other way?

I updated the patch after fighting with borrow checker a bit, does seems bit ugly to me, kindly have a look.

pulkit: Ugh! Btw, how did you figured out that we are falling back with `HgError:unsupported("bundle ..
$ cd .. $ cd ..
#endif
#if no-outer-repo #if no-outer-repo
Implicit -R: Implicit -R:
$ hg ann a/a $ hg ann a/a
0: a 0: a
$ hg ann a/a a/a $ hg ann a/a a/a