Skip to content

Commit

Permalink
fix(pacmak): generated dependencies are not consistent with source np…
Browse files Browse the repository at this point in the history
…m module (#1141)

* 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

* pr review feedback actions

* add missing file

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
RomainMuller and mergify[bot] committed Dec 27, 2019
1 parent dafba48 commit 03221fe
Show file tree
Hide file tree
Showing 41 changed files with 409 additions and 506 deletions.
52 changes: 20 additions & 32 deletions packages/@jsii/spec/lib/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/@jsii/spec/lib/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export * from './assembly';
export * from './configuration';
export * from './name-tree';
export * from './validate-assembly';
export * from './configuration';
4 changes: 2 additions & 2 deletions packages/@scope/jsii-calc-base/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.13",
Expand Down
29 changes: 3 additions & 26 deletions packages/@scope/jsii-calc-base/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -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.",
Expand Down Expand Up @@ -174,5 +151,5 @@
}
},
"version": "0.20.11",
"fingerprint": "lSQlk5mMPd7fxaZoCdCekSFY4rDOAu3VnLuIcFUXA6o="
"fingerprint": "vaUHiWCfpSsCfXz18WPVQ3HhCBTPj23pDky4IqmEWxw="
}
32 changes: 4 additions & 28 deletions packages/@scope/jsii-calc-lib/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -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": {
Expand All @@ -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.",
Expand Down Expand Up @@ -540,5 +516,5 @@
}
},
"version": "0.20.11",
"fingerprint": "WSHD7tywHgFC9jmImvDuy7NGYCDWAAb49jk0ZPzCoFM="
"fingerprint": "tdRWsMWbPQxej79pQ39gbrrViv/Wl6vfTLY59IJPi4w="
}
85 changes: 7 additions & 78 deletions packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -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": {
Expand All @@ -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": {
Expand All @@ -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.",
Expand Down Expand Up @@ -12093,5 +12022,5 @@
}
},
"version": "0.20.11",
"fingerprint": "5M7u8+bWxu4amY/gs4Z/K4IpYqJCMGtnQiVoerS4uac="
"fingerprint": "j7Hsf1Hd/mih3Gr3UR7vYcXeLrbixxffBTzvSokikjM="
}
4 changes: 2 additions & 2 deletions packages/jsii-pacmak/lib/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 {
Expand Down
17 changes: 17 additions & 0 deletions packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,30 @@ export class DotNetGenerator extends Generator {
await fs.mkdirp(path.join(outdir, packageId));
await fs.copyFile(tarball, path.join(outdir, packageId, tarballFileName));

// Create an anchor file for the current model
this.generateDependencyAnchorFile();

// Copying the .jsii file
await fs.copyFile(this.jsiiFilePath, path.join(outdir, packageId, spec.SPEC_FILE_NAME));

// Saving the generated code.
return this.code.save(outdir);
}

/**
* Generates the anchor file
*/
protected generateDependencyAnchorFile() {
const namespace = `${this.assembly.targets!.dotnet!.namespace}.Internal.DependencyResolution`;
this.openFileIfNeeded('Anchor', namespace, false, false);
this.code.openBlock('public sealed class Anchor');
this.code.openBlock('public Anchor()');
this.typeresolver.namespaceDependencies.forEach(value => this.code.line(`new ${value.namespace}.Internal.DependencyResolution.Anchor();`));
this.code.closeBlock();
this.code.closeBlock();
this.closeFileIfNeeded('Anchor', namespace, false);
}

/**
* Not used as we override the save() method
*/
Expand Down
Loading

0 comments on commit 03221fe

Please sign in to comment.