Skip to content

Commit

Permalink
fix(pacmak): generated dependencies are arbitrary
Browse files Browse the repository at this point in the history
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 declarations of the entire
  (transitive) dependency closure of the package into the `.jsii` file.
  Instead, only direct dependencies are represented. This is fine
  because `jsii` already demands that all assembliesof which a type is
  re-exported be declared (at least) as a `peerDependency`, and the
  change also ensures that `peerDependency` declarations are considered
  in the same way that regular `dependencies` are.
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.
3. `jsii-pacmak` renders the declared SemVer range in the relevant form
  for the target being generated.

Fixes #676
Related to #1137
  • Loading branch information
RomainMuller committed Dec 18, 2019
1 parent 5f8a27f commit e335466
Show file tree
Hide file tree
Showing 28 changed files with 177 additions and 475 deletions.
12 changes: 3 additions & 9 deletions packages/@jsii/spec/lib/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,6 @@ export interface Assembly extends Documentable {
*/
dependencies?: { [assembly: string]: PackageVersion };

/**
* Closure of all dependency assemblies, direct and transitive.
*
* @default none
*/
dependencyClosure?: { [assembly: string]: PackageVersion };

/**
* List if bundled dependencies (these are not expected to be jsii
* assemblies).
Expand Down Expand Up @@ -219,14 +212,15 @@ export interface AssemblyTargets {
*/
export interface PackageVersion {
/**
* Version of the package.
* The declared version of this dependency. This may be a semver range.
*
* @minLength 1
*/
version: string;

/**
* Targets for a given assembly.
* Targets configured for a given assembly. Useful when needing to determine
* names of dependencies to generate in absence of the actual assembly.
*
* @default none
*/
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.11",
Expand Down
27 changes: 1 addition & 26 deletions packages/@scope/jsii-calc-base/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -32,31 +32,6 @@
"version": "0.20.11"
}
},
"dependencyClosure": {
"@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"
}
},
"description": "An example direct dependency for jsii-calc.",
"homepage": "https://github.com/aws/jsii",
"jsiiVersion": "0.20.11",
Expand Down Expand Up @@ -174,5 +149,5 @@
}
},
"version": "0.20.11",
"fingerprint": "lSQlk5mMPd7fxaZoCdCekSFY4rDOAu3VnLuIcFUXA6o="
"fingerprint": "tjQFbb9hxfsDXAot1ihz85sbx0EdlUmqSfRXt+WfahY="
}
52 changes: 2 additions & 50 deletions packages/@scope/jsii-calc-lib/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -29,55 +29,7 @@
"module": "scope.jsii_calc_base"
}
},
"version": "0.20.11"
}
},
"dependencyClosure": {
"@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"
"version": "^0.20.11"
}
},
"description": "A simple calcuator library built on JSII.",
Expand Down Expand Up @@ -539,5 +491,5 @@
}
},
"version": "0.20.11",
"fingerprint": "D1VJJVqcvEKInLQ3r/WxIN332Yo1IRlVWkCPByyIIb0="
"fingerprint": "mexN5AIs1N8eRcRRklY0n6B8cda9Zl8jqBFlCext8c8="
}
81 changes: 4 additions & 77 deletions packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
"module": "scope.jsii_calc_base"
}
},
"version": "0.20.11"
"version": "^0.20.11"
},
"@scope/jsii-calc-base-of-base": {
"targets": {
Expand All @@ -78,7 +78,7 @@
"module": "scope.jsii_calc_base_of_base"
}
},
"version": "0.20.11"
"version": "^0.20.11"
},
"@scope/jsii-calc-lib": {
"targets": {
Expand All @@ -103,80 +103,7 @@
"module": "scope.jsii_calc_lib"
}
},
"version": "0.20.11"
}
},
"dependencyClosure": {
"@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"
"version": "^0.20.11"
}
},
"description": "A simple calcuator built on JSII.",
Expand Down Expand Up @@ -12002,5 +11929,5 @@
}
},
"version": "0.20.11",
"fingerprint": "eenqZx1+dNMvfZCY02AIrinXeK98iQC4XLLj0yPIXuM="
"fingerprint": "4NkZ2cKdxrdzq1duPFnWy7f3Zqg75tgCIzPOV1BRhRI="
}
4 changes: 2 additions & 2 deletions packages/jsii-pacmak/lib/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -509,13 +509,13 @@ export abstract class Generator implements IGenerator {
return this.assembly;
}

const found = (this.assembly.dependencyClosure ?? {})[name];
const found = (this.assembly.dependencies ?? {})[name];

if (found) {
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
46 changes: 44 additions & 2 deletions packages/jsii-pacmak/lib/targets/dotnet/filegenerator.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { CodeMaker } from 'codemaker';
import { Assembly } from '@jsii/spec';
import * as path from 'path';
import * as semver from 'semver';
import * as xmlbuilder from 'xmlbuilder';
import { DotNetNameUtils } from './nameutils';
import * as logging from '../../logging';
Expand All @@ -9,12 +10,53 @@ import { TARGET_FRAMEWORK } from '../dotnet';

// Represents a dependency in the dependency tree.
export class DotNetDependency {
public readonly version: string;

public constructor(
public readonly namespace: string,
public readonly packageId: string,
public readonly fqn: string,
public readonly version: string,
public readonly partOfCompilation: boolean) {
version: string,
public readonly partOfCompilation: boolean
) {
const range = new semver.Range(version);
// See: https://docs.microsoft.com/en-us/nuget/concepts/package-versioning#version-ranges-and-wildcards
this.version = range.set.map(set => {
if (set.length === 1) {
switch (set[0].operator || '=') {
// "[version]" => means exactly version
case '=': return `[${set[0].semver.raw}]`;
// "(version,]" => means greater than version
case '>': return `(${set[0].semver.raw},]`;
// "[version,]" => means greater than or equal to that version
case '>=': return `[${set[0].semver.raw},]`;
// "[,version)" => means less than version
case '<': return `[,${set[0].semver.raw})`;
// "[,version]" => means less than or equal to version
case '<=': return `[,${set[0].semver.raw}]`;
}
} else if (set.length === 2) {
const nugetRange = this.toNuGetRange(set[0], set[1]);
if (nugetRange) {
return nugetRange;
}
}
throw new Error(`Unsupported SemVer range set: ${set.map(comp => comp.value).join(', ')}`);
}).join(', ');
}

private toNuGetRange(left: semver.Comparator, right: semver.Comparator): string | undefined {
if (left.operator.startsWith('<') && right.operator.startsWith('>')) {
// Order isn't ideal, just re-invoke with the correct ordering...
return this.toNuGetRange(right, left);
}
if (!left.operator.startsWith('>') || !right.operator.startsWith('<')) {
// We only support ranges defined like "> (or >=) left, < (or <=) right"
return undefined;
}
const leftBrace = left.operator.endsWith('=') ? '[' : '(';
const rightBrace = right.operator.endsWith('=') ? ']' : ')';
return `${leftBrace}${left.semver.raw},${right.semver.raw}${rightBrace}`;
}
}

Expand Down
Loading

0 comments on commit e335466

Please sign in to comment.