Sync eng/common directory with azure-sdk-tools for PR 15278#7083
Sync eng/common directory with azure-sdk-tools for PR 15278#7083
Conversation
Adds an opt-in capability to the shared TypeSpec emitter pipeline template that bundles an emitter package and uploads it to the typespec playground blob storage. The uploaded <pkgName>/latest.json import map is consumed by in-browser playgrounds (e.g. https://azure.github.io/typespec-azure) via their additionalPlaygroundPackages mechanism. Self-contained tooling lives in eng/common/playground-bundle/ and mirrors @typespec/bundle-uploader from microsoft/typespec (which is private and cannot be installed from npm). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
azure-sdk
added
EngSys
There was a problem hiding this comment.
Pull request overview
Syncs in a self-contained TypeSpec “bundle + upload to playground storage” tool into eng/common, and wires it into the shared archetype-typespec-emitter.yml template so emitters can optionally publish bundles for in-browser playground consumption.
Changes:
- Added
eng/common/playground-bundle/Node tooling (script + pinned dependencies + docs) to create a TypeSpec bundle and upload it to thetypespecstorage account. - Extended
archetype-typespec-emitter.ymlwith a newUploadPlaygroundBundleparameter and conditional steps to install and run the uploader in internal, non-PR builds.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/common/playground-bundle/upload.mjs | New uploader script that bundles an emitter and uploads JS + manifest + latest import map to blob storage. |
| eng/common/playground-bundle/package.json | New package definition with pinned dependencies and Node engine requirement. |
| eng/common/playground-bundle/package-lock.json | Lockfile to make installs reproducible for the uploader tooling. |
| eng/common/playground-bundle/README.md | Documentation for how the uploader is used by the pipeline template. |
| eng/common/pipelines/templates/archetype-typespec-emitter.yml | Adds UploadPlaygroundBundle parameter and conditional install/upload steps in the Build job. |
Files not reviewed (1)
- eng/common/playground-bundle/package-lock.json: Language not supported
| const content = JSON.stringify(manifest); | ||
| await blob.upload(content, content.length, { | ||
| blobHTTPHeaders: { blobContentType: "application/json; charset=utf-8" }, | ||
| conditions: { ifNoneMatch: "*" }, | ||
| }); |
There was a problem hiding this comment.
BlockBlobClient.upload() expects the content length in bytes, but content.length is a JS string character count. For non-ASCII characters this can send an incorrect contentLength and cause upload failures/corruption. Use Buffer.byteLength(content, 'utf8') for the length, or switch to uploadData(Buffer.from(content, 'utf8'), ...) (which avoids manually providing a length).
| async function updatePackageLatest(container, pkgName, index) { | ||
| const blob = container.getBlockBlobClient(normalizePath(joinUnix(pkgName, "latest.json"))); | ||
| const content = JSON.stringify(index); | ||
| await blob.upload(content, content.length, { |
There was a problem hiding this comment.
Same issue as uploadManifest: content.length is character count, not byte count, but upload() needs a byte length. Prefer Buffer.byteLength(...) or uploadData(Buffer.from(content, 'utf8'), ...) to avoid mismatched lengths.
| await blob.upload(content, content.length, { | |
| await blob.upload(content, Buffer.byteLength(content, "utf8"), { |
| if (arg === "--package-path") { | ||
| args.packagePath = argv[++i]; | ||
| } else if (arg === "--version") { | ||
| args.version = argv[++i]; | ||
| } else { | ||
| throw new Error(`Unknown argument: ${arg}`); | ||
| } |
There was a problem hiding this comment.
parseArgs increments the index and assigns the next argv element for --package-path/--version, but it doesn't validate that a value actually exists. If the caller passes --version (or --package-path) as the last argument, this will silently set undefined and lead to a later, less actionable error. Consider explicitly checking for a missing value and throwing a targeted error.
| - script: npm ci | ||
| displayName: 'Install playground bundle uploader' | ||
| workingDirectory: $(Build.SourcesDirectory)/eng/common/playground-bundle | ||
| env: | ||
| NPM_CONFIG_USERCONFIG: $(emitterNpmrcPath) | ||
|
|
||
| - task: AzureCLI@2 | ||
| displayName: 'Upload playground bundle' | ||
| inputs: | ||
| azureSubscription: "TypeSpec Storage" | ||
| scriptType: "bash" | ||
| scriptLocation: "inlineScript" | ||
| workingDirectory: $(Build.SourcesDirectory) | ||
| inlineScript: > | ||
| node eng/common/playground-bundle/upload.mjs | ||
| --package-path ${{ parameters.EmitterPackagePath }} | ||
| --version $(initialize.emitterVersion) |
There was a problem hiding this comment.
This stage runs npm ci and then node .../upload.mjs, but the Build job doesn't pin/ensure a Node version (unlike other jobs in this template that use UseNode@1). Since eng/common/playground-bundle/package.json requires Node >= 20.19.0, this can break when the agent image default Node changes. Consider adding a UseNode@1 step (e.g. 22.x) before the npm/node steps here to make the environment deterministic.
Sync eng/common directory with azure-sdk-tools for PR Azure/azure-sdk-tools#15278 See eng/common workflow