Skip to content

[pipelines] add PNPM_CONFIG_REGISTRY endpoint - #11700

Open
iscai-msft wants to merge 2 commits into
mainfrom
pnpmIgnoreNpmConfig
Open

[pipelines] add PNPM_CONFIG_REGISTRY endpoint#11700
iscai-msft wants to merge 2 commits into
mainfrom
pnpmIgnoreNpmConfig

Conversation

@iscai-msft

Copy link
Copy Markdown
Member

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates TypeSpec’s publishing/regeneration pipelines and Docker build to support pnpm v11+ configuration via pnpm_config_* / PNPM_CONFIG_* variables, including allowing a registry endpoint override for compliance scenarios.

Changes:

  • Propagate .npmrc via both NPM_CONFIG_USERCONFIG and PNPM_CONFIG_USERCONFIG for tsp-client-driven installs.
  • Write both npm_config_registry and pnpm_config_registry into a temporary .env for regeneration flows.
  • Add an overridable NPM_REGISTRY pipeline variable and pass it into the Docker build as --build-arg NPM_REGISTRY=..., plus set pnpm/npm registry env vars in the Dockerfile.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/http-client-csharp/eng/scripts/Submit-AzureSdkForNetPr.ps1 Sets NPM_CONFIG_USERCONFIG and PNPM_CONFIG_USERCONFIG so tsp-client can resolve packages using the repo .npmrc.
packages/http-client-csharp/eng/scripts/RegenPreview.ps1 Writes both npm and pnpm registry env vars to a temporary .env for tsp-client execution.
packages/http-client-csharp/eng/pipeline/publish.yml Adds pnpm-related pipeline variables (store/cache/force) alongside npm variables.
eng/tsp-core/pipelines/publish.yml Introduces an npmRegistry variable (overridable via NPM_REGISTRY) and passes it as a Docker build-arg.
docker/Dockerfile Accepts NPM_REGISTRY build-arg and sets npm/pnpm registry env vars for dependency installs in the build stages.
Suppressed comments (1)

docker/Dockerfile:37

  • The final stage also defaults ARG NPM_REGISTRY= to empty and then sets NPM_CONFIG_REGISTRY to that value. For docker builds that don’t pass the build-arg, this can force npm’s registry to an empty value during npm install -g /tmp/compiler.tgz, which can break dependency resolution.
# Inherit registry setting for final stage
ARG NPM_REGISTRY=
ENV NPM_CONFIG_REGISTRY=${NPM_REGISTRY}


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/http-client-csharp/eng/pipeline/publish.yml Outdated
Comment thread docker/Dockerfile
Comment on lines +7 to +12
# Optional: Override npm registry for CFS compliance.
# pnpm 11+ requires PNPM_CONFIG_REGISTRY (ignores NPM_CONFIG_REGISTRY).
ARG NPM_REGISTRY=
ENV PNPM_CONFIG_REGISTRY=${NPM_REGISTRY}
ENV NPM_CONFIG_REGISTRY=${NPM_REGISTRY}

- Use underscore in pnpm_config_store_dir (not hyphen)
- Default NPM_REGISTRY to https://registry.npmjs.org/ instead of empty
$originalSdkEnv = if (Test-Path $sdkEnvFile) { Get-Content $sdkEnvFile -Raw } else { $null }
Set-Content $sdkEnvFile "npm_config_registry=$artifactFeedRegistry`n" -Encoding utf8 -NoNewline
# Set both npm and pnpm registry env vars (pnpm 11+ ignores npm_config_* prefix)
Set-Content $sdkEnvFile "npm_config_registry=$artifactFeedRegistry`npnpm_config_registry=$artifactFeedRegistry`n" -Encoding utf8 -NoNewline

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't use pnpm when running this script. It's primarily used locally. Do we need this change here? If so, do we need to add some delimiter between the 2 registry values ?

Copilot AI review requested due to automatic review settings August 17, 2026 17:17
@pkg-pr-new

pkg-pr-new Bot commented Aug 17, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@11700

commit: 8446caf

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

docker/Dockerfile:36

  • Final Docker image stage only sets NPM_CONFIG_REGISTRY, but the earlier comment states pnpm 11+ requires PNPM_CONFIG_REGISTRY. If any runtime path uses pnpm (e.g., tsp install or future additions), the overridden registry won't apply in the final image. Set PNPM_CONFIG_REGISTRY in the final stage as well to truly inherit the registry setting.
# Inherit registry setting for final stage
ARG NPM_REGISTRY=https://registry.npmjs.org/
ENV NPM_CONFIG_REGISTRY=${NPM_REGISTRY}

@azure-sdk-automation

Copy link
Copy Markdown

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp eng

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants