( )⚙ D10188 rhg: Add an allow-list of ignored extensions

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10188

rhg: Add an allow-list of ignored extensions
ClosedPublic

Authored by SimonSapin on Mar 12 2021, 5:07 PM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Commits
rHG1a036d33bc18: rhg: Add an allow-list of ignored extensions
Summary

Because rhg doesn’t know how a Python extension would affect
behavior it implements in Rust, when an unsupported extension
is enabled it conservatively falls back to Python-based hg.

However many users will have unsupported extensions enabled in practice.
Maybe they don’t actually affect rhg behavior, but we don’t know.
This adds a rhg.ignored-extensions configuration that lets
users list extensions that rhg can safely ignore and proceed even
if they’re not supported in Rust.

Diff Detail

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

Event Timeline

SimonSapin created this revision.Mar 12 2021, 5:07 PM
marmoute accepted this revision.Mar 15 2021, 5:41 AM
marmoute added a subscriber: marmoute.

Looks good, with a small comment on the hint style that we can follow up on.

rust/rhg/src/main.rs
381

Maybe put the hint in (…) as we do for other hint in Mercurial.
(and maybe most this to an explicite hint attribute.

baymax updated this revision to Diff 26338.Mar 15 2021, 9:10 AM

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

SimonSapin added a commit: rHG1a036d33bc18: rhg: Add an allow-list of ignored extensions.Mar 16 2021, 5:45 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG1a036d33bc18: rhg: Add an allow-list of ignored extensions (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/hg-core/src/config/config.rs (26 lines)
Mrust/rhg/src/main.rs (10 lines)

Diff 26293

rust/hg-core/src/config/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.
use super::layer; use super::layer;
use super::values; use super::values;
use crate::config::layer::{ use crate::config::layer::{
ConfigError, ConfigLayer, ConfigOrigin, ConfigValue, ConfigError, ConfigLayer, ConfigOrigin, ConfigValue,
}; };
use crate::utils::files::get_bytes_from_os_str; use crate::utils::files::get_bytes_from_os_str;
use crate::utils::SliceExt;
use format_bytes::{write_bytes, DisplayBytes}; use format_bytes::{write_bytes, DisplayBytes};
use std::collections::HashSet; use std::collections::HashSet;
use std::env; use std::env;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::str; use std::str;
use crate::errors::{HgResultExt, IoResultExt}; use crate::errors::{HgResultExt, IoResultExt};
pub fn get_bool( pub fn get_bool(
&self, &self,
section: &[u8], section: &[u8],
item: &[u8], item: &[u8],
) -> Result<bool, ConfigValueParseError> { ) -> Result<bool, ConfigValueParseError> {
Ok(self.get_option(section, item)?.unwrap_or(false)) Ok(self.get_option(section, item)?.unwrap_or(false))
} }
/// Returns the corresponding list-value in the config if found, or `None`.
///
/// This is appropriate for new configuration keys. The value syntax is
/// **not** the same as most existing list-valued config, which has Python
/// parsing implemented in `parselist()` in `mercurial/config.py`.
/// Faithfully porting that parsing algorithm to Rust (including behavior
/// that are arguably bugs) turned out to be non-trivial and hasn’t been
/// completed as of this writing.
///
/// Instead, the "simple" syntax is: split on comma, then trim leading and
/// trailing whitespace of each component. Quotes or backslashes are not
/// interpreted in any way. Commas are mandatory between values. Values
/// that contain a comma are not supported.
pub fn get_simple_list(
&self,
section: &[u8],
item: &[u8],
) -> Option<impl Iterator<Item = &[u8]>> {
self.get(section, item).map(|value| {
value
.split(|&byte| byte == b',')
.map(|component| component.trim())
})
}
/// Returns the raw value bytes of the first one found, or `None`. /// Returns the raw value bytes of the first one found, or `None`.
pub fn get(&self, section: &[u8], item: &[u8]) -> Option<&[u8]> { pub fn get(&self, section: &[u8], item: &[u8]) -> Option<&[u8]> {
self.get_inner(section, item) self.get_inner(section, item)
.map(|(_, value)| value.bytes.as_ref()) .map(|(_, value)| value.bytes.as_ref())
} }
/// Returns the layer and the value of the first one found, or `None`. /// Returns the layer and the value of the first one found, or `None`.
fn get_inner( fn get_inner(

rust/rhg/src/main.rs

fn check_extensions(config: &Config) -> Result<(), CommandError> { fn check_extensions(config: &Config) -> Result<(), CommandError> {
let enabled = config.get_section_keys(b"extensions"); let enabled = config.get_section_keys(b"extensions");
let mut unsupported = enabled; let mut unsupported = enabled;
for supported in SUPPORTED_EXTENSIONS { for supported in SUPPORTED_EXTENSIONS {
unsupported.remove(supported); unsupported.remove(supported);
} }
if let Some(ignored_list) =
config.get_simple_list(b"rhg", b"ignored-extensions")
{
for ignored in ignored_list {
unsupported.remove(ignored);
}
}
if unsupported.is_empty() { if unsupported.is_empty() {
Ok(()) Ok(())
} else { } else {
Err(CommandError::UnsupportedFeature { Err(CommandError::UnsupportedFeature {
message: format_bytes!( message: format_bytes!(
b"extensions: {}", b"extensions: {}.\nConsider adding them to 'rhg.ignored-extensions' config.",
marmouteUnsubmitted
Not Done

Maybe put the hint in (…) as we do for other hint in Mercurial.
(and maybe most this to an explicite hint attribute.

marmoute: Maybe put the hint in `(…)` as we do for other hint in Mercurial. (and maybe most this to an…
join(unsupported, b", ") join(unsupported, b", ")
), ),
}) })
} }
} }