Skip to content

Commit

Permalink
fix(pacmak): .NET build downloading packages from NuGet (#949)
Browse files Browse the repository at this point in the history
Fix situation where the .NET build accidentally downloads dependency
packages from NuGet, instead of using the locally built packages.

Caused by packages being built in the order given on the command line,
and if a consumer is built before a dependency, the NuGet build would
fall back to loading the package from the online repository.

Apply a topological sort to package building to prevent this from
happening.
  • Loading branch information
rix0rrr committed Nov 7, 2019
1 parent fc02abd commit 433d1f8
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 6 deletions.
20 changes: 14 additions & 6 deletions packages/jsii-pacmak/lib/npm-modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,23 @@ import { resolveDependencyDirectory } from './util';

import logging = require('../lib/logging');
import { JsiiModule } from './packaging';
import { topologicalSort } from './toposort';


/**
* Find all modules that need to be packagerd
*
* If the input list is empty, include the current directory. The result
* is NOT topologically sorted.
* If the input list is empty, include the current directory.
*
* The result is topologically sorted.
*/
export async function findJsiiModules(directories: string[], recurse: boolean) {
const ret: JsiiModule[] = [];
const visited = new Set<string>();
for (const dir of directories.length > 0 ? directories : ['.']) {
await visitPackage(dir, true);
}
return ret;
return topologicalSort(ret, m => m.name, m => m.dependencyNames);

async function visitPackage(dir: string, isRoot: boolean) {
const realPath = await fs.realpath(dir);
Expand All @@ -35,9 +37,15 @@ export async function findJsiiModules(directories: string[], recurse: boolean) {
}
}

if (!pkg.name) {
throw new Error(`package.json does not have a 'name' field: ${JSON.stringify(pkg, undefined, 2)}`);
}

const dependencyNames = Object.keys(pkg.dependencies || {});

// if --recurse is set, find dependency dirs and build them.
if (recurse) {
for (const dep of Object.keys(pkg.dependencies || {})) {
for (const dep of dependencyNames) {
const depDir = resolveDependencyDirectory(realPath, dep);
await visitPackage(depDir, false);
}
Expand All @@ -51,10 +59,10 @@ export async function findJsiiModules(directories: string[], recurse: boolean) {
name: pkg.name,
moduleDirectory: realPath,
defaultOutputDirectory: outputDirectory,
availableTargets: targets
availableTargets: targets,
dependencyNames
}));
}

}

export async function updateAllNpmIgnores(packages: JsiiModule[]) {
Expand Down
7 changes: 7 additions & 0 deletions packages/jsii-pacmak/lib/packaging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,15 @@ export interface JsiiModuleOptions {
* Output directory where to package everything
*/
defaultOutputDirectory: string;

/**
* Names of packages this package depends on, if any
*/
dependencyNames?: string[];
}
export class JsiiModule {
public readonly name: string;
public readonly dependencyNames: string[];
public readonly moduleDirectory: string;
public readonly availableTargets: string[];
public outputDirectory: string;
Expand All @@ -41,6 +47,7 @@ export class JsiiModule {
this.moduleDirectory = options.moduleDirectory;
this.availableTargets = options.availableTargets;
this.outputDirectory = options.defaultOutputDirectory;
this.dependencyNames = options.dependencyNames || [];
}

/**
Expand Down
48 changes: 48 additions & 0 deletions packages/jsii-pacmak/lib/toposort.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
export type KeyFunc<T> = (x: T) => string;
export type DepFunc<T> = (x: T) => string[];

/**
* Return a topological sort of all elements of xs, according to the given dependency functions
*
* Dependencies outside the referenced set are ignored.
*
* Not a stable sort, but in order to keep the order as stable as possible, we'll sort by key
* among elements of equal precedence.
*
* @param xs - The elements to sort
* @param keyFn - Return an element's identifier
* @param depFn - Return the identifiers of an element's dependencies
*/
export function topologicalSort<T>(xs: Iterable<T>, keyFn: KeyFunc<T>, depFn: DepFunc<T>): T[] {
const remaining = new Map<string, TopoElement<T>>();
for (const element of xs) {
const key = keyFn(element);
remaining.set(key, { key, element, dependencies: depFn(element) });
}

const ret = new Array<T>();
while (remaining.size > 0) {
// All elements with no more deps in the set can be ordered
const selectable = Array.from(remaining.values()).filter(e => e.dependencies.every(d => !remaining.has(d)));

selectable.sort((a, b) => a.key < b.key ? -1 : b.key < a.key ? 1 : 0);

for (const selected of selectable) {
ret.push(selected.element);
remaining.delete(selected.key);
}

// If we didn't make any progress, we got stuck
if (selectable.length === 0) {
throw new Error(`Could not determine ordering between: ${Array.from(remaining.keys()).join(', ')}`);
}
}

return ret;
}

interface TopoElement<T> {
key: string;
dependencies: string[];
element: T;
}
2 changes: 2 additions & 0 deletions packages/jsii-pacmak/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
"@types/fs-extra": "^8.0.1",
"@types/jest": "^24.0.22",
"@types/node": "^10.17.4",
"mock-fs": "^4.10.2",
"@types/mock-fs": "^4.10.0",
"@types/yargs": "^13.0.3",
"@typescript-eslint/eslint-plugin": "^2.6.1",
"@typescript-eslint/parser": "^2.6.1",
Expand Down
65 changes: 65 additions & 0 deletions packages/jsii-pacmak/test/npm-modules.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import mockfs = require('mock-fs');
import { findJsiiModules } from '../lib/npm-modules';

test('findJsiiModules is sorted topologically', async () => {
mockfs({
'/packageA/package.json': JSON.stringify({
name: 'packageA',
jsii: {
outdir: 'dist',
targets: {
python: {}
}
},
dependencies: {
packageB: '*'
}
}),
'/packageB/package.json': JSON.stringify({
name: 'packageB',
jsii: {
outdir: 'dist',
targets: {
python: {}
}
}
}),
});

try {
const mods = await findJsiiModules(['/packageA', '/packageB'], false);
expect(mods.map(m => m.name)).toEqual(['packageB', 'packageA']);
} finally {
mockfs.restore();
}
});

test('findJsiiModules without deps loads packages in given order', async () => {
mockfs({
'/packageA/package.json': JSON.stringify({
name: 'packageA',
jsii: {
outdir: 'dist',
targets: {
python: {}
}
},
}),
'/packageB/package.json': JSON.stringify({
name: 'packageB',
jsii: {
outdir: 'dist',
targets: {
python: {}
}
}
}),
});

try {
const mods = await findJsiiModules(['/packageA', '/packageB'], false);
expect(mods.map(m => m.name)).toEqual(['packageA', 'packageB']);
} finally {
mockfs.restore();
}
});
12 changes: 12 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,13 @@
dependencies:
"@types/node" "*"

"@types/mock-fs@^4.10.0":
version "4.10.0"
resolved "https://registry.yarnpkg.com/@types/mock-fs/-/mock-fs-4.10.0.tgz#460061b186993d76856f669d5317cda8a007c24b"
integrity sha512-FQ5alSzmHMmliqcL36JqIA4Yyn9jyJKvRSGV3mvPh108VFatX7naJDzSG4fnFQNZFq9dIx0Dzoe6ddflMB2Xkg==
dependencies:
"@types/node" "*"

"@types/node@*":
version "12.11.1"
resolved "https://registry.yarnpkg.com/@types/node/-/node-12.11.1.tgz#1fd7b821f798b7fa29f667a1be8f3442bb8922a3"
Expand Down Expand Up @@ -5414,6 +5421,11 @@ mkdirp@*, mkdirp@^0.5.0, mkdirp@^0.5.1:
dependencies:
minimist "0.0.8"

mock-fs@^4.10.2:
version "4.10.3"
resolved "https://registry.yarnpkg.com/mock-fs/-/mock-fs-4.10.3.tgz#d0550663dd2b5d33a7c1b8713c6925aab07a04ae"
integrity sha512-bcukePBvuA3qovmq0Qtqu9+1APCIGkFHnsozrPIVromt5XFGGgkQSfaN0H6RI8gStHkO/hRgimvS3tooNes4pQ==

modify-values@^1.0.0:
version "1.0.1"
resolved "https://registry.yarnpkg.com/modify-values/-/modify-values-1.0.1.tgz#b3939fa605546474e3e3e3c63d64bd43b4ee6022"
Expand Down

0 comments on commit 433d1f8

Please sign in to comment.