Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable symlink checking for uncached Snapshot captures #8074

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 25 additions & 2 deletions src/rust/engine/fs/src/lib.rs
Expand Up @@ -541,20 +541,35 @@ pub struct GlobWithSource {
}

///
/// All Stats consumed or return by this type are relative to the root.
/// All Stats consumed or returned by this type are relative to the root.
///
/// If `symlink_aware` is true (as it is by default), `scandir` will produce `Link` entries so that
/// a consumer can explicitly track their expansion. Otherwise, it will allow the operating system
/// to expand the link to its underlying type without regard to the links traversed, and `scandir`
/// will produce only `Dir` and `File` entries.
///
#[derive(Clone)]
pub struct PosixFS {
root: Dir,
ignore: Arc<GitignoreStyleExcludes>,
executor: task_executor::Executor,
symlink_aware: bool,
}

impl PosixFS {
pub fn new<P: AsRef<Path>>(
root: P,
ignore_patterns: &[String],
executor: task_executor::Executor,
) -> Result<PosixFS, String> {
Self::new_symlink_aware(root, ignore_patterns, executor, true)
}

pub fn new_symlink_aware<P: AsRef<Path>>(
stuhood marked this conversation as resolved.
Show resolved Hide resolved
root: P,
ignore_patterns: &[String],
executor: task_executor::Executor,
symlink_aware: bool,
) -> Result<PosixFS, String> {
let root: &Path = root.as_ref();
let canonical_root = root
Expand Down Expand Up @@ -583,6 +598,7 @@ impl PosixFS {
root: canonical_root,
ignore: ignore,
executor: executor,
symlink_aware: symlink_aware,
})
}

Expand All @@ -605,10 +621,17 @@ impl PosixFS {
.read_dir()?
.map(|readdir| {
let dir_entry = readdir?;
let metadata = if self.symlink_aware {
// Use the dir_entry metadata, which is symlink aware.
dir_entry.metadata()?
} else {
// Use an independent stat call to get metadata, which is symlink oblivious.
std::fs::metadata(dir_abs.join(dir_entry.file_name()))?
};
PosixFS::stat_internal(
dir_relative_to_root.0.join(dir_entry.file_name()),
&root,
&std::fs::symlink_metadata(dir_abs.join(dir_entry.file_name()))?,
&metadata,
)
})
.filter(|s| match s {
Expand Down
10 changes: 8 additions & 2 deletions src/rust/engine/fs/store/src/snapshot.rs
Expand Up @@ -459,7 +459,8 @@ impl Snapshot {
/// Note that we don't use a Graph here, and don't cache any intermediate steps, we just place
/// the resultant Snapshot into the store and return it. This is important, because we're reading
/// things from arbitrary filepaths which we don't want to cache in the graph, as we don't watch
/// them for changes.
/// them for changes. Because we're not caching things, we can safely configure the virtual
/// filesystem to be symlink-oblivious.
///
/// If the `digest_hint` is given, first attempt to load the Snapshot using that Digest, and only
/// fall back to actually walking the filesystem if we don't have it (either due to garbage
Expand All @@ -478,7 +479,12 @@ impl Snapshot {
future::result(digest_hint.ok_or_else(|| "No digest hint provided.".to_string()))
.and_then(move |digest| Snapshot::from_digest(store, digest))
.or_else(|_| {
let posix_fs = Arc::new(try_future!(PosixFS::new(root_path, &[], executor)));
let posix_fs = Arc::new(try_future!(PosixFS::new_symlink_aware(
root_path,
&[],
executor,
false
)));

posix_fs
.expand(path_globs)
Expand Down