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

Formatter fix to not fail when encountering an invalid symlink. #6172

Merged
merged 5 commits into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ regex = { version = "1.6.0" }
serde_yaml = { version = "0.9.16" }
serde-wasm-bindgen = { version = "0.4.5" }
tokio = { version = "1.23.0", features = ["full", "tracing"] }
tokio-stream = { version = "0.1.12", features = ["fs"] }
tokio-util = { version = "0.7.4", features = ["full"] }
wasm-bindgen = { version = "0.2.83", features = ["serde-serialize"] }
wasm-bindgen-test = { version = "0.3.33" }
Expand Down
19 changes: 16 additions & 3 deletions build/base/src/fs/wrappers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use crate::prelude::*;

use std::fs::DirEntry;
use std::fs::File;
use std::fs::Metadata;

Expand All @@ -20,6 +21,12 @@ pub fn metadata<P: AsRef<Path>>(path: P) -> Result<Metadata> {
std::fs::metadata(&path).anyhow_err()
}

/// See [std::fs::symlink_metadata].
#[context("Failed to obtain symlink metadata for file: {}", path.as_ref().display())]
pub fn symlink_metadata<P: AsRef<Path>>(path: P) -> Result<Metadata> {
std::fs::symlink_metadata(&path).anyhow_err()
}

/// See [std::fs::copy].
#[context("Failed to copy file from {} to {}", from.as_ref().display(), to.as_ref().display())]
pub fn copy(from: impl AsRef<Path>, to: impl AsRef<Path>) -> Result<u64> {
Expand All @@ -39,9 +46,15 @@ pub fn read(path: impl AsRef<Path>) -> Result<Vec<u8>> {
}

/// See [std::fs::read_dir].
#[context("Failed to read the directory: {}", path.as_ref().display())]
pub fn read_dir(path: impl AsRef<Path>) -> Result<std::fs::ReadDir> {
std::fs::read_dir(&path).anyhow_err()
pub fn read_dir(path: impl AsRef<Path>) -> Result<impl Iterator<Item = Result<DirEntry>>> {
let path = path.as_ref().to_path_buf();
let read_dir = std::fs::read_dir(&path)
.map_err(|e| anyhow!("Failed to read the directory: '{}'. Error: {}", path.display(), e))?;
Ok(read_dir.into_iter().map(move |elem_result| {
elem_result.map_err(|e| {
anyhow!("Failed to read sub-item from the directory '{}'. Error: {}", path.display(), e)
})
}))
}

/// See [std::fs::read_to_string].
Expand Down
4 changes: 2 additions & 2 deletions build/build/src/project/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ pub async fn bundled_engine_versions(
let mut ret = vec![];

let mut dir_reader = ide_ci::fs::tokio::read_dir(&project_manager_bundle.dist).await?;
while let Some(entry) = dir_reader.next_entry().await? {
if entry.metadata().await?.is_dir() {
while let Some(entry) = dir_reader.try_next().await? {
if ide_ci::fs::tokio::metadata(&entry.path()).await?.is_dir() {
ret.push(Version::from_str(entry.file_name().as_str())?);
}
}
Expand Down
1 change: 1 addition & 0 deletions build/ci_utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ sysinfo = "0.26.2"
tar = "0.4.37"
tempfile = "3.2.0"
tokio = { workspace = true }
tokio-stream = { workspace = true }
tokio-util = { workspace = true }
tracing = { version = "0.1.37" }
tracing-subscriber = { version = "0.3.11", features = ["env-filter"] }
Expand Down
27 changes: 24 additions & 3 deletions build/ci_utils/src/fs/wrappers/tokio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@ pub fn metadata<P: AsRef<Path>>(path: P) -> BoxFuture<'static, Result<std::fs::M
tokio::fs::metadata(path).anyhow_err().boxed()
}

/// See [tokio::fs::symlink_metadata].
pub fn symlink_metadata<P: AsRef<Path>>(path: P) -> BoxFuture<'static, Result<std::fs::Metadata>> {
let path = path.as_ref().to_owned();
tokio::fs::symlink_metadata(path.clone())
.with_context(move || {
format!("Failed to obtain symlink metadata for file: {}", path.display())
})
.boxed()
}

#[context("Failed to open path for reading: {}", path.as_ref().display())]
pub async fn open(path: impl AsRef<Path>) -> Result<File> {
File::open(&path).await.anyhow_err()
Expand Down Expand Up @@ -38,9 +48,20 @@ pub async fn create_dir_all(path: impl AsRef<Path>) -> Result {
tokio::fs::create_dir_all(&path).await.anyhow_err()
}

#[context("Failed to read the directory: {}", path.as_ref().display())]
pub async fn read_dir(path: impl AsRef<Path>) -> Result<tokio::fs::ReadDir> {
tokio::fs::read_dir(&path).await.anyhow_err()
pub async fn read_dir(
path: impl AsRef<Path>,
) -> Result<impl Stream<Item = Result<tokio::fs::DirEntry>>> {
let path = path.as_ref().to_path_buf();
let read_dir = tokio::fs::read_dir(&path)
.await
.with_context(|| format!("Failed to read the directory: {}", path.display()))?;

let stream = tokio_stream::wrappers::ReadDirStream::new(read_dir);
Ok(stream.map(move |entry| {
entry.with_context(|| {
format!("Failed to get entry when reading the directory: {}", path.display())
})
}))
}

#[context("Failed to remove directory with the subtree: {}", path.as_ref().display())]
Expand Down
15 changes: 11 additions & 4 deletions build/enso-formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,9 @@ pub struct RustSourcePath {
/// uses non-documented API and is slow as well (8 seconds for the whole codebase). It should be
/// possible to improve the latter solution to get good performance, but it seems way harder than it
/// should be.
pub async fn process_path(path: impl AsRef<Path>, action: Action) -> Result {
let paths = discover_paths(&path)?;
#[context("Enso Formatter: failed to process root path '{}'.", path.as_ref().display())]
pub async fn process_path(path: impl AsRef<Path> + Copy, action: Action) -> Result {
let paths = discover_paths(path)?;
let total = paths.len();
let mut hash_map = HashMap::<PathBuf, u64>::new();
for (i, sub_path) in paths.iter().enumerate() {
Expand Down Expand Up @@ -396,20 +397,26 @@ pub async fn process_path(path: impl AsRef<Path>, action: Action) -> Result {
}

/// Discover all paths containing Rust sources, recursively.
#[context("Discovering Rust paths failed for '{}' failed.", path.as_ref().display())]
pub fn discover_paths(path: impl AsRef<Path>) -> Result<Vec<RustSourcePath>> {
let mut vec = Vec::default();
discover_paths_internal(&mut vec, path, false)?;
discover_paths_internal(&mut vec, &path, false)?;
Ok(vec)
}

#[context("Discovering paths failed for '{}' failed.", path.as_ref().display())]
pub fn discover_paths_internal(
vec: &mut Vec<RustSourcePath>,
path: impl AsRef<Path>,
is_main_dir: bool,
) -> Result {
use ide_ci::fs;
let path = path.as_ref();
let md = fs::metadata(path)?;
// Below we use `symlink_metadata` instead of `metadata` because the latter follows symlinks.
// We don't want the formatter to fail if it encounters a symlink to a non-existing file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space.

// All files to be formatted should be reachable from the repository root without following
// any symlinks.
let md = fs::symlink_metadata(path)?;
if md.is_dir() && !path.file_name().contains(&"target") {
let dir_name = path.file_name();
// FIXME: This should cover 'tests' folder also, but only the files that contain actual
Expand Down