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

On Forc.toml change, forc should try to update only package nodes that appear in the lock::Diff #1686

Merged
merged 18 commits into from
Jun 2, 2022
Merged
Show file tree
Hide file tree
Changes from 9 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
150 changes: 146 additions & 4 deletions forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use petgraph::{self, visit::EdgeRef, Directed, Direction};
use serde::{Deserialize, Serialize};
use std::{
collections::{hash_map, BTreeSet, HashMap, HashSet},
fs,
hash::{Hash, Hasher},
path::{Path, PathBuf},
str::FromStr,
Expand All @@ -21,9 +22,8 @@ use sway_core::{
CompileAstResult, CompileError, TreeType, TypedParseTree,
};
use sway_utils::constants;
use url::Url;

use tracing::info;
use url::Url;

type GraphIx = u32;
type Node = Pinned;
Expand Down Expand Up @@ -193,6 +193,148 @@ impl BuildPlan {
})
}

fn get_difference_with_manifest(
&self,
proj_node: NodeIx,
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved
manifest_file: &ManifestFile,
sway_git_tag: &str,
) -> Result<(Vec<Pkg>, Vec<Pkg>)> {
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved
let manifest = Manifest::from_file(manifest_file.path(), sway_git_tag)?;

// Collect dependency `Source`s from graph.
let plan_dep_pkgs: BTreeSet<_> = self
.graph
.edges_directed(proj_node, Direction::Outgoing)
.map(|e| {
let dep_name = e.weight();
let dep_pkg = self.graph[e.target()].unpinned(&self.path_map);
(dep_name, dep_pkg)
})
.clone()
.collect();

// Collect dependency `Source`s from manifest.
let proj_id = self.graph[proj_node].id();
let proj_path = &self.path_map[&proj_id];
let manifest_dep_pkgs = manifest
.deps()
.map(|(dep_name, dep)| {
// NOTE: Temporarily warn about `version` until we have support for registries.
if let Dependency::Detailed(det) = dep {
if det.version.is_some() {
println_yellow_err(&format!(
" WARNING! Dependency \"{}\" specifies the unused `version` field: \
consider using `branch` or `tag` instead",
dep_name
));
}
}

let name = dep.package().unwrap_or(dep_name).to_string();
let source = dep_to_source(proj_path, dep)?;
let dep_pkg = Pkg { name, source };
Ok((dep_name, dep_pkg))
})
.collect::<Result<BTreeSet<_>>>()?;
let added: Vec<Pkg> = manifest_dep_pkgs
.difference(&plan_dep_pkgs)
.clone()
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved
.map(|pkg| (pkg.1.clone()))
.collect();
let removed: Vec<Pkg> = plan_dep_pkgs
.difference(&manifest_dep_pkgs)
.clone()
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved
.map(|pkg| (pkg.1.clone()))
.collect();

Ok((added, removed))
}

pub fn from_old_manifest(
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved
&mut self,
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved
manifest_file: &ManifestFile,
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved
sway_git_tag: &str,
offline_mode: bool,
) -> Result<Self> {
let mut graph = self.graph.clone();
let proj_node = *self
.compilation_order
.last()
.ok_or_else(|| anyhow!("Invalid Graph"))?;
let proj_id = graph[proj_node].id();
let (added, removed) =
self.get_difference_with_manifest(proj_node, manifest_file, sway_git_tag)?;

use petgraph::visit::{Bfs, Walker};
//find root_node_id
let root_node_id = *self.compilation_order().last().unwrap();
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved
let visited_nodes: Vec<NodeIx> = Bfs::new(&graph, root_node_id).iter(&graph).collect();
for node in visited_nodes {
if node == root_node_id {
continue;
}
// if a -> b in the graph, a depends on b. if a is removed b will have 0
// incoming edge, if there is no other parent dependening on b remove it.
// to remove a either it will be in removed list or it needs to have 0 incoming edge.
if graph.edges_directed(node, Direction::Incoming).count() == 0
|| removed
.clone()
.into_iter()
.any(|removed_dep| removed_dep.name == graph[node].name)
{
graph.remove_node(node);
}
}
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved

let mut visited_map: HashMap<Pinned, NodeIx> = HashMap::new();
// After removing some nodes the index of project node can change.
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved
// To find the root node again, create a new compilation order after
// removing removed packages, create the compilation order again
let compilation_order_after_remove = compilation_order(&graph)?;
let mut path_map = graph_to_path_map(
manifest_file.dir(),
&graph,
&compilation_order_after_remove,
sway_git_tag,
)?;
compilation_order_after_remove
.clone()
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved
.into_iter()
.for_each(|node_index| {
visited_map.insert(graph[node_index].clone(), node_index);
});

let fetch_ts = std::time::Instant::now();
let fetch_id = fetch_id(&self.path_map[&proj_id], fetch_ts);
let proj_node_after_delete = compilation_order_after_remove.last().unwrap();
for added_package in added {
let pkg = &added_package;
let pinned_pkg = pin_pkg(fetch_id, pkg, &mut path_map, sway_git_tag)?;
let manifest = Manifest::from_dir(&path_map[&pinned_pkg.id()], sway_git_tag)?;
let added_package_node = graph.add_node(pinned_pkg.clone());
fetch_children(
fetch_id,
offline_mode,
added_package_node,
&manifest,
sway_git_tag,
&mut graph,
&mut path_map,
&mut visited_map,
)?;
graph.add_edge(
*proj_node_after_delete,
added_package_node,
added_package.name.to_string(),
);
}
let compilation_order = compilation_order(&graph)?;
Ok(Self {
graph,
path_map,
compilation_order,
})
}
/// Attempt to load the build plan from the `Lock`.
pub fn from_lock(proj_path: &Path, lock: &Lock, sway_git_tag: &str) -> Result<Self> {
let graph = lock.to_graph()?;
Expand Down Expand Up @@ -358,7 +500,7 @@ impl Pinned {
let id = self.id();
let source = match &self.source {
SourcePinned::Git(git) => Source::Git(git.source.clone()),
SourcePinned::Path => Source::Path(path_map[&id].to_path_buf()),
SourcePinned::Path => Source::Path(fs::canonicalize(&path_map[&id]).unwrap()),
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved
SourcePinned::Registry(reg) => Source::Registry(reg.source.clone()),
};
let name = self.name.clone();
Expand Down Expand Up @@ -905,7 +1047,7 @@ fn dep_to_source(pkg_path: &Path, dep: &Dependency) -> Result<Source> {
Dependency::Detailed(ref det) => match (&det.path, &det.version, &det.git) {
(Some(relative_path), _, _) => {
let path = pkg_path.join(relative_path);
Source::Path(path)
Source::Path(fs::canonicalize(path)?)
kayagokalp marked this conversation as resolved.
Show resolved Hide resolved
}
(_, _, Some(repo)) => {
let reference = match (&det.branch, &det.tag, &det.rev) {
Expand Down
4 changes: 3 additions & 1 deletion forc/src/ops/forc_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ pub fn build(command: BuildCommand) -> Result<pkg::Compiled> {

// Load the build plan from the lock file.
let plan_result = pkg::BuildPlan::from_lock_file(&lock_path, SWAY_GIT_TAG);
let mut old_plan = pkg::BuildPlan::from_lock_file(&lock_path, SWAY_GIT_TAG)
.or_else(|_| pkg::BuildPlan::new(&manifest, SWAY_GIT_TAG, offline))?;

// Retrieve the old lock file state so we can produce a diff.
let old_lock = plan_result
Expand All @@ -69,8 +71,8 @@ pub fn build(command: BuildCommand) -> Result<pkg::Compiled> {
} else {
e
};
let plan = old_plan.from_old_manifest(&manifest, SWAY_GIT_TAG, offline)?;
info!(" Creating a new `Forc.lock` file. (Cause: {})", cause);
let plan = pkg::BuildPlan::new(&manifest, SWAY_GIT_TAG, offline)?;
let lock = Lock::from_graph(plan.graph());
let diff = lock.diff(&old_lock);
lock::print_diff(&manifest.project.name, &diff);
Expand Down