This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10094

rhg: Make fallback to Python the default behavior
AbandonedPublic

Authored by SimonSapin on Mar 3 2021, 12:08 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

SimonSapin created this revision.Mar 3 2021, 12:08 PM
SimonSapin updated this revision to Diff 26050.Mar 4 2021, 6:10 AM
baymax updated this revision to Diff 26068.Mar 4 2021, 8:00 AM

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

Alphare accepted this revision.Mar 5 2021, 3:42 AM
SimonSapin updated this revision to Diff 26167.Mar 9 2021, 4:39 AM
marmoute added a subscriber: marmoute.Mar 9 2021, 12:57 PM

Why do we want it the default behavior in general instead of the default behavior just during the test ?

My first though is that it seems safer to keep the explicit abort at this stage of the project. Whats your reasoning to flipping the switch now?

SimonSapin added a comment.Mar 11 2021, 3:50 AM

My reasoning is that we don’t want to push onto users the burden of keeping track of what functionality is supported natively by rhg or not, especially since we want that set to grow over time. Enabling fallback and using rhg as a drop-in replacement for hg that behaves the same (modulo speed and possible bugs) seems to me like the only scenario where rhg is really useful. The "abort with an error message" exists for tests where we want to assert that something is supported natively. The "abort silently" mode was initially designed to let a wrapper script implement fallback, but there’s no point "leaving that as an exercise to readers" if rhg can do it itself.

Alphare added a comment.Mar 11 2021, 4:02 AM

Just my two cents:

The "abort silently" mode was initially designed to let a wrapper script implement fallback, but there’s no point "leaving that as an exercise to readers" if rhg can do it itself.

Having a built-in fallback is good for ease of use, but we should also keep the external fallback with 252 exit code (not that you've ever said the contrary, just making sure).

Now that rhg is testable through (almost) the entire CI, I think we should encourage the default value to be something usable for users, I agree with Simon.

marmoute added a comment.Mar 11 2021, 4:12 AM

But as far as I understand, the default fallback value is the standard hg executable which is arbitrary and dangerous. For example if a user want to test the behavior of the new mercurial compares to the older one and directly call /some/path/rust/target/release/rhg, it will silently fall back to the system hg which is a pretty surprising behavior.

SimonSapin abandoned this revision.Mar 12 2021, 5:15 PM

Marmoute makes a good point about the default to looking up hg in $PATH being fragile, making it easy to accidentally use not the expected installation of Mercurial.

This is replaced by https://phab.mercurial-scm.org/D10187 and https://phab.mercurial-scm.org/D10189

Revision Contents
Changeset List

PathPackages
Mrust/rhg/src/main.rs (32 lines)
Mtests/test-rhg.t (95 lines)

Diff 26167

rust/rhg/src/main.rs

Abort, Abort,
/// Silently exit with code 252. /// Silently exit with code 252.
AbortSilent, AbortSilent,
/// Try running a Python implementation /// Try running a Python implementation
Fallback { executable: Vec<u8> }, Fallback { executable: Vec<u8> },
} }
impl OnUnsupported { impl OnUnsupported {
const DEFAULT: Self = OnUnsupported::Abort;
const DEFAULT_FALLBACK_EXECUTABLE: &'static [u8] = b"hg"; const DEFAULT_FALLBACK_EXECUTABLE: &'static [u8] = b"hg";
fn from_config(config: &Config) -> Self { fn from_config(config: &Config) -> Self {
match config let fallback = b"fallback";
match &*config
.get(b"rhg", b"on-unsupported") .get(b"rhg", b"on-unsupported")
.map(|value| value.to_ascii_lowercase()) .unwrap_or(fallback)
.as_deref() .to_ascii_lowercase()
{ {
Some(b"abort") => OnUnsupported::Abort, b"abort" => OnUnsupported::Abort,
Some(b"abort-silent") => OnUnsupported::AbortSilent, b"abort-silent" => OnUnsupported::AbortSilent,
Some(b"fallback") => OnUnsupported::Fallback { value => {
if value != fallback {
// TODO: warn about unknown config value
}
OnUnsupported::Fallback {
executable: config executable: config
.get(b"rhg", b"fallback-executable") .get(b"rhg", b"fallback-executable")
.unwrap_or(Self::DEFAULT_FALLBACK_EXECUTABLE) .unwrap_or(Self::DEFAULT_FALLBACK_EXECUTABLE)
.to_owned(), .to_owned(),
}, }
None => Self::DEFAULT,
Some(_) => {
// TODO: warn about unknown config value
Self::DEFAULT
} }
} }
} }
} }

tests/test-rhg.t

#require rust #require rust
Define an rhg function that will only run if rhg exists Define an rhg function that will only run if rhg exists
$ RHG="$RUNTESTDIR/../rust/target/release/rhg" $ RHG="$RUNTESTDIR/../rust/target/release/rhg"
$ rhg() { $ rhg() {
> if [ -f "$RHG" ]; then > if [ -f "$RHG" ]; then
> "$RHG" "$@" > "$RHG" "$@"
> else > else
> echo "skipped: Cannot find rhg. Try to run cargo build in rust/rhg." > echo "skipped: Cannot find rhg. Try to run cargo build in rust/rhg."
> exit 80 > exit 80
> fi > fi
> } > }
$ NO_FALLBACK="--config=rhg.on-unsupported=abort"
Unimplemented command Unimplemented command
$ rhg unimplemented-command $ rhg $NO_FALLBACK unimplemented-command
unsupported feature: error: Found argument 'unimplemented-command' which wasn't expected, or isn't valid in this context unsupported feature: error: Found argument 'unimplemented-command' which wasn't expected, or isn't valid in this context
USAGE: USAGE:
rhg [OPTIONS] <SUBCOMMAND> rhg --config <CONFIG>... <SUBCOMMAND>
For more information try --help For more information try --help
[252] [252]
$ rhg unimplemented-command --config rhg.on-unsupported=abort-silent $ rhg unimplemented-command --config rhg.on-unsupported=abort-silent
[252] [252]
Finding root Finding root
$ rhg root $ rhg $NO_FALLBACK root
abort: no repository found in '$TESTTMP' (.hg not found)! abort: no repository found in '$TESTTMP' (.hg not found)!
[255] [255]
$ hg init repository $ hg init repository
$ cd repository $ cd repository
$ rhg root $ rhg $NO_FALLBACK root
$TESTTMP/repository $TESTTMP/repository
Reading and setting configuration Reading and setting configuration
$ echo "[ui]" >> $HGRCPATH $ echo "[ui]" >> $HGRCPATH
$ echo "username = user1" >> $HGRCPATH $ echo "username = user1" >> $HGRCPATH
$ rhg config ui.username $ rhg $NO_FALLBACK config ui.username
user1 user1
$ echo "[ui]" >> .hg/hgrc $ echo "[ui]" >> .hg/hgrc
$ echo "username = user2" >> .hg/hgrc $ echo "username = user2" >> .hg/hgrc
$ rhg config ui.username $ rhg $NO_FALLBACK config ui.username
user2 user2
$ rhg --config ui.username=user3 config ui.username $ rhg $NO_FALLBACK --config ui.username=user3 config ui.username
user3 user3
Unwritable file descriptor Unwritable file descriptor
$ rhg root > /dev/full $ rhg $NO_FALLBACK root > /dev/full
abort: No space left on device (os error 28) abort: No space left on device (os error 28)
[255] [255]
Deleted repository Deleted repository
$ rm -rf `pwd` $ rm -rf `pwd`
$ rhg root $ rhg $NO_FALLBACK root
abort: $ENOENT$: current directory abort: $ENOENT$: current directory
[255] [255]
Listing tracked files Listing tracked files
$ cd $TESTTMP $ cd $TESTTMP
$ hg init repository $ hg init repository
$ cd repository $ cd repository
$ for i in 1 2 3; do $ for i in 1 2 3; do
> echo $i >> file$i > echo $i >> file$i
> hg add file$i > hg add file$i
> done > done
> hg commit -m "commit $i" -q > hg commit -m "commit $i" -q
Listing tracked files from root Listing tracked files from root
$ rhg files $ rhg $NO_FALLBACK files
file1 file1
file2 file2
file3 file3
Listing tracked files from subdirectory Listing tracked files from subdirectory
$ mkdir -p path/to/directory $ mkdir -p path/to/directory
$ cd path/to/directory $ cd path/to/directory
$ rhg files $ rhg $NO_FALLBACK files
../../../file1 ../../../file1
../../../file2 ../../../file2
../../../file3 ../../../file3
Listing tracked files through broken pipe Listing tracked files through broken pipe
$ rhg files | head -n 1 $ rhg $NO_FALLBACK files | head -n 1
../../../file1 ../../../file1
Debuging data in inline index Debuging data in inline index
$ cd $TESTTMP $ cd $TESTTMP
$ rm -rf repository $ rm -rf repository
$ hg init repository $ hg init repository
$ cd repository $ cd repository
$ for i in 1 2 3 4 5 6; do $ for i in 1 2 3 4 5 6; do
> echo $i >> file-$i > echo $i >> file-$i
> hg add file-$i > hg add file-$i
> hg commit -m "Commit $i" -q > hg commit -m "Commit $i" -q
> done > done
$ rhg debugdata -c 2 $ rhg $NO_FALLBACK debugdata -c 2
8d0267cb034247ebfa5ee58ce59e22e57a492297 8d0267cb034247ebfa5ee58ce59e22e57a492297
test test
0 0 0 0
file-3 file-3
Commit 3 (no-eol) Commit 3 (no-eol)
$ rhg debugdata -m 2 $ rhg $NO_FALLBACK debugdata -m 2
file-1\x00b8e02f6433738021a065f94175c7cd23db5f05be (esc) file-1\x00b8e02f6433738021a065f94175c7cd23db5f05be (esc)
file-2\x005d9299349fc01ddd25d0070d149b124d8f10411e (esc) file-2\x005d9299349fc01ddd25d0070d149b124d8f10411e (esc)
file-3\x002661d26c649684b482d10f91960cc3db683c38b4 (esc) file-3\x002661d26c649684b482d10f91960cc3db683c38b4 (esc)
Debuging with full node id Debuging with full node id
$ rhg debugdata -c `hg log -r 0 -T '{node}'` $ rhg $NO_FALLBACK debugdata -c `hg log -r 0 -T '{node}'`
d1d1c679d3053e8926061b6f45ca52009f011e3f d1d1c679d3053e8926061b6f45ca52009f011e3f
test test
0 0 0 0
file-1 file-1
Commit 1 (no-eol) Commit 1 (no-eol)
Specifying revisions by changeset ID Specifying revisions by changeset ID
$ hg log -T '{node}\n' $ hg log -T '{node}\n'
c6ad58c44207b6ff8a4fbbca7045a5edaa7e908b c6ad58c44207b6ff8a4fbbca7045a5edaa7e908b
d654274993d0149eecc3cc03214f598320211900 d654274993d0149eecc3cc03214f598320211900
f646af7e96481d3a5470b695cf30ad8e3ab6c575 f646af7e96481d3a5470b695cf30ad8e3ab6c575
cf8b83f14ead62b374b6e91a0e9303b85dfd9ed7 cf8b83f14ead62b374b6e91a0e9303b85dfd9ed7
91c6f6e73e39318534dc415ea4e8a09c99cd74d6 91c6f6e73e39318534dc415ea4e8a09c99cd74d6
6ae9681c6d30389694d8701faf24b583cf3ccafe 6ae9681c6d30389694d8701faf24b583cf3ccafe
$ rhg files -r cf8b83 $ rhg $NO_FALLBACK files -r cf8b83
file-1 file-1
file-2 file-2
file-3 file-3
$ rhg cat -r cf8b83 file-2 $ rhg $NO_FALLBACK cat -r cf8b83 file-2
2 2
$ rhg cat -r c file-2 $ rhg $NO_FALLBACK cat -r c file-2
abort: ambiguous revision identifier c abort: ambiguous revision identifier c
[255] [255]
$ rhg cat -r d file-2 $ rhg $NO_FALLBACK cat -r d file-2
2 2
Cat files Cat files
$ cd $TESTTMP $ cd $TESTTMP
$ rm -rf repository $ rm -rf repository
$ hg init repository $ hg init repository
$ cd repository $ cd repository
$ echo "original content" > original $ echo "original content" > original
$ hg add original $ hg add original
$ hg commit -m "add original" original $ hg commit -m "add original" original
$ rhg cat -r 0 original $ rhg $NO_FALLBACK cat -r 0 original
original content original content
Cat copied file should not display copy metadata Cat copied file should not display copy metadata
$ hg copy original copy_of_original $ hg copy original copy_of_original
$ hg commit -m "add copy of original" $ hg commit -m "add copy of original"
$ rhg cat -r 1 copy_of_original $ rhg $NO_FALLBACK cat -r 1 copy_of_original
original content original content
Fallback to Python Fallback to Python
$ rhg cat original $ rhg $NO_FALLBACK cat original
unsupported feature: `rhg cat` without `--rev` / `-r` unsupported feature: `rhg cat` without `--rev` / `-r`
[252] [252]
$ FALLBACK="--config rhg.on-unsupported=fallback" $ rhg cat original
$ rhg cat original $FALLBACK
original content original content
$ rhg cat original $FALLBACK --config rhg.fallback-executable=false $ rhg cat original --config rhg.fallback-executable=false
[1] [1]
$ rhg cat original $FALLBACK --config rhg.fallback-executable=hg-non-existent $ rhg cat original --config rhg.fallback-executable=hg-non-existent
tried to fall back to a 'hg-non-existent' sub-process but got error $ENOENT$ tried to fall back to a 'hg-non-existent' sub-process but got error $ENOENT$
unsupported feature: `rhg cat` without `--rev` / `-r` unsupported feature: `rhg cat` without `--rev` / `-r`
[252] [252]
$ rhg cat original $FALLBACK --config rhg.fallback-executable="$RHG" $ rhg cat original --config rhg.fallback-executable="$RHG"
Blocking recursive fallback. The 'rhg.fallback-executable = */rust/target/release/rhg' config points to `rhg` itself. (glob) Blocking recursive fallback. The 'rhg.fallback-executable = */rust/target/release/rhg' config points to `rhg` itself. (glob)
unsupported feature: `rhg cat` without `--rev` / `-r` unsupported feature: `rhg cat` without `--rev` / `-r`
[252] [252]
Requirements Requirements
$ rhg debugrequirements $ rhg $NO_FALLBACK debugrequirements
dotencode dotencode
fncache fncache
generaldelta generaldelta
revlogv1 revlogv1
sparserevlog sparserevlog
store store
$ echo indoor-pool >> .hg/requires $ echo indoor-pool >> .hg/requires
$ rhg files $ rhg $NO_FALLBACK files
unsupported feature: repository requires feature unknown to this Mercurial: indoor-pool unsupported feature: repository requires feature unknown to this Mercurial: indoor-pool
[252] [252]
$ rhg cat -r 1 copy_of_original $ rhg $NO_FALLBACK cat -r 1 copy_of_original
unsupported feature: repository requires feature unknown to this Mercurial: indoor-pool unsupported feature: repository requires feature unknown to this Mercurial: indoor-pool
[252] [252]
$ rhg debugrequirements $ rhg $NO_FALLBACK debugrequirements
unsupported feature: repository requires feature unknown to this Mercurial: indoor-pool unsupported feature: repository requires feature unknown to this Mercurial: indoor-pool
[252] [252]
$ echo -e '\xFF' >> .hg/requires $ echo -e '\xFF' >> .hg/requires
$ rhg debugrequirements $ rhg $NO_FALLBACK debugrequirements
abort: corrupted repository: parse error in 'requires' file abort: corrupted repository: parse error in 'requires' file
[255] [255]
Persistent nodemap Persistent nodemap
$ cd $TESTTMP $ cd $TESTTMP
$ rm -rf repository $ rm -rf repository
$ hg init repository $ hg init repository
$ cd repository $ cd repository
$ rhg debugrequirements | grep nodemap $ rhg $NO_FALLBACK debugrequirements | grep nodemap
[1] [1]
$ hg debugbuilddag .+5000 --overwritten-file --config "storage.revlog.nodemap.mode=warn" $ hg debugbuilddag .+5000 --overwritten-file --config "storage.revlog.nodemap.mode=warn"
$ hg id -r tip $ hg id -r tip
c3ae8dec9fad tip c3ae8dec9fad tip
$ ls .hg/store/00changelog* $ ls .hg/store/00changelog*
.hg/store/00changelog.d .hg/store/00changelog.d
.hg/store/00changelog.i .hg/store/00changelog.i
$ rhg files -r c3ae8dec9fad $ rhg $NO_FALLBACK files -r c3ae8dec9fad
of of
$ cd $TESTTMP $ cd $TESTTMP
$ rm -rf repository $ rm -rf repository
$ hg --config format.use-persistent-nodemap=True init repository $ hg --config format.use-persistent-nodemap=True init repository
$ cd repository $ cd repository
$ rhg debugrequirements | grep nodemap $ rhg $NO_FALLBACK debugrequirements | grep nodemap
persistent-nodemap persistent-nodemap
$ hg debugbuilddag .+5000 --overwritten-file --config "storage.revlog.nodemap.mode=warn" $ hg debugbuilddag .+5000 --overwritten-file --config "storage.revlog.nodemap.mode=warn"
$ hg id -r tip $ hg id -r tip
c3ae8dec9fad tip c3ae8dec9fad tip
$ ls .hg/store/00changelog* $ ls .hg/store/00changelog*
.hg/store/00changelog-*.nd (glob) .hg/store/00changelog-*.nd (glob)
.hg/store/00changelog.d .hg/store/00changelog.d
.hg/store/00changelog.i .hg/store/00changelog.i
.hg/store/00changelog.n .hg/store/00changelog.n
Specifying revisions by changeset ID Specifying revisions by changeset ID
$ rhg files -r c3ae8dec9fad $ rhg $NO_FALLBACK files -r c3ae8dec9fad
of of
$ rhg cat -r c3ae8dec9fad of $ rhg $NO_FALLBACK cat -r c3ae8dec9fad of
r5000 r5000
Crate a shared repository Crate a shared repository
$ echo "[extensions]" >> $HGRCPATH $ echo "[extensions]" >> $HGRCPATH
$ echo "share = " >> $HGRCPATH $ echo "share = " >> $HGRCPATH
$ cd $TESTTMP $ cd $TESTTMP
$ hg init repo1 $ hg init repo1
$ echo a > repo1/a $ echo a > repo1/a
$ hg -R repo1 commit -A -m'init' $ hg -R repo1 commit -A -m'init'
adding a adding a
$ hg share repo1 repo2 $ hg share repo1 repo2
updating working directory updating working directory
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
And check that basic rhg commands work with sharing And check that basic rhg $NO_FALLBACK commands work with sharing
$ rhg files -R repo2 $ rhg $NO_FALLBACK files -R repo2
repo2/a repo2/a
$ rhg -R repo2 cat -r 0 repo2/a $ rhg $NO_FALLBACK -R repo2 cat -r 0 repo2/a
a a
Same with relative sharing Same with relative sharing
$ hg share repo2 repo3 --relative $ hg share repo2 repo3 --relative
updating working directory updating working directory
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ rhg files -R repo3 $ rhg $NO_FALLBACK files -R repo3
repo3/a repo3/a
$ rhg -R repo3 cat -r 0 repo3/a $ rhg $NO_FALLBACK -R repo3 cat -r 0 repo3/a
a a
Same with share-safe Same with share-safe
$ echo "[format]" >> $HGRCPATH $ echo "[format]" >> $HGRCPATH
$ echo "use-share-safe = True" >> $HGRCPATH $ echo "use-share-safe = True" >> $HGRCPATH
$ cd $TESTTMP $ cd $TESTTMP
$ hg init repo4 $ hg init repo4
$ cd repo4 $ cd repo4
$ echo a > a $ echo a > a
$ hg commit -A -m'init' $ hg commit -A -m'init'
adding a adding a
$ cd .. $ cd ..
$ hg share repo4 repo5 $ hg share repo4 repo5
updating working directory updating working directory
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
And check that basic rhg commands work with sharing And check that basic rhg commands work with sharing
$ cd repo5 $ cd repo5
$ rhg files $ rhg $NO_FALLBACK files
a a
$ rhg cat -r 0 a $ rhg $NO_FALLBACK cat -r 0 a
a a
The blackbox extension is supported The blackbox extension is supported
$ echo "[extensions]" >> $HGRCPATH $ echo "[extensions]" >> $HGRCPATH
$ echo "blackbox =" >> $HGRCPATH $ echo "blackbox =" >> $HGRCPATH
$ echo "[blackbox]" >> $HGRCPATH $ echo "[blackbox]" >> $HGRCPATH
$ echo "maxsize = 1" >> $HGRCPATH $ echo "maxsize = 1" >> $HGRCPATH
$ rhg files > /dev/null $ rhg $NO_FALLBACK files > /dev/null
$ cat .hg/blackbox.log $ cat .hg/blackbox.log
????/??/?? ??:??:??.??? * @d3873e73d99ef67873dac33fbcc66268d5d2b6f4 (*)> (rust) files exited 0 after 0.??? seconds (glob) ????/??/?? ??:??:??.??? * @d3873e73d99ef67873dac33fbcc66268d5d2b6f4 (*)> (rust) '--config=rhg.on-unsupported=abort' files exited 0 after 0.??? seconds (glob)
$ cat .hg/blackbox.log.1 $ cat .hg/blackbox.log.1
????/??/?? ??:??:??.??? * @d3873e73d99ef67873dac33fbcc66268d5d2b6f4 (*)> (rust) files (glob) ????/??/?? ??:??:??.??? * @d3873e73d99ef67873dac33fbcc66268d5d2b6f4 (*)> (rust) '--config=rhg.on-unsupported=abort' files (glob)