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
aws · Aug 4, 2022 · c65b1d9 · c65b1d9
1 parent bcffb4b
commit c65b1d9
Show file tree

Hide file tree

Showing 8 changed files with 184 additions and 7 deletions.
diff --git 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
@@ -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="
 }
diff --git 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="
 }
diff --git 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="
 }
diff --git 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<string | undefined> {
+  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
@@ -44,7 +44,10 @@ export class FileGenerator {
   }
 
   // Generates the .csproj file
-  public generateProjectFile(dependencies: Map<string, DotNetDependency>) {
+  public generateProjectFile(
+    dependencies: Map<string, DotNetDependency>,
+    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
diff --git 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' }),