This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11377

rhg: add support for calling `rhg cat` without a revision
ClosedPublic

Authored by Alphare on Sep 1 2021, 12:42 PM.

Details

Reviewers
SimonSapin
Group Reviewers
hg-reviewers
Commits
rHGd919b0ca8449: rhg: add support for calling `rhg cat` without a revision
Summary

Turns out the necessary pieces were there already.
Like the Python implementation, we default to the first parent of the dirstate.

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

Alphare created this revision.Sep 1 2021, 12:42 PM
Alphare added a comment.Sep 1 2021, 12:44 PM

Note to reviewers: this needs the other patches in the stack to pass all tests, but in order to make the tests in the patches actually meaningful, this had to go first.

Alphare added a comment.Sep 2 2021, 3:52 AM

https://foss.heptapod.net/octobus/mercurial-devel/-/jobs/243241 Successful CI run (since baymax does not need to refresh this stack)

SimonSapin added a subscriber: SimonSapin.Sep 2 2021, 2:29 PM
SimonSapin added inline comments.
rust/rhg/src/commands/cat.rs
63

What should happen when p1 is null? (Such as in a new empty repository.) Is this case covered by tests?

63

Making a hex string just to parse it again feels a bit silly, but is likely required by the current signature of hg::operations::cat. That can be refactored later.

Alphare added inline comments.Sep 8 2021, 6:27 AM
rust/rhg/src/commands/cat.rs
63

What should happen when p1 is null? (Such as in a new empty repository.) Is this case covered by tests?

If p1 is nullrev, it's the same path as any other revision, but if the dirstate does not exist, then it's an abort in the same way as hg does it. Should I add a testcase in test-cat.t or somewhere similar?

SimonSapin accepted this revision.Sep 9 2021, 9:41 AM
SimonSapin added inline comments.
rust/rhg/src/commands/cat.rs
63

I guess I should have just tried it:

$ cd /tmp
$ hg init repo
$ cd repo
$ hg cat a
a: no such file in rev 000000000000

Indeed that look like the same code path. Still, yes adding that test sounds good.

Alphare added a commit: rHGd919b0ca8449: rhg: add support for calling `rhg cat` without a revision.Oct 1 2021, 9:21 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGd919b0ca8449: rhg: add support for calling `rhg cat` without a revision (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/rhg/src/commands/cat.rs (43 lines)
Mtests/test-rhg.t (43 lines)

Diff 30488

rust/rhg/src/commands/cat.rs

let stripped = normalized let stripped = normalized
.strip_prefix(&working_directory) .strip_prefix(&working_directory)
// TODO: error message for path arguments outside of the repo // TODO: error message for path arguments outside of the repo
.map_err(|_| CommandError::abort(""))?; .map_err(|_| CommandError::abort(""))?;
let hg_file = HgPathBuf::try_from(stripped.to_path_buf()) let hg_file = HgPathBuf::try_from(stripped.to_path_buf())
.map_err(|e| CommandError::abort(e.to_string()))?; .map_err(|e| CommandError::abort(e.to_string()))?;
files.push(hg_file); files.push(hg_file);
} }
// TODO probably move this to a util function like `repo.default_rev` or
// something when it's used somewhere else
let rev = match rev {
Some(r) => r.to_string(),
None => format!("{:x}", repo.dirstate_parents()?.p1),
SimonSapinUnsubmitted
Not Done

What should happen when p1 is null? (Such as in a new empty repository.) Is this case covered by tests?

SimonSapin: What should happen when p1 is null? (Such as in a new empty repository.) Is this case covered…
AlphareAuthorUnsubmitted
Done

What should happen when p1 is null? (Such as in a new empty repository.) Is this case covered by tests?

If p1 is nullrev, it's the same path as any other revision, but if the dirstate does not exist, then it's an abort in the same way as hg does it. Should I add a testcase in test-cat.t or somewhere similar?

Alphare: > What should happen when p1 is null? (Such as in a new empty repository.) Is this case covered…
SimonSapinUnsubmitted
Not Done

I guess I should have just tried it:

$ cd /tmp
$ hg init repo
$ cd repo
$ hg cat a
a: no such file in rev 000000000000

Indeed that look like the same code path. Still, yes adding that test sounds good.

SimonSapin: I guess I should have just tried it: ``` $ cd /tmp $ hg init repo $ cd repo $ hg cat a a: no…
SimonSapinUnsubmitted
Not Done

Making a hex string just to parse it again feels a bit silly, but is likely required by the current signature of hg::operations::cat. That can be refactored later.

SimonSapin: Making a hex string just to parse it again feels a bit silly, but is likely required by the…
};
match rev { let output = cat(&repo, &rev, &files).map_err(|e| (e, rev.as_str()))?;
Some(rev) => {
let output = cat(&repo, rev, &files).map_err(|e| (e, rev))?;
invocation.ui.write_stdout(&output.concatenated)?; invocation.ui.write_stdout(&output.concatenated)?;
if !output.missing.is_empty() { if !output.missing.is_empty() {
let short = format!("{:x}", output.node.short()).into_bytes(); let short = format!("{:x}", output.node.short()).into_bytes();
for path in &output.missing { for path in &output.missing {
invocation.ui.write_stderr(&format_bytes!( invocation.ui.write_stderr(&format_bytes!(
b"{}: no such file in rev {}\n", b"{}: no such file in rev {}\n",
path.as_bytes(), path.as_bytes(),
short short
))?; ))?;
} }
} }
if output.found_any { if output.found_any {
Ok(()) Ok(())
} else { } else {
Err(CommandError::Unsuccessful) Err(CommandError::Unsuccessful)
} }
} }
None => Err(CommandError::unsupported(
"`rhg cat` without `--rev` / `-r`",
)),
}
}

tests/test-rhg.t

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
Without `--rev`
$ $NO_FALLBACK rhg cat original
original content
With `--rev`
$ $NO_FALLBACK rhg cat -r 0 original $ $NO_FALLBACK rhg 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"
$ $NO_FALLBACK rhg cat original
original content
$ $NO_FALLBACK rhg cat -r 1 copy_of_original $ $NO_FALLBACK rhg cat -r 1 copy_of_original
original content original content
Fallback to Python Fallback to Python
$ $NO_FALLBACK rhg cat original $ $NO_FALLBACK rhg cat original --exclude="*.rs"
unsupported feature: `rhg cat` without `--rev` / `-r` unsupported feature: error: Found argument '--exclude' which wasn't expected, or isn't valid in this context
USAGE:
rhg cat [OPTIONS] <FILE>...
For more information try --help
[252] [252]
$ rhg cat original $ rhg cat original --exclude="*.rs"
original content original content
$ FALLBACK_EXE="$RHG_FALLBACK_EXECUTABLE" $ FALLBACK_EXE="$RHG_FALLBACK_EXECUTABLE"
$ unset RHG_FALLBACK_EXECUTABLE $ unset RHG_FALLBACK_EXECUTABLE
$ rhg cat original $ rhg cat original --exclude="*.rs"
abort: 'rhg.on-unsupported=fallback' without 'rhg.fallback-executable' set. abort: 'rhg.on-unsupported=fallback' without 'rhg.fallback-executable' set.
[255] [255]
$ RHG_FALLBACK_EXECUTABLE="$FALLBACK_EXE" $ RHG_FALLBACK_EXECUTABLE="$FALLBACK_EXE"
$ export RHG_FALLBACK_EXECUTABLE $ export RHG_FALLBACK_EXECUTABLE
$ rhg cat original --config rhg.fallback-executable=false $ rhg cat original --exclude="*.rs" --config rhg.fallback-executable=false
[1] [1]
$ rhg cat original --config rhg.fallback-executable=hg-non-existent $ rhg cat original --exclude="*.rs" --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: error: Found argument '--exclude' which wasn't expected, or isn't valid in this context
USAGE:
rhg cat [OPTIONS] <FILE>...
For more information try --help
[252] [252]
$ rhg cat original --config rhg.fallback-executable=rhg $ rhg cat original --exclude="*.rs" --config rhg.fallback-executable=rhg
Blocking recursive fallback. The 'rhg.fallback-executable = rhg' config points to `rhg` itself. Blocking recursive fallback. The 'rhg.fallback-executable = rhg' config points to `rhg` itself.
unsupported feature: `rhg cat` without `--rev` / `-r` unsupported feature: error: Found argument '--exclude' which wasn't expected, or isn't valid in this context
USAGE:
rhg cat [OPTIONS] <FILE>...
For more information try --help
[252] [252]
Requirements Requirements
$ $NO_FALLBACK rhg debugrequirements $ $NO_FALLBACK rhg debugrequirements
dotencode dotencode
fncache fncache
generaldelta generaldelta
persistent-nodemap persistent-nodemap