From dea8394c35423e70251346574f0acc187f8f72a1 Mon Sep 17 00:00:00 2001 From: magnalite Date: Thu, 1 Jun 2023 18:52:05 +0100 Subject: [PATCH] Fix dev dependency link generation --- src/installation.rs | 25 +++++++++--------- src/resolution.rs | 10 +++++-- .../default.project.json | 6 +++++ .../src/init.lua | 5 ++++ .../wally.toml | 15 +++++++++++ test-projects/dev-dependency/wally.toml | 4 --- tests/integration/install.rs | 5 ++++ .../integration__install__dev_dependency.snap | 16 +++++------- ...v_dependency_also_required_as_non_dev.snap | 26 +++++++++++++++++++ 9 files changed, 84 insertions(+), 28 deletions(-) create mode 100644 test-projects/dev-dependency-also-required-as-non-dev/default.project.json create mode 100644 test-projects/dev-dependency-also-required-as-non-dev/src/init.lua create mode 100644 test-projects/dev-dependency-also-required-as-non-dev/wally.toml create mode 100644 tests/integration/snapshots/integration__install__dev_dependency_also_required_as_non_dev.snap diff --git a/src/installation.rs b/src/installation.rs index cc07045f..b23e0b7e 100644 --- a/src/installation.rs +++ b/src/installation.rs @@ -89,27 +89,30 @@ impl InstallationContext { // package links for its dependencies. if package_id == &root_package_id { if let Some(deps) = shared_deps { - self.write_root_package_links(Realm::Shared, deps, Realm::Shared)?; + self.write_root_package_links(Realm::Shared, deps, resolved)?; } if let Some(deps) = server_deps { - self.write_root_package_links(Realm::Server, deps, Realm::Server)?; + self.write_root_package_links(Realm::Server, deps, resolved)?; } if let Some(deps) = dev_deps { - self.write_root_package_links(Realm::Dev, deps, Realm::Dev)?; + self.write_root_package_links(Realm::Dev, deps, resolved)?; } } else { let metadata = resolved.metadata.get(package_id).unwrap(); - let package_realm = metadata.origin_realm; if let Some(deps) = shared_deps { - self.write_package_links(package_id, package_realm, deps, Realm::Shared)?; + self.write_package_links(package_id, package_realm, deps, resolved)?; } if let Some(deps) = server_deps { - self.write_package_links(package_id, package_realm, deps, Realm::Server)?; + self.write_package_links(package_id, package_realm, deps, resolved)?; + } + + if let Some(deps) = dev_deps { + self.write_package_links(package_id, package_realm, deps, resolved)?; } let source_registry = &resolved.metadata[package_id].source_registry; @@ -185,10 +188,6 @@ impl InstallationContext { })?; let contents = formatdoc! {r#" - if not game:GetService("RunService"):IsServer() then - error("{full_name} is a server-only package.", 2) - end - return require({packages}._Index["{full_name}"]["{short_name}"]) "#, packages = server_path, @@ -203,7 +202,7 @@ impl InstallationContext { &self, root_realm: Realm, dependencies: impl IntoIterator, - dependencies_realm: Realm, + resolved: &Resolve, ) -> anyhow::Result<()> { log::debug!("Writing root package links"); @@ -217,6 +216,7 @@ impl InstallationContext { fs::create_dir_all(base_path)?; for (dep_name, dep_package_id) in dependencies { + let dependencies_realm = resolved.metadata.get(dep_package_id).unwrap().origin_realm; let path = base_path.join(format!("{}.lua", dep_name)); let contents = match (root_realm, dependencies_realm) { @@ -240,7 +240,7 @@ impl InstallationContext { package_id: &PackageId, package_realm: Realm, dependencies: impl IntoIterator, - dependencies_realm: Realm, + resolved: &Resolve, ) -> anyhow::Result<()> { log::debug!("Writing package links for {}", package_id); @@ -256,6 +256,7 @@ impl InstallationContext { fs::create_dir_all(&base_path)?; for (dep_name, dep_package_id) in dependencies { + let dependencies_realm = resolved.metadata.get(dep_package_id).unwrap().origin_realm; let path = base_path.join(format!("{}.lua", dep_name)); let contents = match (package_realm, dependencies_realm) { diff --git a/src/resolution.rs b/src/resolution.rs index 5ff83697..6dcb493f 100644 --- a/src/resolution.rs +++ b/src/resolution.rs @@ -49,6 +49,9 @@ impl Resolve { } /// A single node in the package resolution graph. +/// Origin realm is the "most restrictive" realm the package can still be dependended +/// upon. It is where the package gets placed during install. +/// See [ origin_realm clarification ]. In the resolve function for more info. #[derive(Debug, Serialize)] pub struct ResolvePackageMetadata { pub realm: Realm, @@ -132,11 +135,14 @@ pub fn resolve( .get_mut(package_id) .expect("activated package was missing metadata"); - // We want to set the origin to the least restrictive origin possible. + // [ origin_realm clarification ] + // We want to set the origin to the most restrictive origin possible. // For example we want to keep packages in the dev realm unless a dependency // with a shared/server origin requires it. This way server/shared dependencies // which only originate from dev dependencies get put into the dev folder even - // if they usually belong to another realm. + // if they usually belong to another realm. Likewise we want to keep shared + // dependencies in the server realm unless they are explicitly required as a + // shared dependency. let realm_match = match (metadata.origin_realm, dependency_request.origin_realm) { (_, Realm::Shared) => Realm::Shared, (Realm::Shared, _) => Realm::Shared, diff --git a/test-projects/dev-dependency-also-required-as-non-dev/default.project.json b/test-projects/dev-dependency-also-required-as-non-dev/default.project.json new file mode 100644 index 00000000..0beb06ef --- /dev/null +++ b/test-projects/dev-dependency-also-required-as-non-dev/default.project.json @@ -0,0 +1,6 @@ +{ + "name": "dev-dependency", + "tree": { + "$path": "src" + } +} \ No newline at end of file diff --git a/test-projects/dev-dependency-also-required-as-non-dev/src/init.lua b/test-projects/dev-dependency-also-required-as-non-dev/src/init.lua new file mode 100644 index 00000000..facc02ae --- /dev/null +++ b/test-projects/dev-dependency-also-required-as-non-dev/src/init.lua @@ -0,0 +1,5 @@ +local Minimal = require(script.Parent.Minimal) + +return function() + print(Minimal) +end \ No newline at end of file diff --git a/test-projects/dev-dependency-also-required-as-non-dev/wally.toml b/test-projects/dev-dependency-also-required-as-non-dev/wally.toml new file mode 100644 index 00000000..7be707d1 --- /dev/null +++ b/test-projects/dev-dependency-also-required-as-non-dev/wally.toml @@ -0,0 +1,15 @@ +[package] +name = "biff/dev-dependency-also-required-as-non-dev" +version = "0.1.0" +license = "MIT" +realm = "server" +registry = "test-registries/primary-registry" + +[place] +server-packages = "game.ServerScriptStorage.Packages" + +[server-dependencies] +Transitive = "biff/transitive-dependency@0.1.0" + +[dev-dependencies] +Minimal = "biff/minimal@0.1.0" diff --git a/test-projects/dev-dependency/wally.toml b/test-projects/dev-dependency/wally.toml index 2ee2b8cc..df73c9fa 100644 --- a/test-projects/dev-dependency/wally.toml +++ b/test-projects/dev-dependency/wally.toml @@ -8,9 +8,5 @@ registry = "test-registries/primary-registry" [place] server-packages = "game.ServerScriptStorage.Packages" -[server-dependencies] -Minimal = "biff/minimal@0.1.0" -One = "biff/one-dependency@0.1.0" - [dev-dependencies] Transitive = "biff/transitive-dependency@0.1.0" diff --git a/tests/integration/install.rs b/tests/integration/install.rs index 0e6db16a..795df36f 100644 --- a/tests/integration/install.rs +++ b/tests/integration/install.rs @@ -30,6 +30,11 @@ fn dev_dependency() { run_test("dev-dependency"); } +#[test] +fn dev_dependency_also_required_as_non_dev() { + run_test("dev-dependency-also-required-as-non-dev"); +} + #[test] fn cross_realm_dependency() { run_test("cross-realm-dependency"); diff --git a/tests/integration/snapshots/integration__install__dev_dependency.snap b/tests/integration/snapshots/integration__install__dev_dependency.snap index 7a9157e2..99466925 100644 --- a/tests/integration/snapshots/integration__install__dev_dependency.snap +++ b/tests/integration/snapshots/integration__install__dev_dependency.snap @@ -4,14 +4,6 @@ expression: result --- DevPackages: Transitive.lua: "return require(script.Parent._Index[\"biff_transitive-dependency@0.1.0\"][\"transitive-dependency\"])\n" - _Index: - biff_transitive-dependency@0.1.0: - OneDependency.lua: "if not game:GetService(\"RunService\"):IsServer() then\n error(\"biff_one-dependency@0.1.0 is a server-only package.\", 2)\nend\n\nreturn require(game.ServerScriptStorage.Packages._Index[\"biff_one-dependency@0.1.0\"][\"one-dependency\"])\n" - transitive-dependency: - init.lua: "return \"hey\"" -ServerPackages: - Minimal.lua: "return require(script.Parent._Index[\"biff_minimal@0.1.0\"][\"minimal\"])\n" - One.lua: "return require(script.Parent._Index[\"biff_one-dependency@0.1.0\"][\"one-dependency\"])\n" _Index: biff_minimal@0.1.0: minimal: @@ -20,9 +12,13 @@ ServerPackages: Minimal.lua: "return require(script.Parent.Parent[\"biff_minimal@0.1.0\"][\"minimal\"])\n" one-dependency: init.lua: "return \"hey\"" + biff_transitive-dependency@0.1.0: + OneDependency.lua: "return require(script.Parent.Parent[\"biff_one-dependency@0.1.0\"][\"one-dependency\"])\n" + transitive-dependency: + init.lua: "return \"hey\"" default.project.json: "{\n\t\"name\": \"dev-dependency\",\n\t\"tree\": {\n\t\t\"$path\": \"src\"\n\t}\n}" src: init.lua: "local Minimal = require(script.Parent.Minimal)\n\nreturn function()\n\tprint(Minimal)\nend" -wally.lock: "# This file is automatically @generated by Wally.\n# It is not intended for manual editing.\nregistry = \"test\"\n\n[[package]]\nname = \"biff/dev-dependency\"\nversion = \"0.1.0\"\ndependencies = [[\"Minimal\", \"biff/minimal@0.1.0\"], [\"One\", \"biff/one-dependency@0.1.0\"], [\"Transitive\", \"biff/transitive-dependency@0.1.0\"]]\n\n[[package]]\nname = \"biff/minimal\"\nversion = \"0.1.0\"\ndependencies = []\n\n[[package]]\nname = \"biff/one-dependency\"\nversion = \"0.1.0\"\ndependencies = [[\"Minimal\", \"biff/minimal@0.1.0\"]]\n\n[[package]]\nname = \"biff/transitive-dependency\"\nversion = \"0.1.0\"\ndependencies = [[\"OneDependency\", \"biff/one-dependency@0.1.0\"]]\n" -wally.toml: "[package]\nname = \"biff/dev-dependency\"\nversion = \"0.1.0\"\nlicense = \"MIT\"\nrealm = \"server\"\nregistry = \"test-registries/primary-registry\"\n\n[place]\nserver-packages = \"game.ServerScriptStorage.Packages\"\n\n[server-dependencies]\nMinimal = \"biff/minimal@0.1.0\"\nOne = \"biff/one-dependency@0.1.0\"\n\n[dev-dependencies]\nTransitive = \"biff/transitive-dependency@0.1.0\"\n" +wally.lock: "# This file is automatically @generated by Wally.\n# It is not intended for manual editing.\nregistry = \"test\"\n\n[[package]]\nname = \"biff/dev-dependency\"\nversion = \"0.1.0\"\ndependencies = [[\"Transitive\", \"biff/transitive-dependency@0.1.0\"]]\n\n[[package]]\nname = \"biff/minimal\"\nversion = \"0.1.0\"\ndependencies = []\n\n[[package]]\nname = \"biff/one-dependency\"\nversion = \"0.1.0\"\ndependencies = [[\"Minimal\", \"biff/minimal@0.1.0\"]]\n\n[[package]]\nname = \"biff/transitive-dependency\"\nversion = \"0.1.0\"\ndependencies = [[\"OneDependency\", \"biff/one-dependency@0.1.0\"]]\n" +wally.toml: "[package]\nname = \"biff/dev-dependency\"\nversion = \"0.1.0\"\nlicense = \"MIT\"\nrealm = \"server\"\nregistry = \"test-registries/primary-registry\"\n\n[place]\nserver-packages = \"game.ServerScriptStorage.Packages\"\n\n[dev-dependencies]\nTransitive = \"biff/transitive-dependency@0.1.0\"\n" diff --git a/tests/integration/snapshots/integration__install__dev_dependency_also_required_as_non_dev.snap b/tests/integration/snapshots/integration__install__dev_dependency_also_required_as_non_dev.snap new file mode 100644 index 00000000..f150e119 --- /dev/null +++ b/tests/integration/snapshots/integration__install__dev_dependency_also_required_as_non_dev.snap @@ -0,0 +1,26 @@ +--- +source: tests/integration/install.rs +expression: result +--- +DevPackages: + Minimal.lua: "return require(game.ServerScriptStorage.Packages._Index[\"biff_minimal@0.1.0\"][\"minimal\"])\n" +ServerPackages: + Transitive.lua: "return require(script.Parent._Index[\"biff_transitive-dependency@0.1.0\"][\"transitive-dependency\"])\n" + _Index: + biff_minimal@0.1.0: + minimal: + init.lua: "return \"hey\"" + biff_one-dependency@0.1.0: + Minimal.lua: "return require(script.Parent.Parent[\"biff_minimal@0.1.0\"][\"minimal\"])\n" + one-dependency: + init.lua: "return \"hey\"" + biff_transitive-dependency@0.1.0: + OneDependency.lua: "return require(script.Parent.Parent[\"biff_one-dependency@0.1.0\"][\"one-dependency\"])\n" + transitive-dependency: + init.lua: "return \"hey\"" +default.project.json: "{\n\t\"name\": \"dev-dependency\",\n\t\"tree\": {\n\t\t\"$path\": \"src\"\n\t}\n}" +src: + init.lua: "local Minimal = require(script.Parent.Minimal)\n\nreturn function()\n\tprint(Minimal)\nend" +wally.lock: "# This file is automatically @generated by Wally.\n# It is not intended for manual editing.\nregistry = \"test\"\n\n[[package]]\nname = \"biff/dev-dependency-also-required-as-non-dev\"\nversion = \"0.1.0\"\ndependencies = [[\"Transitive\", \"biff/transitive-dependency@0.1.0\"], [\"Minimal\", \"biff/minimal@0.1.0\"]]\n\n[[package]]\nname = \"biff/minimal\"\nversion = \"0.1.0\"\ndependencies = []\n\n[[package]]\nname = \"biff/one-dependency\"\nversion = \"0.1.0\"\ndependencies = [[\"Minimal\", \"biff/minimal@0.1.0\"]]\n\n[[package]]\nname = \"biff/transitive-dependency\"\nversion = \"0.1.0\"\ndependencies = [[\"OneDependency\", \"biff/one-dependency@0.1.0\"]]\n" +wally.toml: "[package]\nname = \"biff/dev-dependency-also-required-as-non-dev\"\nversion = \"0.1.0\"\nlicense = \"MIT\"\nrealm = \"server\"\nregistry = \"test-registries/primary-registry\"\n\n[place]\nserver-packages = \"game.ServerScriptStorage.Packages\"\n\n[server-dependencies]\nTransitive = \"biff/transitive-dependency@0.1.0\"\n\n[dev-dependencies]\nMinimal = \"biff/minimal@0.1.0\"\n" +