( )⚙ D10077 rhg: Don’t attempt to read persistent nodemap without .hg/requires opt-in

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10077

rhg: Don’t attempt to read persistent nodemap without .hg/requires opt-in
ClosedPublic

Authored by SimonSapin on Feb 25 2021, 3:32 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG842f2372ced6: rhg: Don’t attempt to read persistent nodemap without .hg/requires opt-in

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

SimonSapin created this revision.Feb 25 2021, 3:32 PM
pulkit added a subscriber: pulkit.Feb 27 2021, 12:11 PM
pulkit added inline comments.
rust/hg-core/src/revlog/nodemap_docket.rs
47

IIRC, the python code raises an error in this case.

SimonSapin added inline comments.Mar 1 2021, 6:02 AM
rust/hg-core/src/revlog/nodemap_docket.rs
47

I what case do you mean? This Rust code looks equivalent to me to the Python code here https://www.mercurial-scm.org/repo/hg/file/5.7/mercurial/revlog.py#l450

(Callers set persistentnodemap to opener.options.get(b'persistent-nodemap', False), which in turn is set based on if requirementsmod.NODEMAP_REQUIREMENT in requirements.)

SimonSapin added a commit: rHG842f2372ced6: rhg: Don’t attempt to read persistent nodemap without .hg/requires opt-in.Mar 4 2021, 7:38 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG842f2372ced6: rhg: Don’t attempt to read persistent nodemap without .hg/requires opt-in (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 26065

rust/hg-core/src/revlog/nodemap_docket.rs

use crate::errors::{HgError, HgResultExt}; use crate::errors::{HgError, HgResultExt};
use crate::requirements;
use bytes_cast::{unaligned, BytesCast}; use bytes_cast::{unaligned, BytesCast};
use memmap::Mmap; use memmap::Mmap;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use super::revlog::RevlogError; use super::revlog::RevlogError;
use crate::repo::Repo; use crate::repo::Repo;
use crate::utils::strip_suffix; use crate::utils::strip_suffix;
/// * The docket has an unsupported version number (repositories created by /// * The docket has an unsupported version number (repositories created by
/// later hg, maybe that should be a requirement instead?), or /// later hg, maybe that should be a requirement instead?), or
/// * The docket file points to a missing (likely deleted) data file (this /// * The docket file points to a missing (likely deleted) data file (this
/// can happen in a rare race condition). /// can happen in a rare race condition).
pub fn read_from_file( pub fn read_from_file(
repo: &Repo, repo: &Repo,
index_path: &Path, index_path: &Path,
) -> Result<Option<(Self, Mmap)>, RevlogError> { ) -> Result<Option<(Self, Mmap)>, RevlogError> {
if !repo
.requirements()
.contains(requirements::NODEMAP_REQUIREMENT)
{
// If .hg/requires does not opt it, don’t try to open a nodemap
return Ok(None);
pulkitUnsubmitted
Not Done

IIRC, the python code raises an error in this case.

pulkit: IIRC, the python code raises an error in this case.
SimonSapinAuthorUnsubmitted
Done

I what case do you mean? This Rust code looks equivalent to me to the Python code here https://www.mercurial-scm.org/repo/hg/file/5.7/mercurial/revlog.py#l450

(Callers set persistentnodemap to opener.options.get(b'persistent-nodemap', False), which in turn is set based on if requirementsmod.NODEMAP_REQUIREMENT in requirements.)

SimonSapin: I what case do you mean? This Rust code looks equivalent to me to the Python code here https…
}
let docket_path = index_path.with_extension("n"); let docket_path = index_path.with_extension("n");
let docket_bytes = if let Some(bytes) = let docket_bytes = if let Some(bytes) =
repo.store_vfs().read(&docket_path).io_not_found_as_none()? repo.store_vfs().read(&docket_path).io_not_found_as_none()?
{ {
bytes bytes
} else { } else {
return Ok(None); return Ok(None);
}; };
.io_not_found_as_none()? .io_not_found_as_none()?
{ {
if mmap.len() >= data_length { if mmap.len() >= data_length {
Ok(Some((docket, mmap))) Ok(Some((docket, mmap)))
} else { } else {
Err(HgError::corrupted("persistent nodemap too short").into()) Err(HgError::corrupted("persistent nodemap too short").into())
} }
} else { } else {
// Even if .hg/requires opted in, some revlogs are deemed small
// enough to not need a persistent nodemap.
Ok(None) Ok(None)
} }
} }
} }
fn rawdata_path(docket_path: &Path, uid: &str) -> PathBuf { fn rawdata_path(docket_path: &Path, uid: &str) -> PathBuf {
let docket_name = docket_path let docket_name = docket_path
.file_name() .file_name()