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

[packages] Modify dependency graph construction to use lock files #12575

Merged
merged 44 commits into from
Aug 10, 2023
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
43e0b02
[packages] Modify dependency graph construction to use lock files
awelc Jun 12, 2023
b4ed4f0
Implemented new algorithm
awelc Jun 15, 2023
a964279
Fixes: process internal deps first and reroot packages
awelc Jun 15, 2023
d5a614a
Cycle detection plus other minor changes
awelc Jun 16, 2023
e1d32dd
Process overrides first when combining graphs
awelc Jun 16, 2023
7657b58
Removed unnecessary piece of data passed around
awelc Jun 16, 2023
e801299
Fixed remaining issues and regenerated tests
awelc Jun 20, 2023
703a65a
Test-related fixes
awelc Jun 20, 2023
5af6367
Refactoring
awelc Jun 20, 2023
ad79e62
More refactoring and doc updates
awelc Jun 20, 2023
5f765cd
More doc changes
awelc Jun 20, 2023
696fbd6
Removed a no longer necessary piece of data and updated test output
awelc Jun 20, 2023
9278cd9
Regenerated test output
awelc Jun 21, 2023
43c3012
Regenerated test output
awelc Jun 21, 2023
2439390
Added more tests
awelc Jun 21, 2023
03873f9
Addressed (some) review comments
awelc Jun 22, 2023
0e737c3
Introduced dep graph builder to reduce function parameter clutter
awelc Jun 22, 2023
b79b452
Additional comments
awelc Jun 22, 2023
737d798
A revised sub-graph merging procedure
awelc Jun 29, 2023
6bf1df8
A small refinment to graph merging
awelc Jun 29, 2023
0b27fa9
Another small refinment to graph merging
awelc Jun 29, 2023
68429f1
Removed a forgotten debug print statement
awelc Jul 3, 2023
0af2547
Fixed dependency digest generation problem
awelc Jul 3, 2023
51b2937
Started addressing review comments
awelc Jul 5, 2023
971e861
We should prune overridden dependencies
awelc Jul 6, 2023
2d1cd04
Fixed a comment
awelc Jul 6, 2023
b4130c1
Fixed a pruning problem
awelc Jul 6, 2023
f490f77
Minor refactoring
awelc Jul 6, 2023
697b705
Reformatted error messages
awelc Jul 7, 2023
b46e24b
Added test with lock files
awelc Jul 7, 2023
f8ebbf2
Fixed re-rooting problem and added an additional test
awelc Jul 7, 2023
54fb5b3
Added additional comments
awelc Jul 7, 2023
c348b0a
Output formatting refinements
awelc Jul 11, 2023
de397f4
Started addressing another round of review comments
awelc Jul 28, 2023
a1b8bd8
Minor refactoring and a fix
awelc Jul 28, 2023
a6fb214
Finessed the pruning implementation
awelc Jul 29, 2023
d82cacf
Updated a comment
awelc Jul 29, 2023
95c0659
Regenereated test output
awelc Jul 29, 2023
743b30b
Regenerated test output
awelc Jul 29, 2023
48d4242
Finessed dependency output formatting
awelc Jul 31, 2023
1573bd1
Fixed a direct dependency comparison problem
awelc Aug 4, 2023
642e03a
Addressed review comments
awelc Aug 4, 2023
e0713b7
Regenerated test output
awelc Aug 9, 2023
1884005
Added an auxiliary struct for clarity
awelc Aug 10, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ async fn test_generate_lock_file() {

[move]
version = 0
manifest_digest = "1401DE1C3C3FF6D20EB27741A0A7B5D61E34836CB6C90ECC2F2DE97C47B4D0F9"

dependencies = [
{ name = "Examples" },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
Command `build -v`:
Error: Found cycle between packages: Foo -> Bar -> Foo
Error: Failed to resolve dependencies for package 'Foo'

Caused by:
0: Failed to resolve dependencies for package 'Bar'
1: Failed to resolve dependencies for package 'Foo'
2: Found cycle between packages: Bar -> Foo -> Bar
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,4 @@ Error: Failed to resolve dependencies for package 'A'

Caused by:
0: Parsing manifest for 'Foo'
1: Unable to find package manifest at "./foo"
2: No such file or directory (os error 2)
1: No such file or directory (os error 2)
68 changes: 36 additions & 32 deletions external-crates/move/tools/move-package/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,9 @@ use clap::*;
use move_compiler::editions::{Edition, Flavor};
use move_core_types::account_address::AccountAddress;
use move_model::model::GlobalEnv;
use resolution::{
dependency_cache::DependencyCache, dependency_graph::DependencyGraph,
resolution_graph::ResolvedGraph,
};
use resolution::{dependency_graph::DependencyGraphBuilder, resolution_graph::ResolvedGraph};
use serde::{Deserialize, Serialize};
use source_package::layout::SourcePackageLayout;
use source_package::{layout::SourcePackageLayout, parsed_manifest::DependencyKind};
use std::{
collections::BTreeMap,
io::Write,
Expand All @@ -31,7 +28,6 @@ use crate::{
build_plan::BuildPlan, compiled_package::CompiledPackage, model_builder::ModelBuilder,
},
package_lock::PackageLock,
source_package::manifest_parser,
};

#[derive(Debug, Parser, Clone, Serialize, Deserialize, Eq, PartialEq, PartialOrd, Default)]
Expand Down Expand Up @@ -138,14 +134,12 @@ impl BuildConfig {

pub fn download_deps_for_package<W: Write>(&self, path: &Path, writer: &mut W) -> Result<()> {
let path = SourcePackageLayout::try_find_root(path)?;
let toml_manifest =
self.parse_toml_manifest(path.join(SourcePackageLayout::Manifest.path()))?;
let manifest_string =
std::fs::read_to_string(path.join(SourcePackageLayout::Manifest.path()))?;
let lock_string = std::fs::read_to_string(path.join(SourcePackageLayout::Lock.path())).ok();
let _mutx = PackageLock::lock(); // held until function returns

// This should be locked as it inspects the environment for `MOVE_HOME` which could
// possibly be set by a different process in parallel.
let manifest = manifest_parser::parse_source_manifest(toml_manifest)?;
resolution::download_dependency_repos(&manifest, self, &path, writer)?;
resolution::download_dependency_repos(manifest_string, lock_string, self, &path, writer)?;
Ok(())
}

Expand All @@ -158,29 +152,39 @@ impl BuildConfig {
self.dev_mode = true;
}
let path = SourcePackageLayout::try_find_root(path)?;
let toml_manifest =
self.parse_toml_manifest(path.join(SourcePackageLayout::Manifest.path()))?;
let manifest_string =
std::fs::read_to_string(path.join(SourcePackageLayout::Manifest.path()))?;
let lock_string = std::fs::read_to_string(path.join(SourcePackageLayout::Lock.path())).ok();
let _mutx = PackageLock::lock(); // held until function returns

// This should be locked as it inspects the environment for `MOVE_HOME` which could
// possibly be set by a different process in parallel.
let manifest = manifest_parser::parse_source_manifest(toml_manifest)?;

let mut dependency_cache = DependencyCache::new(self.skip_fetch_latest_git_deps);
let dependency_graph =
DependencyGraph::new(&manifest, path.clone(), &mut dependency_cache, writer)?;

let install_dir = self.install_dir.as_ref().unwrap_or(&path).to_owned();
let lock = dependency_graph.write_to_lock(install_dir)?;
if let Some(lock_path) = &self.lock_file {
lock.commit(lock_path)?;
let mut dep_graph_builder =
DependencyGraphBuilder::new(self.skip_fetch_latest_git_deps, writer);
let (dependency_graph, modified) = dep_graph_builder.get_graph(
&DependencyKind::default(),
path.clone(),
manifest_string,
lock_string,
)?;

if modified {
let install_dir = self.install_dir.as_ref().unwrap_or(&path).to_owned();
let lock = dependency_graph.write_to_lock(install_dir)?;
if let Some(lock_path) = &self.lock_file {
lock.commit(lock_path)?;
}
}

ResolvedGraph::resolve(dependency_graph, self, &mut dependency_cache, writer)
}

fn parse_toml_manifest(&self, path: PathBuf) -> Result<toml::Value> {
let manifest_string = std::fs::read_to_string(path)?;
manifest_parser::parse_move_manifest_string(manifest_string)
let DependencyGraphBuilder {
mut dependency_cache,
progress_output,
..
} = dep_graph_builder;

ResolvedGraph::resolve(
dependency_graph,
self,
&mut dependency_cache,
progress_output,
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ impl DependencyCache {
"FETCHING GIT DEPENDENCY".bold().green(),
git_url,
)?;

// If the cached folder does not exist, download and clone accordingly
Command::new("git")
.args([OsStr::new("clone"), os_git_url, git_path.as_os_str()])
Expand Down
Loading