facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-04 22:07:44 +03:00
Code Issues Projects Releases Wiki Activity

pushrebase - allow exemptions from casefolding check

Browse Source 
Summary:
The casefolding pushrebase check is blocking the sync of www commits causing
problems. Let's exempt www/ dir from it.

We'll also have to modify commit hooks - but that's a separate thing.

Reviewed By: markbt

Differential Revision: D46860559

fbshipit-source-id: 87db959e0d025c0c1fc5c6cfecbdcf96af9e0f81
This commit is contained in:
Mateusz Kwapich 2023-06-20 16:48:55 -07:00 committed by Facebook GitHub Bot
parent e662d3f7cd
commit 0ee50a92c3
6 changed files with 169 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
configerator/structs/scm/mononoke/repos/repos.thrift
View File

@ -1,4 +1,4 @@
// @generated SignedSource<<c2ffba2b1070ae44d5b3b3939ecfe81f>>
// @generated SignedSource<<e3921062e94ce9de9b16b8d4b6a92925>>
// DO NOT EDIT THIS FILE MANUALLY!
// This file is a mechanical copy of the version in the configerator repo. To
// modify it, edit the copy in the configerator repo instead and copy it over by
@ -142,7 +142,7 @@ struct RawRepoConfig {
19: optional RawPushrebaseParams pushrebase;
20: optional RawLfsParams lfs;
22: optional i64 hash_validation_percentage;
23: optional string skiplist_index_blobstore_key;
// 23: deleted
24: optional RawBundle2ReplayParams bundle2_replay_params;
25: optional RawInfinitepushParams infinitepush;
26: optional i64 list_keys_patterns_max;
@ -516,6 +516,8 @@ struct RawPushrebaseParams {
// If assigning globalrevs on a large repo, only do it if the
// small repo is pushredirected.
14: optional i32 globalrevs_small_repo_id;
// Case conflicts in those paths will be ignored
15: optional list<string> casefolding_check_excluded_paths;
} (rust.exhaustive)
struct RawBookmarkConfig {

1
eden/mononoke/metaconfig/parser/src/config.rs
View File

@ -1156,6 +1156,7 @@ mod test {
recursion_limit: Some(1024),
forbid_p2_root_rebases: false,
casefolding_check: false,
casefolding_check_excluded_paths: Default::default(),
not_generated_filenodes_limit: 500,
monitoring_bookmark: None,
},

9
eden/mononoke/metaconfig/parser/src/convert/repo.rs
View File

@ -295,6 +295,15 @@ impl Convert for RawPushrebaseParams {
casefolding_check: self
.casefolding_check
.unwrap_or(default.flags.casefolding_check),
casefolding_check_excluded_paths: self
.casefolding_check_excluded_paths
.map(|raw| {
raw.into_iter()
.map(|path| MPath::new_opt(path.as_bytes()))
.collect::<Result<PrefixTrie>>()
})
.transpose()?
.unwrap_or_default(),
not_generated_filenodes_limit: 500,
monitoring_bookmark: self.monitoring_bookmark,
},

3
eden/mononoke/metaconfig/types/src/lib.rs
View File

@ -625,6 +625,8 @@ pub struct PushrebaseFlags {
pub forbid_p2_root_rebases: bool,
/// Whether to do chasefolding check during pushrebase
pub casefolding_check: bool,
/// Whether to do chasefolding check during pushrebase
pub casefolding_check_excluded_paths: PrefixTrie,
/// How many commits are allowed to not have filenodes generated.
pub not_generated_filenodes_limit: u64,
/// Which bookmark to track in ODS
@ -638,6 +640,7 @@ impl Default for PushrebaseFlags {
recursion_limit: Some(16384), // this number is fairly arbirary
forbid_p2_root_rebases: true,
casefolding_check: true,
casefolding_check_excluded_paths: PrefixTrie::new(),
not_generated_filenodes_limit: 500,
monitoring_bookmark: None,
}

88
eden/mononoke/mononoke_types/src/path.rs
View File

@ -1086,16 +1086,18 @@ impl FromIterator<Option<MPath>> for PrefixTrie {
}
}
pub struct CaseConflictTrie {
children: HashMap<MPathElement, CaseConflictTrie>,
pub struct CaseConflictTrie<'a> {
children: HashMap<MPathElement, CaseConflictTrie<'a>>,
lowercase_to_original: HashMap<String, MPathElement>,
exclusions: &'a PrefixTrie,
}
impl CaseConflictTrie {
fn new() -> CaseConflictTrie {
impl<'a> CaseConflictTrie<'a> {
fn new(exclusions: &'a PrefixTrie) -> CaseConflictTrie {
CaseConflictTrie {
children: HashMap::new(),
lowercase_to_original: HashMap::new(),
children: Default::default(),
lowercase_to_original: Default::default(),
exclusions,
}
}
@ -1103,18 +1105,29 @@ impl CaseConflictTrie {
self.children.is_empty()
}
/// Returns `true` if element was added successfully, or `false`
/// Returns Ok(()) if element was added successfully, or Err
/// if trie already contains case conflicting entry.
fn add<'p, P: IntoIterator<Item = &'p MPathElement>>(
&mut self,
path: P,
) -> Result<(), ReverseMPath> {
fn add<'p, P>(&mut self, path: P) -> Result<(), ReverseMPath>
where
P: IntoIterator<Item = &'p MPathElement> + Clone,
{
if self.exclusions.contains_prefix(path.clone().into_iter()) {
return Ok(());
}
self.add_internal(path)
}
fn add_internal<'p, P>(&mut self, path: P) -> Result<(), ReverseMPath>
where
P: IntoIterator<Item = &'p MPathElement>,
{
let mut iter = path.into_iter();
match iter.next() {
None => Ok(()),
Some(element) => {
if let Some(child) = self.children.get_mut(element) {
return child.add(iter).map_err(|mut e| {
return child.add_internal(iter).map_err(|mut e| {
e.elements.push(element.clone());
e
});
@ -1132,8 +1145,8 @@ impl CaseConflictTrie {
self.children
.entry(element.clone())
.or_insert_with(CaseConflictTrie::new)
.add(iter)
.or_insert_with(|| CaseConflictTrie::new(self.exclusions))
.add_internal(iter)
}
}
}
@ -1141,14 +1154,28 @@ impl CaseConflictTrie {
/// Remove path from a trie
///
/// Returns `true` if path was removed, otherwise `false`.
fn remove<'p, P: IntoIterator<Item = &'p MPathElement>>(&mut self, path: P) -> bool {
fn remove<'p, P>(&mut self, path: P) -> bool
where
P: IntoIterator<Item = &'p MPathElement> + Clone,
{
if self.exclusions.contains_prefix(path.clone().into_iter()) {
return true;
}
self.remove_internal(path)
}
fn remove_internal<'p, P>(&mut self, path: P) -> bool
where
P: IntoIterator<Item = &'p MPathElement>,
{
let mut iter = path.into_iter();
match iter.next() {
None => true,
Some(element) => {
let (found, remove) = match self.children.get_mut(element) {
None => return false,
Some(child) => (child.remove(iter), child.is_empty()),
Some(child) => (child.remove_internal(iter), child.is_empty()),
};
if remove {
self.children.remove(element);
@ -1221,12 +1248,12 @@ impl<'a> CaseConflictTrieUpdate for &'a BonsaiChangeset {
/// Returns first path pair that would introduce a case-conflict, if any. The first element is the
/// first one that was added into the Trie, and the second is the last.
pub fn check_case_conflicts<P, I>(iter: I) -> Option<(MPath, MPath)>
pub fn check_case_conflicts<P, I>(iter: I, exclusions: &PrefixTrie) -> Option<(MPath, MPath)>
where
P: CaseConflictTrieUpdate,
I: IntoIterator<Item = P>,
{
let mut trie = CaseConflictTrie::new();
let mut trie = CaseConflictTrie::new(exclusions);
for update in iter {
let conflict = update.apply(&mut trie);
if conflict.is_some() {
@ -1236,20 +1263,6 @@ where
None
}
// TODO: Do we need this? Why?
impl<P> FromIterator<P> for CaseConflictTrie
where
P: CaseConflictTrieUpdate,
{
fn from_iter<I: IntoIterator<Item = P>>(iter: I) -> Self {
let mut trie = CaseConflictTrie::new();
for update in iter {
let _ = update.apply(&mut trie);
}
trie
}
}
#[cfg(test)]
mod test {
use std::mem::size_of;
@ -1509,8 +1522,11 @@ mod test {
MPath::new(mpath).unwrap()
}
let mut trie: CaseConflictTrie = vec!["a/b/c", "a/d", "c/d/a"].into_iter().map(m).collect();
let exclusions = Default::default();
let mut trie = CaseConflictTrie::new(&exclusions);
assert!(trie.add(&m("a/b/c")).is_ok());
assert!(trie.add(&m("a/d")).is_ok());
assert!(trie.add(&m("c/d/a")).is_ok());
assert!(trie.add(&m("a/b/c")).is_ok());
assert!(trie.add(&m("a/B/d")).is_err());
assert!(trie.add(&m("a/b/C")).is_err());
@ -1525,11 +1541,11 @@ mod test {
m("a/c"),
];
assert_eq!(
check_case_conflicts(paths.iter()), // works from &MPath
check_case_conflicts(paths.iter(), &Default::default()), // works from &MPath
Some((m("a/b"), m("a/B/d"))),
);
assert_eq!(
check_case_conflicts(paths.into_iter()), // works from MPath
check_case_conflicts(paths.into_iter(), &Default::default()), // works from MPath
Some((m("a/b"), m("a/B/d"))),
);
}

101
eden/mononoke/pushrebase/src/lib.rs
View File

@ -357,7 +357,10 @@ async fn rebase_in_loop(
}
if config.casefolding_check {
let conflict = check_case_conflicts(server_bcs.iter().chain(client_bcs.iter()));
let conflict = check_case_conflicts(
server_bcs.iter().chain(client_bcs.iter()),
&config.casefolding_check_excluded_paths,
);
if let Some(conflict) = conflict {
return Err(PushrebaseError::PotentialCaseConflict(conflict.1));
}
@ -1266,6 +1269,7 @@ mod tests {
use maplit::hashset;
use mononoke_types::BonsaiChangesetMut;
use mononoke_types::FileType;
use mononoke_types::PrefixTrie;
use mononoke_types::RepositoryId;
use mutable_counters::MutableCounters;
use mutable_counters::MutableCountersRef;
@ -2154,6 +2158,101 @@ mod tests {
Ok(())
})
}
#[fbinit::test]
fn pushrebase_case_conflict_exclusion(fb: FacebookInit) -> Result<(), Error> {
let runtime = tokio::runtime::Runtime::new().unwrap();
runtime.block_on(async move {
let ctx = CoreContext::test_mock(fb);
let repo: PushrebaseTestRepo = ManyFilesDirs::get_custom_test_repo(fb).await;
let root = repo
.bonsai_hg_mapping()
.get_bonsai_from_hg(
&ctx,
HgChangesetId::from_str("5a28e25f924a5d209b82ce0713d8d83e68982bc8")?,
)
.await?
.ok_or_else(|| Error::msg("Root is missing"))?;
let bcs1 = CreateCommitContext::new(&ctx, &repo, vec![root])
.add_file("dir1/File_1_in_dir1", "data")
.commit()
.await?;
let hgcs1 = hashset![repo.derive_hg_changeset(&ctx, bcs1).await?];
let bcs2 = CreateCommitContext::new(&ctx, &repo, vec![root])
.add_file("dir2/File_1_in_dir2", "data")
.commit()
.await?;
let hgcs2 = hashset![repo.derive_hg_changeset(&ctx, bcs2).await?];
let book = master_bookmark();
set_bookmark(
ctx.clone(),
&repo,
&book,
"2f866e7e549760934e31bf0420a873f65100ad63",
)
.await?;
let result = do_pushrebase(&ctx, &repo, &Default::default(), &book, &hgcs1).await;
match result {
Err(PushrebaseError::PotentialCaseConflict(conflict)) => {
assert_eq!(conflict, MPath::new("dir1/File_1_in_dir1")?)
}
_ => panic!("push-rebase should have failed with case conflict"),
};
// make sure that it is succeeds with exclusion
do_pushrebase(
&ctx,
&repo,
&PushrebaseFlags {
casefolding_check: true,
casefolding_check_excluded_paths: PrefixTrie::from_iter(
vec![Some(MPath::new("dir1")?)].into_iter(),
),
..Default::default()
},
&book,
&hgcs1,
)
.await?;
// revert bookmark back
set_bookmark(
ctx.clone(),
&repo,
&book,
"2f866e7e549760934e31bf0420a873f65100ad63",
)
.await?;
// make sure that exclusion doesn't exclude too much
let result = do_pushrebase(
&ctx,
&repo,
&PushrebaseFlags {
casefolding_check: true,
casefolding_check_excluded_paths: PrefixTrie::from_iter(
vec![Some(MPath::new("dir1")?)].into_iter(),
),
..Default::default()
},
&book,
&hgcs2,
)
.await;
match result {
Err(PushrebaseError::PotentialCaseConflict(conflict)) => {
assert_eq!(conflict, MPath::new("dir2/File_1_in_dir2")?)
}
_ => panic!("push-rebase should have failed with case conflict"),
};
Ok(())
})
}
#[test]
fn pushrebase_intersect_changed() -> Result<(), Error> {