From c65b1d9fd1e0a6e37dad96c763c0bb1a1d15f57d Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Thu, 4 Aug 2022 11:13:24 +0200 Subject: [PATCH] feat(.net): embed package icon when configured (#3676) When an `iconUrl` is configured for a .NET target, attempt to download it for inclusion in the NuGet package with the `PackageIcon` attribute, as the `PackageIconUrl` attribute is deprecated. This feature can be opted out of by setting the `JSII_PACMAK_DOTNET_NO_DOWNLOAD_ICON` environment variable (no matter what value it has). If the download somehow fails, the previous behavior is preserved. The `PackageIconUrl` attribute is still emitted for backwards compatibility with tools that do not (yet) support `PackageIcon`. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- packages/@scope/jsii-calc-base/package.json | 3 +- .../@scope/jsii-calc-base/test/assembly.jsii | 3 +- .../@scope/jsii-calc-lib/test/assembly.jsii | 3 +- packages/jsii-calc/test/assembly.jsii | 3 +- .../lib/targets/dotnet/dotnetgenerator.ts | 131 +++++++++++++++++- .../lib/targets/dotnet/filegenerator.ts | 21 ++- .../__snapshots__/target-dotnet.test.js.snap | 19 ++- .../test/generated-code/harness.ts | 8 ++ 8 files changed, 184 insertions(+), 7 deletions(-) diff --git a/packages/@scope/jsii-calc-base/package.json b/packages/@scope/jsii-calc-base/package.json index 602e7446b3..7849d46e45 100644 --- a/packages/@scope/jsii-calc-base/package.json +++ b/packages/@scope/jsii-calc-base/package.json @@ -65,7 +65,8 @@ }, "dotnet": { "namespace": "Amazon.JSII.Tests.CalculatorNamespace.BaseNamespace", - "packageId": "Amazon.JSII.Tests.CalculatorPackageId.BasePackageId" + "packageId": "Amazon.JSII.Tests.CalculatorPackageId.BasePackageId", + "iconUrl": "https://raw.githubusercontent.com/aws/aws-cdk/main/logo/default-256-dark.png" }, "python": { "distName": "scope.jsii-calc-base", diff --git a/packages/@scope/jsii-calc-base/test/assembly.jsii b/packages/@scope/jsii-calc-base/test/assembly.jsii index 342d94bd6f..1bb56e3f7d 100644 --- a/packages/@scope/jsii-calc-base/test/assembly.jsii +++ b/packages/@scope/jsii-calc-base/test/assembly.jsii @@ -60,6 +60,7 @@ "schema": "jsii/0.10.0", "targets": { "dotnet": { + "iconUrl": "https://raw.githubusercontent.com/aws/aws-cdk/main/logo/default-256-dark.png", "namespace": "Amazon.JSII.Tests.CalculatorNamespace.BaseNamespace", "packageId": "Amazon.JSII.Tests.CalculatorPackageId.BasePackageId" }, @@ -206,5 +207,5 @@ } }, "version": "0.0.0", - "fingerprint": "DVCANvLLzJEu5VNOJVmldbT5wTXhmHlZzk3E6muTR5I=" + "fingerprint": "HiHbzAfeO1CAYQhgLFruW7OWwx9db1OpnC4Pl3Vlqak=" } \ No newline at end of file diff --git a/packages/@scope/jsii-calc-lib/test/assembly.jsii b/packages/@scope/jsii-calc-lib/test/assembly.jsii index 021f3e7e23..671057ad68 100644 --- a/packages/@scope/jsii-calc-lib/test/assembly.jsii +++ b/packages/@scope/jsii-calc-lib/test/assembly.jsii @@ -15,6 +15,7 @@ "@scope/jsii-calc-base": { "targets": { "dotnet": { + "iconUrl": "https://raw.githubusercontent.com/aws/aws-cdk/main/logo/default-256-dark.png", "namespace": "Amazon.JSII.Tests.CalculatorNamespace.BaseNamespace", "packageId": "Amazon.JSII.Tests.CalculatorPackageId.BasePackageId" }, @@ -953,5 +954,5 @@ } }, "version": "0.0.0", - "fingerprint": "wtswDMhtuMcC4+ami5KddQIznqdpVjt8qc7Ba2QnnHk=" + "fingerprint": "kYLYyNyPod3JTyJWmgPJL1Z85k0HEEhQeMIs4zH4bEQ=" } \ No newline at end of file diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii index 7e8504bdc7..c8af814015 100644 --- a/packages/jsii-calc/test/assembly.jsii +++ b/packages/jsii-calc/test/assembly.jsii @@ -44,6 +44,7 @@ "@scope/jsii-calc-base": { "targets": { "dotnet": { + "iconUrl": "https://raw.githubusercontent.com/aws/aws-cdk/main/logo/default-256-dark.png", "namespace": "Amazon.JSII.Tests.CalculatorNamespace.BaseNamespace", "packageId": "Amazon.JSII.Tests.CalculatorPackageId.BasePackageId" }, @@ -17652,5 +17653,5 @@ } }, "version": "3.20.120", - "fingerprint": "/9fE/+p2My22F013iCie8G2N8WE1ZwPqZKkeGKYCbyI=" + "fingerprint": "P+E1ta4n5zaxREzT7CqW574NyC10CNmKv5NAwZ5pRt8=" } \ No newline at end of file diff --git a/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts b/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts index 45af4500e2..710cf4aa3e 100644 --- a/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts +++ b/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts @@ -1,11 +1,14 @@ import * as spec from '@jsii/spec'; import * as clone from 'clone'; import * as fs from 'fs-extra'; +import * as http from 'http'; +import * as https from 'https'; import * as reflect from 'jsii-reflect'; import { Rosetta } from 'jsii-rosetta'; import * as path from 'path'; import { Generator, Legalese } from '../../generator'; +import { debug } from '../../logging'; import { MethodDefinition, PropertyDefinition } from '../_utils'; import { DotNetDocGenerator } from './dotnetdocgenerator'; import { DotNetRuntimeGenerator } from './dotnetruntimegenerator'; @@ -89,7 +92,7 @@ export class DotNetGenerator extends Generator { this.code, ); filegen.generateAssemblyInfoFile(); - filegen.generateProjectFile(this.typeresolver.namespaceDependencies); + // Calling super.save() dumps the tarball in the format name@version.jsii.tgz. // This is not in sync with the Old .NET generator where the name is scope-name-version.tgz. // Hence we are saving the files ourselves here: @@ -103,6 +106,27 @@ export class DotNetGenerator extends Generator { await fs.mkdirp(path.join(outdir, packageId)); await fs.copyFile(tarball, path.join(outdir, packageId, tarballFileName)); + // Attempt to download the package icon from the configured URL so we can use the non-deprecated PackageIcon + // attribute. If this fails or is opted out (via $JSII_PACMAK_DOTNET_NO_DOWNLOAD_ICON being set), then only the + // deprecated PackageIconUrl will be emitted. + const iconFile = + this.assembly.targets?.dotnet?.iconUrl != null && + !process.env.JSII_PACMAK_DOTNET_NO_DOWNLOAD_ICON + ? await tryDownloadResource( + this.assembly.targets.dotnet.iconUrl, + path.join(outdir, packageId), + ).catch((err: any) => { + debug( + `[dotnet] Unable to download package icon, will only use deprecated PackageIconUrl attribute: ${err.cause}`, + ); + return Promise.resolve(undefined); + }) + : undefined; + filegen.generateProjectFile( + this.typeresolver.namespaceDependencies, + iconFile, + ); + // Create an anchor file for the current model this.generateDependencyAnchorFile(); @@ -1191,3 +1215,108 @@ export class DotNetGenerator extends Generator { ); } } + +async function tryDownloadResource( + urlText: string, + into: string, +): Promise { + const url = new URL(urlText); + + let request: typeof http.get | typeof https.get; + switch (url.protocol) { + case 'http:': + request = http.get; + break; + case 'https:': + request = https.get; + break; + default: + // Unhandled protocol... ignoring + debug( + `Unsupported URL protocol for resource download: ${url.protocol} (full URL: ${urlText})`, + ); + return undefined; + } + + return new Promise((ok, ko) => + request(url, (res) => { + switch (res.statusCode) { + case 200: + let fileName = path.basename(url.pathname); + // Ensure there is a content-appropriate extension on the result... + switch (res.headers['content-type']) { + case 'image/gif': + if (!fileName.endsWith('.gif')) { + fileName = `${fileName}.gif`; + } + break; + case 'image/jpeg': + if (!fileName.endsWith('.jpg')) { + fileName = `${fileName}.jpg`; + } + break; + case 'image/png': + if (!fileName.endsWith('.png')) { + fileName = `${fileName}.png`; + } + break; + default: + // Nothing to do... + } + const filePath = path.join('resources', fileName); + try { + fs.mkdirpSync(path.join(into, 'resources')); + } catch (err) { + return ko(err); + } + try { + const fd = fs.openSync( + path.join(into, filePath), + fs.constants.O_CREAT | + fs.constants.O_TRUNC | + fs.constants.O_WRONLY, + ); + res + .once('error', (cause) => { + try { + fs.closeSync(fd); + } catch { + // IGNORE + } + ko(cause); + }) + .on('data', (chunk) => { + const buff = Buffer.from(chunk); + let offset = 0; + while (offset < buff.length) { + try { + offset += fs.writeSync(fd, buff, offset); + } catch (err) { + return ko(err); + } + } + }) + .once('close', () => { + try { + fs.closeSync(fd); + ok(filePath); + } catch (err) { + ko(err); + } + }); + } catch (err) { + return ko(err); + } + break; + default: + ko( + new Error( + `GET ${urlText} -- HTTP ${res.statusCode ?? 0} (${ + res.statusMessage ?? 'Unknown Error' + })`, + ), + ); + } + }).once('error', ko), + ); +} diff --git a/packages/jsii-pacmak/lib/targets/dotnet/filegenerator.ts b/packages/jsii-pacmak/lib/targets/dotnet/filegenerator.ts index e1d44251b5..c7332d93ea 100644 --- a/packages/jsii-pacmak/lib/targets/dotnet/filegenerator.ts +++ b/packages/jsii-pacmak/lib/targets/dotnet/filegenerator.ts @@ -44,7 +44,10 @@ export class FileGenerator { } // Generates the .csproj file - public generateProjectFile(dependencies: Map) { + public generateProjectFile( + dependencies: Map, + iconFile?: string, + ) { const assembly = this.assm; const packageId: string = assembly.targets!.dotnet!.packageId; @@ -62,6 +65,22 @@ export class FileGenerator { propertyGroup.comment('Package Identification'); propertyGroup.ele('Description', this.getDescription()); + if (iconFile != null) { + propertyGroup.ele('PackageIcon', iconFile.split(/[/\\]+/).join('\\')); + // We also need to actually include the icon in the package + const noneNode = rootNode.ele('ItemGroup').ele('None'); + noneNode.att('Include', iconFile.split(/[/\\]+/).join('\\')); + noneNode.att('Pack', 'true'); + noneNode.att( + 'PackagePath', + `\\${path + .dirname(iconFile) + .split(/[/\\]+/) + .join('\\')}`, + ); + } + // We continue to include the PackageIconUrl even if we put PackageIcon for backwards compatibility, as suggested + // by https://docs.microsoft.com/en-us/nuget/reference/msbuild-targets#packageicon if (dotnetInfo!.iconUrl != null) { propertyGroup.ele('PackageIconUrl', dotnetInfo!.iconUrl); } diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.js.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.js.snap index 19e36814c7..46a5f99b31 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.js.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.js.snap @@ -21,6 +21,8 @@ exports[`Generated code for "@scope/jsii-calc-base": / 1`] = ` ┣━ 📄 AssemblyInfo.cs ┣━ 📄 LICENSE ┣━ 📄 NOTICE + ┣━ 📁 resources + ┃ ┗━ 📄 default-256-dark.png ┗━ 📄 scope-jsii-calc-base-0.0.0.tgz `; @@ -29,6 +31,8 @@ exports[`Generated code for "@scope/jsii-calc-base": /dotnet/Amazon.JSII An example direct dependency for jsii-calc. + resources\\default-256-dark.png + https://raw.githubusercontent.com/aws/aws-cdk/main/logo/default-256-dark.png Amazon.JSII.Tests.CalculatorPackageId.BasePackageId Apache-2.0 0.0.0 @@ -48,6 +52,9 @@ exports[`Generated code for "@scope/jsii-calc-base": /dotnet/Amazon.JSII snupkg netcoreapp3.1 + + + @@ -495,6 +502,8 @@ Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved. `; +exports[`Generated code for "@scope/jsii-calc-base": /dotnet/Amazon.JSII.Tests.CalculatorPackageId.BasePackageId/resources/default-256-dark.png 1`] = `dotnet/Amazon.JSII.Tests.CalculatorPackageId.BasePackageId/resources/default-256-dark.png is a binary file`; + exports[`Generated code for "@scope/jsii-calc-base": /dotnet/Amazon.JSII.Tests.CalculatorPackageId.BasePackageId/scope-jsii-calc-base-0.0.0.tgz 1`] = `dotnet/Amazon.JSII.Tests.CalculatorPackageId.BasePackageId/scope-jsii-calc-base-0.0.0.tgz is a tarball`; exports[`Generated code for "@scope/jsii-calc-base-of-base": / 1`] = ` @@ -3095,7 +3104,9 @@ exports[`Generated code for "jsii-calc": / 1`] = ` ┣━ 📄 AssemblyInfo.cs ┣━ 📄 jsii-calc-3.20.120.tgz ┣━ 📄 LICENSE - ┗━ 📄 NOTICE + ┣━ 📄 NOTICE + ┗━ 📁 resources + ┗━ 📄 AWSLogo128x128.png `; exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon.JSII.Tests.CalculatorPackageId.csproj 1`] = ` @@ -3103,6 +3114,7 @@ exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.Calcu A simple calcuator built on JSII. (Stability: Stable) + resources\\AWSLogo128x128.png https://sdk-for-net.amazonwebservices.com/images/AWSLogo128x128.png Amazon.JSII.Tests.CalculatorPackageId Apache-2.0 @@ -3124,6 +3136,9 @@ exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.Calcu snupkg netcoreapp3.1 + + + @@ -19261,3 +19276,5 @@ Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved. `; exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/jsii-calc-3.20.120.tgz 1`] = `dotnet/Amazon.JSII.Tests.CalculatorPackageId/jsii-calc-3.20.120.tgz is a tarball`; + +exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/resources/AWSLogo128x128.png 1`] = `dotnet/Amazon.JSII.Tests.CalculatorPackageId/resources/AWSLogo128x128.png is a binary file`; diff --git a/packages/jsii-pacmak/test/generated-code/harness.ts b/packages/jsii-pacmak/test/generated-code/harness.ts index 4026f9e1ac..f0a622e3d2 100644 --- a/packages/jsii-pacmak/test/generated-code/harness.ts +++ b/packages/jsii-pacmak/test/generated-code/harness.ts @@ -9,6 +9,7 @@ import { shell } from '../../lib/util'; const FILE = Symbol('file'); const MISSING = Symbol('missing'); const TARBALL = Symbol('tarball'); +const BINARY = Symbol('binary'); export const TREE = Symbol('tree'); // Custom serializers so we can see the source without escape sequences @@ -28,6 +29,10 @@ expect.addSnapshotSerializer({ val[TARBALL].endsWith('.tgz') ? 'is' : 'embeds' } a tarball`, }); +expect.addSnapshotSerializer({ + test: (val) => val?.[BINARY] != null, + serialize: (val) => `${val[BINARY]} is a binary file`, +}); expect.addSnapshotSerializer({ test: (val) => val?.[TREE] != null, serialize: (val) => { @@ -101,6 +106,9 @@ export function checkTree( if (file.endsWith('.tgz')) { // Special-cased to avoid binary differences being annoying expect({ [TARBALL]: relativeFile }).toMatchSnapshot(snapshotName); + } else if (file.endsWith('.png')) { + // Special-cased to avoid binary differences being annoying + expect({ [BINARY]: relativeFile }).toMatchSnapshot(snapshotName); } else { expect({ [FILE]: fs.readFileSync(file, { encoding: 'utf-8' }),