Set Durability to 'HIGH' for most inputs and third-party libraries (#12566)

This commit is contained in:
Micha Reiser 2024-07-30 11:03:59 +02:00 committed by GitHub
parent fb9f566f56
commit a2286c8e47
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 212 additions and 126 deletions

View file

@ -1,8 +1,9 @@
use std::fmt::Formatter;
use std::sync::Arc;
use countme::Count;
use dashmap::mapref::entry::Entry;
use salsa::Setter;
use salsa::{Durability, Setter};
pub use file_root::{FileRoot, FileRootKind};
pub use path::FilePath;
@ -13,28 +14,35 @@ use crate::files::file_root::FileRoots;
use crate::files::private::FileStatus;
use crate::system::{Metadata, SystemPath, SystemPathBuf, SystemVirtualPath, SystemVirtualPathBuf};
use crate::vendored::{VendoredPath, VendoredPathBuf};
use crate::{Db, FxDashMap};
use crate::{vendored, Db, FxDashMap};
mod file_root;
mod path;
/// Interns a file system path and returns a salsa `File` ingredient.
///
/// Returns `None` if the path doesn't exist, isn't accessible, or if the path points to a directory.
/// Returns `Err` if the path doesn't exist, isn't accessible, or if the path points to a directory.
#[inline]
pub fn system_path_to_file(db: &dyn Db, path: impl AsRef<SystemPath>) -> Option<File> {
pub fn system_path_to_file(db: &dyn Db, path: impl AsRef<SystemPath>) -> Result<File, FileError> {
let file = db.files().system(db, path.as_ref());
// It's important that `vfs.file_system` creates a `VfsFile` even for files that don't exist or don't
// exist anymore so that Salsa can track that the caller of this function depends on the existence of
// that file. This function filters out files that don't exist, but Salsa will know that it must
// re-run the calling query whenever the `file`'s status changes (because of the `.status` call here).
file.exists(db).then_some(file)
match file.status(db) {
FileStatus::Exists => Ok(file),
FileStatus::IsADirectory => Err(FileError::IsADirectory),
FileStatus::NotFound => Err(FileError::NotFound),
}
}
/// Interns a vendored file path. Returns `Some` if the vendored file for `path` exists and `None` otherwise.
#[inline]
pub fn vendored_path_to_file(db: &dyn Db, path: impl AsRef<VendoredPath>) -> Option<File> {
pub fn vendored_path_to_file(
db: &dyn Db,
path: impl AsRef<VendoredPath>,
) -> Result<File, FileError> {
db.files().vendored(db, path.as_ref())
}
@ -68,7 +76,7 @@ impl Files {
/// For a non-existing file, creates a new salsa [`File`] ingredient and stores it for future lookups.
///
/// The operation always succeeds even if the path doesn't exist on disk, isn't accessible or if the path points to a directory.
/// In these cases, a file with status [`FileStatus::Deleted`] is returned.
/// In these cases, a file with status [`FileStatus::NotFound`] is returned.
#[tracing::instrument(level = "trace", skip(self, db))]
fn system(&self, db: &dyn Db, path: &SystemPath) -> File {
let absolute = SystemPath::absolute(path, db.system().current_directory());
@ -78,27 +86,32 @@ impl Files {
.system_by_path
.entry(absolute.clone())
.or_insert_with(|| {
// TODO: Set correct durability according to source root.
let metadata = db.system().path_metadata(path);
let durability = self
.root(db, path)
.map_or(Durability::default(), |root| root.durability(db));
match metadata {
Ok(metadata) if metadata.file_type().is_file() => File::new(
db,
FilePath::System(absolute),
let (permissions, revision, status) = match metadata {
Ok(metadata) if metadata.file_type().is_file() => (
metadata.permissions(),
metadata.revision(),
FileStatus::Exists,
Count::default(),
),
_ => File::new(
db,
FilePath::System(absolute),
None,
FileRevision::zero(),
FileStatus::Deleted,
Count::default(),
),
}
Ok(metadata) if metadata.file_type().is_directory() => {
(None, FileRevision::zero(), FileStatus::IsADirectory)
}
_ => (None, FileRevision::zero(), FileStatus::NotFound),
};
File::builder(
FilePath::System(absolute),
permissions,
revision,
status,
Count::default(),
)
.durability(durability)
.new(db)
})
}
@ -114,20 +127,27 @@ impl Files {
/// Looks up a vendored file by its path. Returns `Some` if a vendored file for the given path
/// exists and `None` otherwise.
#[tracing::instrument(level = "trace", skip(self, db))]
fn vendored(&self, db: &dyn Db, path: &VendoredPath) -> Option<File> {
fn vendored(&self, db: &dyn Db, path: &VendoredPath) -> Result<File, FileError> {
let file = match self.inner.vendored_by_path.entry(path.to_path_buf()) {
Entry::Occupied(entry) => *entry.get(),
Entry::Vacant(entry) => {
let metadata = db.vendored().metadata(path).ok()?;
let metadata = match db.vendored().metadata(path) {
Ok(metadata) => match metadata.kind() {
vendored::FileType::File => metadata,
vendored::FileType::Directory => return Err(FileError::IsADirectory),
},
Err(_) => return Err(FileError::NotFound),
};
let file = File::new(
db,
let file = File::builder(
FilePath::Vendored(path.to_path_buf()),
Some(0o444),
metadata.revision(),
FileStatus::Exists,
Count::default(),
);
)
.durability(Durability::HIGH)
.new(db);
entry.insert(file);
@ -135,7 +155,7 @@ impl Files {
}
};
Some(file)
Ok(file)
}
/// Looks up a virtual file by its `path`.
@ -210,8 +230,7 @@ impl Files {
let inner = Arc::clone(&db.files().inner);
for entry in inner.system_by_path.iter_mut() {
if entry.key().starts_with(&path) {
let file = entry.value();
file.sync(db);
File::sync_system_path(db, entry.key(), Some(*entry.value()));
}
}
@ -219,7 +238,9 @@ impl Files {
for root in roots.all() {
if root.path(db).starts_with(&path) {
root.set_revision(db).to(FileRevision::now());
root.set_revision(db)
.with_durability(Durability::HIGH)
.to(FileRevision::now());
}
}
}
@ -236,14 +257,15 @@ impl Files {
pub fn sync_all(db: &mut dyn Db) {
let inner = Arc::clone(&db.files().inner);
for entry in inner.system_by_path.iter_mut() {
let file = entry.value();
file.sync(db);
File::sync_system_path(db, entry.key(), Some(*entry.value()));
}
let roots = inner.roots.read().unwrap();
for root in roots.all() {
root.set_revision(db).to(FileRevision::now());
root.set_revision(db)
.with_durability(Durability::HIGH)
.to(FileRevision::now());
}
}
}
@ -335,6 +357,7 @@ impl File {
#[tracing::instrument(level = "debug", skip(db))]
pub fn sync_path(db: &mut dyn Db, path: &SystemPath) {
let absolute = SystemPath::absolute(path, db.system().current_directory());
Files::touch_root(db, &absolute);
Self::sync_system_path(db, &absolute, None);
}
@ -345,6 +368,7 @@ impl File {
match path {
FilePath::System(system) => {
Files::touch_root(db, &system);
Self::sync_system_path(db, &system, Some(self));
}
FilePath::Vendored(_) => {
@ -357,34 +381,56 @@ impl File {
}
fn sync_system_path(db: &mut dyn Db, path: &SystemPath, file: Option<File>) {
Files::touch_root(db, path);
let Some(file) = file.or_else(|| db.files().try_system(db, path)) else {
return;
};
let metadata = db.system().path_metadata(path);
Self::sync_impl(db, metadata, file);
let durability = db.files().root(db, path).map(|root| root.durability(db));
Self::sync_impl(db, metadata, file, durability);
}
fn sync_system_virtual_path(db: &mut dyn Db, path: &SystemVirtualPath, file: File) {
let metadata = db.system().virtual_path_metadata(path);
Self::sync_impl(db, metadata, file);
Self::sync_impl(db, metadata, file, None);
}
/// Private method providing the implementation for [`Self::sync_system_path`] and
/// [`Self::sync_system_virtual_path`].
fn sync_impl(db: &mut dyn Db, metadata: crate::system::Result<Metadata>, file: File) {
fn sync_impl(
db: &mut dyn Db,
metadata: crate::system::Result<Metadata>,
file: File,
durability: Option<Durability>,
) {
let (status, revision, permission) = match metadata {
Ok(metadata) if metadata.file_type().is_file() => (
FileStatus::Exists,
metadata.revision(),
metadata.permissions(),
),
_ => (FileStatus::Deleted, FileRevision::zero(), None),
Ok(metadata) if metadata.file_type().is_directory() => {
(FileStatus::IsADirectory, FileRevision::zero(), None)
}
_ => (FileStatus::NotFound, FileRevision::zero(), None),
};
file.set_status(db).to(status);
file.set_revision(db).to(revision);
file.set_permissions(db).to(permission);
let durability = durability.unwrap_or_default();
if file.status(db) != status {
file.set_status(db).with_durability(durability).to(status);
}
if file.revision(db) != revision {
file.set_revision(db)
.with_durability(durability)
.to(revision);
}
if file.permissions(db) != permission {
file.set_permissions(db)
.with_durability(durability)
.to(permission);
}
}
/// Returns `true` if the file exists.
@ -401,15 +447,35 @@ mod private {
/// The file exists.
Exists,
/// The file was deleted, didn't exist to begin with or the path isn't a file.
Deleted,
/// The path isn't a file and instead points to a directory.
IsADirectory,
/// The path doesn't exist, isn't accessible, or no longer exists.
NotFound,
}
}
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum FileError {
IsADirectory,
NotFound,
}
impl std::fmt::Display for FileError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
FileError::IsADirectory => f.write_str("Is a directory"),
FileError::NotFound => f.write_str("Not found"),
}
}
}
impl std::error::Error for FileError {}
#[cfg(test)]
mod tests {
use crate::file_revision::FileRevision;
use crate::files::{system_path_to_file, vendored_path_to_file};
use crate::files::{system_path_to_file, vendored_path_to_file, FileError};
use crate::system::DbWithTestSystem;
use crate::tests::TestDb;
use crate::vendored::tests::VendoredFileSystemBuilder;
@ -435,7 +501,7 @@ mod tests {
let test = system_path_to_file(&db, "test.py");
assert_eq!(test, None);
assert_eq!(test, Err(FileError::NotFound));
}
#[test]
@ -477,6 +543,9 @@ mod tests {
fn stubbed_vendored_file_non_existing() {
let db = TestDb::new();
assert_eq!(vendored_path_to_file(&db, "test.py"), None);
assert_eq!(
vendored_path_to_file(&db, "test.py"),
Err(FileError::NotFound)
);
}
}

View file

@ -1,6 +1,7 @@
use std::fmt::Formatter;
use path_slash::PathExt;
use salsa::Durability;
use crate::file_revision::FileRevision;
use crate::system::{SystemPath, SystemPathBuf};
@ -83,7 +84,9 @@ impl FileRoots {
let mut route = normalized_path.replace('{', "{{").replace('}', "}}");
// Insert a new source root
let root = FileRoot::new(db, path, kind, FileRevision::now());
let root = FileRoot::builder(path, kind, FileRevision::now())
.durability(Durability::HIGH)
.new(db);
// Insert a path that matches the root itself
self.by_path.insert(route.clone(), root).unwrap();

View file

@ -95,8 +95,8 @@ impl FilePath {
#[inline]
pub fn to_file(&self, db: &dyn Db) -> Option<File> {
match self {
FilePath::System(path) => system_path_to_file(db, path),
FilePath::Vendored(path) => vendored_path_to_file(db, path),
FilePath::System(path) => system_path_to_file(db, path).ok(),
FilePath::Vendored(path) => vendored_path_to_file(db, path).ok(),
FilePath::SystemVirtual(_) => None,
}
}

View file

@ -1,4 +1,5 @@
use crate::{system::SystemPathBuf, Db};
use salsa::Durability;
#[salsa::input(singleton)]
pub struct Program {
@ -10,7 +11,9 @@ pub struct Program {
impl Program {
pub fn from_settings(db: &dyn Db, settings: ProgramSettings) -> Self {
Program::new(db, settings.target_version, settings.search_paths)
Program::builder(settings.target_version, settings.search_paths)
.durability(Durability::HIGH)
.new(db)
}
}

View file

@ -207,7 +207,9 @@ impl MemoryFileSystem {
let normalized = self.normalize_path(path.as_ref());
get_or_create_file(&mut by_path, &normalized)?.content = content.to_string();
let file = get_or_create_file(&mut by_path, &normalized)?;
file.content = content.to_string();
file.last_modified = FileTime::now();
Ok(())
}

View file

@ -1,3 +1,7 @@
use std::any::Any;
use std::panic::RefUnwindSafe;
use std::sync::Arc;
use ruff_notebook::{Notebook, NotebookError};
use ruff_python_trivia::textwrap;
@ -6,9 +10,6 @@ use crate::system::{
DirectoryEntry, MemoryFileSystem, Metadata, Result, System, SystemPath, SystemVirtualPath,
};
use crate::Db;
use std::any::Any;
use std::panic::RefUnwindSafe;
use std::sync::Arc;
use super::walk_directory::WalkDirectoryBuilder;