Skip to content
This repository was archived by the owner on May 4, 2024. It is now read-only.

[move-package] Support dependency overrides #964

Draft
awelc wants to merge 10 commits into
base: sui-move
Choose a base branch
from
Draft

[move-package] Support dependency overrides #964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
98570d0
[move-package] Support dependency overrides
awelc Mar 4, 2023
eb54a5c
Updated test outputs
awelc Mar 4, 2023
bd139e9
Brought back graph entries
awelc Mar 7, 2023
8f938e1
Dependency override does not have to be an option
awelc Mar 7, 2023
2bcfc51
Cosmetic Rust cleanup
awelc Mar 7, 2023
2f0f791
Updated test output
awelc Mar 7, 2023
e1b18b5
Remove dependency overrides from lock files
awelc Mar 7, 2023
8c71fe4
A solution for explicitly defined overrides
awelc Mar 8, 2023
b5f59d6
Normalized local paths when reading manifest
awelc Mar 8, 2023
4a80f06
Cosmetic comment change
awelc Mar 8, 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
413 changes: 321 additions & 92 deletions language/tools/move-package/src/resolution/dependency_graph.rs
View file
Open in desktop

Large diffs are not rendered by default.

18 changes: 17 additions & 1 deletion language/tools/move-package/src/source_package/manifest_parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,11 @@ pub fn parse_dependency(dep_name: &str, mut tval: TV) -> Result<PM::Dependency>
.transpose()?;
let version = table.remove("version").map(parse_version).transpose()?;
let digest = table.remove("digest").map(parse_digest).transpose()?;
let dep_override = table
.remove("override")
.map(parse_dep_override)
.transpose()?
.map_or(false, |o| o);

@amnn amnn Mar 8, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: unwrap_or?

Suggested change
.map_or(false, |o| o);
.unwrap_or(false);


let kind = match (
table.remove("local"),
Expand All @@ -350,7 +355,10 @@ pub fn parse_dependency(dep_name: &str, mut tval: TV) -> Result<PM::Dependency>
bail!("Local source path not a string")
};

PM::DependencyKind::Local(local)
PM::DependencyKind::Local(
// with allow_cwd_parent set to true, it never fails
PM::normalize_path(local, true /* allow_cwd_parent */).unwrap(),
)
}

(None, subdir, Some(git_url), None) => {
Expand Down Expand Up @@ -433,6 +441,7 @@ pub fn parse_dependency(dep_name: &str, mut tval: TV) -> Result<PM::Dependency>
subst,
version,
digest,
dep_override,
}))
}

Expand Down Expand Up @@ -502,6 +511,13 @@ fn parse_digest(tval: TV) -> Result<PM::PackageDigest> {
Ok(PM::PackageDigest::from(digest_str))
}

fn parse_dep_override(tval: TV) -> Result<PM::DepOverride> {
if !tval.is_bool() {
bail!("Invalid dependency override value");
}
Ok(tval.as_bool().unwrap())
}

// check that only recognized names are provided at the top-level
fn warn_if_unknown_field_names(table: &toml::map::Map<String, TV>, known_names: &[&str]) {
let mut unknown_names = BTreeSet::new();
Expand Down
4 changes: 3 additions & 1 deletion language/tools/move-package/src/source_package/parsed_manifest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub type NamedAddress = Symbol;
pub type PackageName = Symbol;
pub type FileName = Symbol;
pub type PackageDigest = Symbol;
pub type DepOverride = bool;

pub type AddressDeclarations = BTreeMap<NamedAddress, Option<AccountAddress>>;
pub type DevAddressDeclarations = BTreeMap<NamedAddress, AccountAddress>;
Expand Down Expand Up @@ -55,6 +56,7 @@ pub struct InternalDependency {
pub subst: Option<Substitution>,
pub version: Option<Version>,
pub digest: Option<PackageDigest>,
pub dep_override: DepOverride,
}

#[derive(Debug, Clone, Eq, PartialEq)]
Expand Down Expand Up @@ -154,7 +156,7 @@ impl Default for DependencyKind {
/// or is prefixed by accesses to parent directories when `allow_cwd_parent` is false.
///
/// Returns the normalized path on success.
fn normalize_path(path: impl AsRef<Path>, allow_cwd_parent: bool) -> Result<PathBuf> {
pub fn normalize_path(path: impl AsRef<Path>, allow_cwd_parent: bool) -> Result<PathBuf> {
use Component::*;

let mut stack = Vec::new();
Expand Down
46 changes: 28 additions & 18 deletions language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,38 +17,38 @@ ResolvedGraph {
Outgoing,
),
],
"B": [
"A": [
(
"C",
Outgoing,
"Root",
Incoming,
),
(
"A",
Incoming,
"B",
Outgoing,
),
(
"Root",
Incoming,
"D",
Outgoing,
),
],
"C": [
"B": [
(
"B",
"A",
Incoming,
),
(
"C",
Outgoing,
),
(
"Root",
Incoming,
),
],
"A": [
"C": [
(
"B",
Outgoing,
),
(
"D",
Outgoing,
Incoming,
),
(
"Root",
Expand All @@ -69,27 +69,31 @@ ResolvedGraph {
),
version: None,
resolver: None,
overridden_in: None,
},
"B": Package {
kind: Local(
"deps_only/B",
),
version: None,
resolver: None,
overridden_in: None,
},
"C": Package {
kind: Local(
"deps_only/C",
),
version: None,
resolver: None,
overridden_in: None,
},
"D": Package {
kind: Local(
"deps_only/D",
),
version: None,
resolver: None,
overridden_in: None,
},
},
always_deps: {
Expand Down Expand Up @@ -142,6 +146,7 @@ ResolvedGraph {
subst: None,
version: None,
digest: None,
dep_override: false,
},
),
},
Expand All @@ -154,6 +159,7 @@ ResolvedGraph {
subst: None,
version: None,
digest: None,
dep_override: false,
},
),
},
Expand Down Expand Up @@ -189,6 +195,7 @@ ResolvedGraph {
subst: None,
version: None,
digest: None,
dep_override: false,
},
),
},
Expand Down Expand Up @@ -266,33 +273,36 @@ ResolvedGraph {
"A": Internal(
InternalDependency {
kind: Local(
"./deps_only/A",
"deps_only/A",
),
subst: None,
version: None,
digest: None,
dep_override: false,
},
),
"C": Internal(
InternalDependency {
kind: Local(
"./deps_only/C",
"deps_only/C",
),
subst: None,
version: None,
digest: None,
dep_override: false,
},
),
},
dev_dependencies: {
"B": Internal(
InternalDependency {
kind: Local(
"./deps_only/B",
"deps_only/B",
),
subst: None,
version: None,
digest: None,
dep_override: false,
},
),
},
Expand Down
4 changes: 3 additions & 1 deletion language/tools/move-package/tests/test_sources/dep_good_digest/Move.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ ResolvedGraph {
),
version: None,
resolver: None,
overridden_in: None,
},
},
always_deps: {
Expand Down Expand Up @@ -104,7 +105,7 @@ ResolvedGraph {
"OtherDep": Internal(
InternalDependency {
kind: Local(
"./deps_only/other_dep",
"deps_only/other_dep",
),
subst: Some(
{
Expand All @@ -117,6 +118,7 @@ ResolvedGraph {
digest: Some(
"6A88B7888D6049EB0121900E22B6FA2C0E702F042C8C8D4FD62AD5C990B9F9A8",
),
dep_override: false,
},
),
},
Expand Down
27 changes: 17 additions & 10 deletions ...e/tools/move-package/tests/test_sources/diamond_problem_backflow_resolution/Move.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ ResolvedGraph {
),
],
"A": [
(
"C",
Outgoing,
),
(
"Root",
Incoming,
),
(
"C",
Outgoing,
),
],
"C": [
(
Expand All @@ -34,14 +34,14 @@ ResolvedGraph {
),
],
"B": [
(
"C",
Outgoing,
),
(
"Root",
Incoming,
),
(
"C",
Outgoing,
),
],
},
package_table: {
Expand All @@ -51,20 +51,23 @@ ResolvedGraph {
),
version: None,
resolver: None,
overridden_in: None,
},
"B": Package {
kind: Local(
"deps_only/B",
),
version: None,
resolver: None,
overridden_in: None,
},
"C": Package {
kind: Local(
"deps_only/C",
),
version: None,
resolver: None,
overridden_in: None,
},
},
always_deps: {
Expand Down Expand Up @@ -123,6 +126,7 @@ ResolvedGraph {
),
version: None,
digest: None,
dep_override: false,
},
),
},
Expand Down Expand Up @@ -171,6 +175,7 @@ ResolvedGraph {
),
version: None,
digest: None,
dep_override: false,
},
),
},
Expand Down Expand Up @@ -238,17 +243,18 @@ ResolvedGraph {
"A": Internal(
InternalDependency {
kind: Local(
"./deps_only/A",
"deps_only/A",
),
subst: None,
version: None,
digest: None,
dep_override: false,
},
),
"B": Internal(
InternalDependency {
kind: Local(
"./deps_only/B",
"deps_only/B",
),
subst: Some(
{
Expand All @@ -259,6 +265,7 @@ ResolvedGraph {
),
version: None,
digest: None,
dep_override: false,
},
),
},
Expand Down
3 changes: 3 additions & 0 deletions language/tools/move-package/tests/test_sources/diamond_problem_dep_conflict/Move.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Failed to resolve dependencies for package 'Root': Resolving dependencies for package 'B': Conflicting dependencies found:
C = { local = "deps_only/C", version = "2.0.0" }
C = { local = "deps_only/C", version = "1.0.0" }
7 changes: 7 additions & 0 deletions language/tools/move-package/tests/test_sources/diamond_problem_dep_conflict/Move.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "Root"
version = "0.0.0"

[dependencies]
A = { local = "./deps_only/A" }
B = { local = "./deps_only/B" }
6 changes: 6 additions & 0 deletions .../tools/move-package/tests/test_sources/diamond_problem_dep_conflict/deps_only/A/Move.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "A"
version = "0.0.0"

[dependencies]
C = { local = "../C", version = "2.0.0" }
6 changes: 6 additions & 0 deletions .../tools/move-package/tests/test_sources/diamond_problem_dep_conflict/deps_only/B/Move.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "B"
version = "0.0.0"

[dependencies]
C = { local = "../C", version = "1.0.0" }
3 changes: 3 additions & 0 deletions .../tools/move-package/tests/test_sources/diamond_problem_dep_conflict/deps_only/C/Move.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[package]
name = "C"
version = "0.0.0"
3 changes: 3 additions & 0 deletions ...tools/move-package/tests/test_sources/diamond_problem_dep_external_conflict/Move.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Failed to resolve dependencies for package 'Root': Adding dependencies from ../resolvers/successful.sh for dependency 'A' in 'Root': Conflicting dependencies found:
ADep = { local = "deps_only/ADep", version = "1.0.0" }
ADep = { local = "deps_only/ADep" } # Resolved by ../resolvers/successful.sh
Loading