fix(pacmak): generated dependencies are arbitrary

The dependency statements generated in the Java, .NET and Python packages generated by `jsii-pacmak` were not reflecting the dependency statement modeled on the source package. In certain pathological cases, such as Python, the dependency declaration was often more permissive than what the source package allowed, resulting in surprising behavior as well as difficult to troubleshoot problems. This updates several elements involved in this problem: 1. The `jsii` compiler no longer carries version information for transitive dependencies of the assembly in the `dependencyClosure` property, and instead only carries the `targets` configuration for those. 2. The version statements in the `dependencies` section of the `.jsii` file no longer contains the "compile-time resolved" version number, but the actual SemVer expression declared in the package's source. Also, the `targets` object was dropped (only the SemVer range statement remains), as this incurred needless duplication of the target configuration (in `dependencies` and `dependencyClosure`). 3. `jsii-pacmak` renders the declared SemVer range in the relevant form for the target being generated. Fixes #676 Related to #1137
aws · Dec 18, 2019 · 009ebeb · 009ebeb
1 parent 5f8a27f
commit 009ebeb
Show file tree

Hide file tree

Showing 30 changed files with 245 additions and 499 deletions.
diff --git a/packages/@jsii/spec/lib/assembly.ts b/packages/@jsii/spec/lib/assembly.ts
@@ -3,7 +3,7 @@ export const SPEC_FILE_NAME = '.jsii';
 /**
  * A JSII assembly specification.
  */
-export interface Assembly extends Documentable {
+export interface Assembly extends AssemblyConfiguration, Documentable {
   /**
    * The version of the spec schema
    */
@@ -88,14 +88,6 @@ export interface Assembly extends Documentable {
    */
   license: string;
 
-  /**
-   * A map of target name to configuration, which is used when generating
-   * packages for various languages.
-   *
-   * @default none
-   */
-  targets?: AssemblyTargets;
-
   /**
    * Arbitrary key-value pairs of metadata, which the maintainer chose to
    * document with the assembly. These entries do not carry normative
@@ -115,18 +107,20 @@ export interface Assembly extends Documentable {
 
   /**
    * Direct dependencies on other assemblies (with semver), the key is the JSII
-   * assembly name.
+   * assembly name, and the value is a SemVer expression..
    *
    * @default none
    */
-  dependencies?: { [assembly: string]: PackageVersion };
+  dependencies?: { [assembly: string]: string };
 
   /**
-   * Closure of all dependency assemblies, direct and transitive.
+   * Target configuration for all the assemblies that are direct or transitive
+   * dependencies of this assembly. This is needed to generate correct native
+   * type names for any transitively inherited member, in certain languages.
    *
    * @default none
    */
-  dependencyClosure?: { [assembly: string]: PackageVersion };
+  dependencyClosure?: { [assembly: string]: AssemblyConfiguration };
 
   /**
    * List if bundled dependencies (these are not expected to be jsii
@@ -151,6 +145,19 @@ export interface Assembly extends Documentable {
   readme?: { markdown: string };
 }
 
+/**
+ * Shareable configuration of a jsii Assembly.
+ */
+export interface AssemblyConfiguration {
+  /**
+   * A map of target name to configuration, which is used when generating
+   * packages for various languages.
+   *
+   * @default none
+   */
+  targets?: AssemblyTargets;
+}
+
 /**
  * Versions of the JSII Assembly Specification.
  */
@@ -214,25 +221,6 @@ export interface AssemblyTargets {
   [language: string]: { [key: string]: any } | undefined;
 }
 
-/**
- * The version of a package.
- */
-export interface PackageVersion {
-  /**
-   * Version of the package.
-   *
-   * @minLength 1
-   */
-  version: string;
-
-  /**
-   * Targets for a given assembly.
-   *
-   * @default none
-   */
-  targets?: AssemblyTargets;
-}
-
 /**
  * Where in the module source the definition for this API item was found
  */

diff --git a/packages/@jsii/spec/lib/index.ts b/packages/@jsii/spec/lib/index.ts
@@ -1,4 +1,4 @@
 export * from './assembly';
+export * from './configuration';
 export * from './name-tree';
 export * from './validate-assembly';
-export * from './configuration';
diff --git a/packages/@scope/jsii-calc-base/package.json b/packages/@scope/jsii-calc-base/package.json
@@ -29,10 +29,10 @@
     "test:update": "npm run build && UPDATE_DIFF=1 npm run test"
   },
   "dependencies": {
-    "@scope/jsii-calc-base-of-base": "^0.20.11"
+    "@scope/jsii-calc-base-of-base": "0.20.11"
   },
   "peerDependencies": {
-    "@scope/jsii-calc-base-of-base": "^0.20.11"
+    "@scope/jsii-calc-base-of-base": "0.20.11"
   },
   "devDependencies": {
     "@types/node": "^10.17.11",

diff --git a/packages/@scope/jsii-calc-base/test/assembly.jsii b/packages/@scope/jsii-calc-base/test/assembly.jsii
@@ -8,29 +8,7 @@
     "url": "https://aws.amazon.com"
   },
   "dependencies": {
-    "@scope/jsii-calc-base-of-base": {
-      "targets": {
-        "dotnet": {
-          "namespace": "Amazon.JSII.Tests.CalculatorNamespace.BaseOfBaseNamespace",
-          "packageId": "Amazon.JSII.Tests.CalculatorPackageId.BaseOfBasePackageId"
-        },
-        "java": {
-          "maven": {
-            "artifactId": "calculator-base-of-base",
-            "groupId": "software.amazon.jsii.tests"
-          },
-          "package": "software.amazon.jsii.tests.calculator.baseofbase"
-        },
-        "js": {
-          "npm": "@scope/jsii-calc-base-of-base"
-        },
-        "python": {
-          "distName": "scope.jsii-calc-base-of-base",
-          "module": "scope.jsii_calc_base_of_base"
-        }
-      },
-      "version": "0.20.11"
-    }
+    "@scope/jsii-calc-base-of-base": "0.20.11"
   },
   "dependencyClosure": {
     "@scope/jsii-calc-base-of-base": {
@@ -53,8 +31,7 @@
           "distName": "scope.jsii-calc-base-of-base",
           "module": "scope.jsii_calc_base_of_base"
         }
-      },
-      "version": "0.20.11"
+      }
     }
   },
   "description": "An example direct dependency for jsii-calc.",
@@ -174,5 +151,5 @@
     }
   },
   "version": "0.20.11",
-  "fingerprint": "lSQlk5mMPd7fxaZoCdCekSFY4rDOAu3VnLuIcFUXA6o="
+  "fingerprint": "vaUHiWCfpSsCfXz18WPVQ3HhCBTPj23pDky4IqmEWxw="
 }
diff --git a/packages/@scope/jsii-calc-lib/test/assembly.jsii b/packages/@scope/jsii-calc-lib/test/assembly.jsii
@@ -8,29 +8,7 @@
     "url": "https://aws.amazon.com"
   },
   "dependencies": {
-    "@scope/jsii-calc-base": {
-      "targets": {
-        "dotnet": {
-          "namespace": "Amazon.JSII.Tests.CalculatorNamespace.BaseNamespace",
-          "packageId": "Amazon.JSII.Tests.CalculatorPackageId.BasePackageId"
-        },
-        "java": {
-          "maven": {
-            "artifactId": "calculator-base",
-            "groupId": "software.amazon.jsii.tests"
-          },
-          "package": "software.amazon.jsii.tests.calculator.base"
-        },
-        "js": {
-          "npm": "@scope/jsii-calc-base"
-        },
-        "python": {
-          "distName": "scope.jsii-calc-base",
-          "module": "scope.jsii_calc_base"
-        }
-      },
-      "version": "0.20.11"
-    }
+    "@scope/jsii-calc-base": "^0.20.11"
   },
   "dependencyClosure": {
     "@scope/jsii-calc-base": {
@@ -53,8 +31,7 @@
           "distName": "scope.jsii-calc-base",
           "module": "scope.jsii_calc_base"
         }
-      },
-      "version": "0.20.11"
+      }
     },
     "@scope/jsii-calc-base-of-base": {
       "targets": {
@@ -76,8 +53,7 @@
           "distName": "scope.jsii-calc-base-of-base",
           "module": "scope.jsii_calc_base_of_base"
         }
-      },
-      "version": "0.20.11"
+      }
     }
   },
   "description": "A simple calcuator library built on JSII.",
@@ -539,5 +515,5 @@
     }
   },
   "version": "0.20.11",
-  "fingerprint": "D1VJJVqcvEKInLQ3r/WxIN332Yo1IRlVWkCPByyIIb0="
+  "fingerprint": "c16s32RYOhQkY/FmEPgjMN+m4/lpF3Rj5a9nsUQpf6Q="
 }
diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii
@@ -34,77 +34,9 @@
     }
   ],
   "dependencies": {
-    "@scope/jsii-calc-base": {
-      "targets": {
-        "dotnet": {
-          "namespace": "Amazon.JSII.Tests.CalculatorNamespace.BaseNamespace",
-          "packageId": "Amazon.JSII.Tests.CalculatorPackageId.BasePackageId"
-        },
-        "java": {
-          "maven": {
-            "artifactId": "calculator-base",
-            "groupId": "software.amazon.jsii.tests"
-          },
-          "package": "software.amazon.jsii.tests.calculator.base"
-        },
-        "js": {
-          "npm": "@scope/jsii-calc-base"
-        },
-        "python": {
-          "distName": "scope.jsii-calc-base",
-          "module": "scope.jsii_calc_base"
-        }
-      },
-      "version": "0.20.11"
-    },
-    "@scope/jsii-calc-base-of-base": {
-      "targets": {
-        "dotnet": {
-          "namespace": "Amazon.JSII.Tests.CalculatorNamespace.BaseOfBaseNamespace",
-          "packageId": "Amazon.JSII.Tests.CalculatorPackageId.BaseOfBasePackageId"
-        },
-        "java": {
-          "maven": {
-            "artifactId": "calculator-base-of-base",
-            "groupId": "software.amazon.jsii.tests"
-          },
-          "package": "software.amazon.jsii.tests.calculator.baseofbase"
-        },
-        "js": {
-          "npm": "@scope/jsii-calc-base-of-base"
-        },
-        "python": {
-          "distName": "scope.jsii-calc-base-of-base",
-          "module": "scope.jsii_calc_base_of_base"
-        }
-      },
-      "version": "0.20.11"
-    },
-    "@scope/jsii-calc-lib": {
-      "targets": {
-        "dotnet": {
-          "namespace": "Amazon.JSII.Tests.CalculatorNamespace.LibNamespace",
-          "packageId": "Amazon.JSII.Tests.CalculatorPackageId.LibPackageId",
-          "versionSuffix": "-devpreview"
-        },
-        "java": {
-          "maven": {
-            "artifactId": "calculator-lib",
-            "groupId": "software.amazon.jsii.tests",
-            "versionSuffix": ".DEVPREVIEW"
-          },
-          "package": "software.amazon.jsii.tests.calculator.lib"
-        },
-        "js": {
-          "npm": "@scope/jsii-calc-lib"
-        },
-        "python": {
-          "distName": "scope.jsii-calc-lib",
-          "module": "scope.jsii_calc_lib"
-        }
-      },
-      "version": "0.20.11"
-    }
+    "@scope/jsii-calc-base": "^0.20.11",
+    "@scope/jsii-calc-base-of-base": "^0.20.11",
+    "@scope/jsii-calc-lib": "^0.20.11"
   },
   "dependencyClosure": {
     "@scope/jsii-calc-base": {
@@ -127,8 +59,7 @@
           "distName": "scope.jsii-calc-base",
           "module": "scope.jsii_calc_base"
         }
-      },
-      "version": "0.20.11"
+      }
     },
     "@scope/jsii-calc-base-of-base": {
       "targets": {
@@ -150,8 +81,7 @@
           "distName": "scope.jsii-calc-base-of-base",
           "module": "scope.jsii_calc_base_of_base"
         }
-      },
-      "version": "0.20.11"
+      }
     },
     "@scope/jsii-calc-lib": {
       "targets": {
@@ -175,8 +105,7 @@
           "distName": "scope.jsii-calc-lib",
           "module": "scope.jsii_calc_lib"
         }
-      },
-      "version": "0.20.11"
+      }
     }
   },
   "description": "A simple calcuator built on JSII.",
@@ -12002,5 +11931,5 @@
     }
   },
   "version": "0.20.11",
-  "fingerprint": "eenqZx1+dNMvfZCY02AIrinXeK98iQC4XLLj0yPIXuM="
+  "fingerprint": "JMTWeMHDPnC/01VAtTulHbaZ1mSgip2tVym5dlwrBZE="
 }
diff --git a/packages/jsii-pacmak/lib/generator.ts b/packages/jsii-pacmak/lib/generator.ts
@@ -502,7 +502,7 @@ export abstract class Generator implements IGenerator {
      * Looks up a jsii module in the dependency tree.
      * @param name The name of the jsii module to look up
      */
-  protected findModule(name: string): spec.PackageVersion {
+  protected findModule(name: string): spec.AssemblyConfiguration {
 
     // if this is the current module, return it
     if (this.assembly.name === name) {
@@ -515,7 +515,7 @@ export abstract class Generator implements IGenerator {
       return found;
     }
 
-    throw new Error(`Unable to find module ${name} as a direct or indirect dependency of ${this.assembly.name}`);
+    throw new Error(`Unable to find module ${name} as a dependency of ${this.assembly.name}`);
   }
 
   protected findType(fqn: string): spec.Type {

diff --git a/packages/jsii-pacmak/lib/targets/dotnet/dotnettyperesolver.ts b/packages/jsii-pacmak/lib/targets/dotnet/dotnettyperesolver.ts
@@ -2,7 +2,7 @@ import * as spec from '@jsii/spec';
 import { DotNetDependency } from './filegenerator';
 import { DotNetNameUtils } from './nameutils';
 
-type FindModuleCallback = (fqn: string) => spec.Assembly | spec.PackageVersion;
+type FindModuleCallback = (fqn: string) => spec.AssemblyConfiguration;
 type FindTypeCallback = (fqn: string) => spec.Type;
 
 export class DotNetTypeResolver {
@@ -72,23 +72,21 @@ export class DotNetTypeResolver {
      */
   public resolveNamespacesDependencies(): void {
     const assmDependencies = this.assembly.dependencies ?? {};
-    for (const depName of Object.keys(assmDependencies)) {
-      const depInfo = assmDependencies[depName];
+    const assmConfigurations = this.assembly.dependencyClosure ?? {};
+    for (const [depName, version] of Object.entries(assmDependencies)) {
+      const depInfo = assmConfigurations[depName];
       if (!this.namespaceDependencies.has(depName)) {
-        const dotnetInfo = depInfo.targets!.dotnet;
-        const namespace = dotnetInfo!.namespace;
-        const packageId = dotnetInfo!.packageId;
-        let version = depInfo.version;
+        const dotnetInfo = depInfo.targets!.dotnet!;
+        const namespace = dotnetInfo.namespace;
+        const packageId = dotnetInfo.packageId;
         const suffix = depInfo.targets!.dotnet!.versionSuffix;
-        if (suffix) {
-          // suffix is guaranteed to start with a leading `-`
-          version = `${depInfo.version}${suffix}`;
-        }
+
         this.namespaceDependencies.set(depName, new DotNetDependency(
           namespace,
           packageId,
           depName,
-          version,
+          // suffix, when present, is guaranteed to start with a leading `-`
+          suffix ? `${version}${suffix}` : version,
           this.assembliesCurrentlyBeingCompiled.includes(depName)));
       }
     }