config: show invalid/dropped config values

Summary:
If config values aren't in configs.allowedconfigs and their containing file isn't in configs.allowedlocations, we drop the config values. Previously there was no way to see them using "sl config", but now they will show up with an "(invalid - dropped)" source.

We achieve this by not actually deleting the config values. If the values are before the active value, we leave them since they will have no effect. If they are after the config value, we scooch them before the active value so they have no effect. If there is no active value, we append a dummy "unset" value so we can keep the invalid values.

Reviewed By: quark-zju

Differential Revision: D49274810

fbshipit-source-id: c2b078718b7bdebec825dc4c382e2b7f8c962988
This commit is contained in:
Muir Manders 2023-10-02 09:57:50 -07:00 committed by Facebook GitHub Bot
parent e156551598
commit 3ce839fbc0
5 changed files with 109 additions and 57 deletions

View File

@ -469,18 +469,17 @@ impl ConfigSet {
) {
for (sname, section) in self.sections.iter_mut() {
for (kname, values) in section.items.iter_mut() {
let values_copy = values.clone();
let mut removals = 0;
// value with a larger index takes precedence.
for (index, value) in values_copy.iter().enumerate() {
// Convert the index into the original index.
let index = index - removals;
let orig_active_value = values.last().cloned();
let mut remove_idxs: Vec<usize> = Vec::new();
// The value index, if any, that will be in effect after removals.
let mut new_active_idx: Option<usize> = None;
// value with a larger index takes precedence.
for (index, value) in values.iter().enumerate() {
// Get the filename of the value's rc location
let path: PathBuf = match value.location() {
None => continue,
Some((path, _)) => path,
};
if let Some((path, _)) = value.location() {
let location: Option<String> = path
.file_name()
.and_then(|f| f.to_str())
@ -510,16 +509,49 @@ impl ConfigSet {
.map(|v| v.as_ref())
.unwrap_or_default(),
);
values.remove(index);
removals += 1;
remove_idxs.push(index);
continue;
}
}
}
new_active_idx = Some(index);
}
// The goal is to leave invalid values as non-active sources so
// they can be seen via the "config" command.
let mut to_splice = Vec::new();
for remove_idx in remove_idxs.iter().rev() {
// Add something to the source to indicate it was marked invalid.
values[*remove_idx].source =
[values[*remove_idx].source.as_ref(), "(invalid - dropped)"]
.join(" ")
.into();
// Keep track of invalid values that are after the desired
// "active" value - we have to do something with them.
if new_active_idx.is_some_and(|new_active_idx| *remove_idx > new_active_idx) {
to_splice.push(values.remove(*remove_idx));
}
}
if let Some(new_active_idx) = new_active_idx {
// If there is an active value, insert invalid values before
// it so they don't take effect.
values.splice(new_active_idx..new_active_idx, to_splice);
} else {
// If there is no active value, insert a dummy "unset" value.
values.push(ValueSource {
value: None,
source: "(invalid - dummy)".into(),
location: None,
});
}
// If the removal changes the config, log it as mismatched.
if let (Some(before_remove), Some(after_remove)) =
(values_copy.last(), values.last())
(orig_active_value, values.last())
{
if before_remove.value != after_remove.value {
let source = match before_remove.location() {

View File

@ -161,7 +161,7 @@ impl<'a> Formattable for ConfigItem<'a> {
write!(
writer,
" {}: {kv_section}{}\n",
source_to_display_string(s),
source_to_display_string(s, options.debug),
value.replace('\n', "\\n"),
)?;
}
@ -194,7 +194,7 @@ fn get_config_item<'a>(
}
Some(ConfigItem {
source: source_to_display_string(config_value_source),
source: source_to_display_string(config_value_source, debug),
section,
key,
value: value.as_ref().map(|v| v.to_string()),
@ -204,7 +204,7 @@ fn get_config_item<'a>(
})
}
fn source_to_display_string(source: &ValueSource) -> String {
fn source_to_display_string(source: &ValueSource, debug: bool) -> String {
source
.location()
.and_then(|(location, range)| {
@ -215,7 +215,12 @@ fn source_to_display_string(source: &ValueSource) -> String {
.filter(|ch| *ch == '\n')
.count();
if !location.as_os_str().is_empty() {
if debug && !source.source().is_empty() {
// The source can contain useful hints, so always show it with --debug.
format!("{}:{}:{}", location.display(), line, source.source())
} else {
format!("{}:{}", location.display(), line)
}
} else {
format!("{}:{}", source.source(), line)
}

View File

@ -44,7 +44,7 @@ Order relative to --config
Attribution works
$ hg config --configfile $TESTTMP/simple.rc mysection --debug
$TESTTMP/simple.rc:2: mysection.myname=myvalue
$TESTTMP/simple.rc:2:--configfile: mysection.myname=myvalue
Cloning adds --configfile values to .hg/hgrc
$ cd ..

View File

@ -189,23 +189,38 @@ Verify configs.allowedlocations limits config loading to the allowed locations
> other_key=other_bar
> EOF
$ hg config --debug | grep ': zz_'
$TESTTMP/shared_copy/.hg/hgrc2:4: zz_other_section.other_key=other_bar
$TESTTMP/shared_copy/.hg/hgrc2:2: zz_section.key=bar
$TESTTMP/shared_copy/.hg/hgrc2:4:repo: zz_other_section.other_key=other_bar
$TESTTMP/shared_copy/.hg/hgrc2:2:repo: zz_section.key=bar
$ hg config --debug --config "configs.allowedlocations=hgrc1 .hgrc" | grep ': zz_'
$TESTTMP/shared_copy/.hg/hgrc1:4: zz_other_section.other_key=other_foo
$TESTTMP/shared_copy/.hg/hgrc1:2: zz_section.key=foo
$TESTTMP/shared_copy/.hg/hgrc1:4:repo: zz_other_section.other_key=other_foo
$TESTTMP/shared_copy/.hg/hgrc1:2:repo: zz_section.key=foo
$ hg config --debug --verbose --config "configs.allowedlocations=hgrc1 .hgrc" | grep ': zz_'
$TESTTMP/shared_copy/.hg/hgrc1:4:repo: zz_other_section.other_key=other_foo
$TESTTMP/shared_copy/.hg/hgrc2:4:repo (invalid - dropped): zz_other_section.other_key=other_bar
$TESTTMP/shared_copy/.hg/hgrc1:2:repo: zz_section.key=foo
$TESTTMP/shared_copy/.hg/hgrc2:2:repo (invalid - dropped): zz_section.key=bar
$ hg config --debug --config "configs.allowedlocations=hgrc2 .hgrc" | grep ': zz_'
$TESTTMP/shared_copy/.hg/hgrc2:4: zz_other_section.other_key=other_bar
$TESTTMP/shared_copy/.hg/hgrc2:2: zz_section.key=bar
$TESTTMP/shared_copy/.hg/hgrc2:4:repo: zz_other_section.other_key=other_bar
$TESTTMP/shared_copy/.hg/hgrc2:2:repo: zz_section.key=bar
$ hg config --debug --config "configs.allowedlocations=hgrc3 .hgrc" | grep ': zz_'
[1]
(invalid - dummy): zz_other_section.other_key=<%unset>
(invalid - dummy): zz_section.key=<%unset>
$ hg config --debug --verbose --config "configs.allowedlocations=hgrc3 .hgrc" | grep ': zz_'
(invalid - dummy): zz_other_section.other_key=<%unset>
$TESTTMP/shared_copy/.hg/hgrc2:4:repo (invalid - dropped): zz_other_section.other_key=other_bar
$TESTTMP/shared_copy/.hg/hgrc1:4:repo (invalid - dropped): zz_other_section.other_key=other_foo
(invalid - dummy): zz_section.key=<%unset>
$TESTTMP/shared_copy/.hg/hgrc2:2:repo (invalid - dropped): zz_section.key=bar
$TESTTMP/shared_copy/.hg/hgrc1:2:repo (invalid - dropped): zz_section.key=foo
$ hg config --debug --config "configs.allowedlocations=hgrc3 .hgrc" --config "configs.allowedconfigs=zz_section.key .hgrc" | grep 'zz_section\.'
--config: configs.allowedconfigs=zz_section.key .hgrc
$TESTTMP/shared_copy/.hg/hgrc2:2: zz_section.key=bar
$TESTTMP/shared_copy/.hg/hgrc2:2:repo: zz_section.key=bar
Verify we load dynamicconfigs during clone
$ cd $TESTTMP

View File

@ -71,7 +71,7 @@ Test repo config loading
> bar=baz
> EOF
$ hg config foo.bar --debug
$TESTTMP/for_testing_dothg_hgrc/.hg/hgrc:2: baz
$TESTTMP/for_testing_dothg_hgrc/.hg/hgrc:2:repo: baz
$ mv .hg/hgrc .hg/config
$ hg config foo.bar --debug
[1]
@ -81,7 +81,7 @@ Test repo config loading
$ sl init
$ cp ../for_testing_dothg_hgrc/.hg/config .sl/config
$ hg config foo.bar --debug
$TESTTMP/for_testing_dotsl_config/.sl/config:2: baz
$TESTTMP/for_testing_dotsl_config/.sl/config:2:repo: baz
$ mv .sl/config .sl/hgrc
$ hg config foo.bar --debug
[1]