Skip to content

Commit

Permalink
fix: canonicalize path
Browse files Browse the repository at this point in the history
  • Loading branch information
mtshiba committed Jul 15, 2023
1 parent 072d62f commit 7e58b59
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 29 deletions.
9 changes: 5 additions & 4 deletions crates/els/rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::collections::HashMap;
use std::path::PathBuf;
use std::time::SystemTime;

use erg_common::pathutil::NormalizedPathBuf;
use erg_common::traits::{Locational, Stream};
use erg_compiler::artifact::IncompleteArtifact;
use erg_compiler::erg_parser::parse::Parsable;
Expand Down Expand Up @@ -146,26 +147,26 @@ impl<Checker: BuildRunnable, Parser: Parsable> Server<Checker, Parser> {
/// self is __included__
pub fn dependencies_of(&self, uri: &NormalizedUrl) -> Vec<NormalizedUrl> {
let graph = self.get_graph().unwrap();
let path = util::uri_to_path(uri);
let path = NormalizedPathBuf::from(util::uri_to_path(uri));
graph.sort().unwrap();
let self_node = graph.get_node(&path).unwrap();
graph
.ref_inner()
.iter()
.filter(|node| node.id == path || self_node.depends_on(&node.id))
.map(|node| NormalizedUrl::new(Url::from_file_path(&node.id).unwrap()))
.map(|node| NormalizedUrl::new(Url::from_file_path(node.id.to_path_buf()).unwrap()))
.collect()
}

/// self is __not included__
pub fn dependents_of(&self, uri: &NormalizedUrl) -> Vec<NormalizedUrl> {
let graph = self.get_graph().unwrap();
let path = util::uri_to_path(uri);
let path = NormalizedPathBuf::from(util::uri_to_path(uri));
graph
.ref_inner()
.iter()
.filter(|node| node.depends_on(&path))
.map(|node| NormalizedUrl::new(Url::from_file_path(&node.id).unwrap()))
.map(|node| NormalizedUrl::new(Url::from_file_path(node.id.to_path_buf()).unwrap()))
.collect()
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/erg_common/pathutil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::normalize_path;
/// `PathBuf` may give false equivalence decisions in non-case-sensitive file systems.
/// Use this for dictionary keys, etc.
/// See also: `els::util::NormalizedUrl`
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash, Default)]
pub struct NormalizedPathBuf(PathBuf);

impl<P: Into<PathBuf>> From<P> for NormalizedPathBuf {
Expand Down Expand Up @@ -41,7 +41,7 @@ impl Deref for NormalizedPathBuf {

impl NormalizedPathBuf {
pub fn new(path: PathBuf) -> Self {
NormalizedPathBuf(normalize_path(path))
NormalizedPathBuf(normalize_path(path.canonicalize().unwrap_or(path)))
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/erg_compiler/module/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl SharedCompilerResource {

pub fn inherit(&self, path: PathBuf) -> Self {
let mut _self = self.clone();
_self.promises.path = path;
_self.promises.path = path.into();
_self
}

Expand Down
77 changes: 57 additions & 20 deletions crates/erg_compiler/module/graph.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use std::fmt;
use std::path::{Path, PathBuf};

use erg_common::pathutil::NormalizedPathBuf;
use erg_common::set;
use erg_common::set::Set;
use erg_common::shared::{MappedRwLockReadGuard, RwLockReadGuard, Shared};
use erg_common::tsort::{tsort, Graph, Node, TopoSortError};
use erg_common::{normalize_path, set};

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum IncRefError {
Expand All @@ -18,7 +19,7 @@ impl IncRefError {
}

#[derive(Debug, Clone, Default)]
pub struct ModuleGraph(Graph<PathBuf, ()>);
pub struct ModuleGraph(Graph<NormalizedPathBuf, ()>);

impl fmt::Display for ModuleGraph {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand All @@ -35,7 +36,7 @@ impl fmt::Display for ModuleGraph {
}

impl IntoIterator for ModuleGraph {
type Item = Node<PathBuf, ()>;
type Item = Node<NormalizedPathBuf, ()>;
type IntoIter = std::vec::IntoIter<Self::Item>;

fn into_iter(self) -> Self::IntoIter {
Expand All @@ -48,20 +49,45 @@ impl ModuleGraph {
Self(Graph::new())
}

pub fn get_node(&self, path: &Path) -> Option<&Node<PathBuf, ()>> {
let path = normalize_path(path.to_path_buf());
pub fn get_node(&self, path: &Path) -> Option<&Node<NormalizedPathBuf, ()>> {
let path = NormalizedPathBuf::new(path.to_path_buf());
self.0.iter().find(|n| n.id == path)
}

fn parents(&self, path: &Path) -> Option<&Set<PathBuf>> {
let path = normalize_path(path.to_path_buf());
/// if `path` depends on `target`, returns `true`, else `false`.
/// if `path` not found, returns `false`
pub fn depends_on(&self, path: &Path, target: &Path) -> bool {
let path = NormalizedPathBuf::new(path.to_path_buf());
let target = NormalizedPathBuf::new(target.to_path_buf());
self.0
.iter()
.find(|n| n.id == path)
.map(|n| n.depends_on.contains(&target))
.unwrap_or(false)
}

pub fn children(&self, path: &Path) -> Set<NormalizedPathBuf> {
let path = NormalizedPathBuf::new(path.to_path_buf());
self.0
.iter()
.filter(|n| n.depends_on.contains(&path))
.map(|n| n.id.clone())
.collect()
}

fn parents(&self, path: &Path) -> Option<&Set<NormalizedPathBuf>> {
let path = NormalizedPathBuf::new(path.to_path_buf());
self.0.iter().find(|n| n.id == path).map(|n| &n.depends_on)
}

pub fn ancestors(&self, path: &Path) -> Set<PathBuf> {
let path = normalize_path(path.to_path_buf());
/// ```erg
/// # a.er
/// b = import "b"
/// ```
/// -> a: child, b: parent
pub fn ancestors(&self, path: &Path) -> Set<NormalizedPathBuf> {
let mut ancestors = set! {};
if let Some(parents) = self.parents(&path) {
if let Some(parents) = self.parents(path) {
for parent in parents.iter() {
ancestors.insert(parent.clone());
ancestors.extend(self.ancestors(parent));
Expand All @@ -71,7 +97,7 @@ impl ModuleGraph {
}

pub fn add_node_if_none(&mut self, path: &Path) {
let path = normalize_path(path.to_path_buf());
let path = NormalizedPathBuf::new(path.to_path_buf());
if self.0.iter().all(|n| n.id != path) {
let node = Node::new(path, (), set! {});
self.0.push(node);
Expand All @@ -80,8 +106,8 @@ impl ModuleGraph {

/// returns Err (and do nothing) if this operation makes a cycle
pub fn inc_ref(&mut self, referrer: &Path, depends_on: PathBuf) -> Result<(), IncRefError> {
let referrer = normalize_path(referrer.to_path_buf());
let depends_on = normalize_path(depends_on);
let referrer = NormalizedPathBuf::new(referrer.to_path_buf());
let depends_on = NormalizedPathBuf::new(depends_on);
if self.ancestors(&depends_on).contains(&referrer) && referrer != depends_on {
return Err(IncRefError::CycleDetected);
}
Expand All @@ -96,7 +122,7 @@ impl ModuleGraph {
Ok(())
}

pub fn iter(&self) -> impl Iterator<Item = &Node<PathBuf, ()>> {
pub fn iter(&self) -> impl Iterator<Item = &Node<NormalizedPathBuf, ()>> {
self.0.iter()
}

Expand All @@ -112,13 +138,13 @@ impl ModuleGraph {
}

pub fn remove(&mut self, path: &Path) {
let path = normalize_path(path.to_path_buf());
let path = NormalizedPathBuf::new(path.to_path_buf());
self.0.retain(|n| n.id != path);
}

pub fn rename_path(&mut self, old: &Path, new: PathBuf) {
let old = normalize_path(old.to_path_buf());
let new = normalize_path(new);
let old = NormalizedPathBuf::new(old.to_path_buf());
let new = NormalizedPathBuf::new(new);
for node in self.0.iter_mut() {
if node.id == old {
node.id = new.clone();
Expand All @@ -145,7 +171,7 @@ impl fmt::Display for SharedModuleGraph {
}

impl IntoIterator for SharedModuleGraph {
type Item = Node<PathBuf, ()>;
type Item = Node<NormalizedPathBuf, ()>;
type IntoIter = std::vec::IntoIter<Self::Item>;

fn into_iter(self) -> Self::IntoIter {
Expand All @@ -158,7 +184,10 @@ impl SharedModuleGraph {
Self(Shared::new(ModuleGraph::new()))
}

pub fn get_node(&self, path: &Path) -> Option<MappedRwLockReadGuard<Node<PathBuf, ()>>> {
pub fn get_node(
&self,
path: &Path,
) -> Option<MappedRwLockReadGuard<Node<NormalizedPathBuf, ()>>> {
if self.0.borrow().get_node(path).is_some() {
Some(RwLockReadGuard::map(self.0.borrow(), |graph| {
graph.get_node(path).unwrap()
Expand All @@ -168,7 +197,15 @@ impl SharedModuleGraph {
}
}

pub fn ancestors(&self, path: &Path) -> Set<PathBuf> {
pub fn depends_on(&self, path: &Path, target: &Path) -> bool {
self.0.borrow().depends_on(path, target)
}

pub fn children(&self, path: &Path) -> Set<NormalizedPathBuf> {
self.0.borrow().children(path)
}

pub fn ancestors(&self, path: &Path) -> Set<NormalizedPathBuf> {
self.0.borrow().ancestors(path)
}

Expand Down
24 changes: 22 additions & 2 deletions crates/erg_compiler/module/promise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl Promise {
#[derive(Debug, Clone, Default)]
pub struct SharedPromises {
graph: SharedModuleGraph,
pub(crate) path: PathBuf,
pub(crate) path: NormalizedPathBuf,
promises: Shared<Dict<NormalizedPathBuf, Promise>>,
}

Expand All @@ -86,7 +86,7 @@ impl SharedPromises {
pub fn new(graph: SharedModuleGraph, path: PathBuf) -> Self {
Self {
graph,
path,
path: NormalizedPathBuf::new(path),
promises: Shared::new(Dict::new()),
}
}
Expand Down Expand Up @@ -125,6 +125,26 @@ impl SharedPromises {
Promise::Running { parent, handle };
return Ok(());
}
// Suppose A depends on B and C, and B depends on C.
// In this case, B must join C before A joins C. Otherwise, a deadlock will occur.
let children = self.graph.children(path);
for child in children.iter() {
if child == &self.path {
continue;
} else if self.graph.depends_on(&self.path, child) {
*self.promises.borrow_mut().get_mut(path).unwrap() =
Promise::Running { parent, handle };
while self
.promises
.borrow()
.get(path)
.is_some_and(|p| !p.is_finished())
{
std::thread::yield_now();
}
return Ok(());
}
}
let res = handle.join();
*self.promises.borrow_mut().get_mut(path).unwrap() = Promise::Finished;
res
Expand Down

0 comments on commit 7e58b59

Please sign in to comment.