Skip to content

fix(csharp): ensure extraDependencies overrides take precedence over bundled deps#14591

Merged
jsklan merged 8 commits intomainfrom
devin/FER-9451-1775256426-csharp-extra-deps-override
Apr 6, 2026
Merged

fix(csharp): ensure extraDependencies overrides take precedence over bundled deps#14591
jsklan merged 8 commits intomainfrom
devin/FER-9451-1775256426-csharp-extra-deps-override

Conversation

@jsklan
Copy link
Copy Markdown
Contributor

@jsklan jsklan commented Apr 3, 2026

Description

Linear ticket: Refs FER-9451

The C# generator's extra-dependencies config option could only add new dependencies — it could not override the versions of bundled dependencies (OneOf, System.Net.Http, etc.). Extra deps were appended alongside bundled ones, producing duplicate <PackageReference> entries in the .csproj. This prevented customers from pinning a specific version (e.g., for CVE remediation).

Mirrors the Java fix in PR #14527.

Changes Made

  • generators/csharp/base/src/project/CsharpProject.ts — In getDependencies(), collect extra dep names into a case-insensitive set, then skip any bundled dep whose package name matches before appending user-specified versions. Same treatment applied to getLegacyFrameworkDependencies() for TFM-conditional deps (System.Text.Json, System.Net.Http).
    • PolySharp special handling: PolySharp requires multi-line XML with <IncludeAssets> and <PrivateAssets> metadata. Instead of using the generic pushIfNotOverridden + extra-deps loop (which would emit a simple single-line reference), PolySharp is always emitted with full metadata using the user's version if overridden, and skipped in the extra-deps loop to avoid duplication.
  • seed/csharp-sdk/seed.yml — Added extra-dependencies-override fixture variant that specifies PolySharp: "1.14.0" to override the bundled PolySharp: "1.15.0".
  • seed/csharp-sdk/imdb/extra-dependencies-override/ — Generated seed output proving the override works: PolySharp at 1.14.0 with <IncludeAssets> and <PrivateAssets> metadata preserved.
  • generators/csharp/sdk/versions.yml — Added v2.56.6 changelog entry.

Key Review Points

  • PolySharp metadata preservation: PolySharp is the only bundled dep with multi-line XML (<IncludeAssets>, <PrivateAssets> child elements). When overridden via extra-dependencies, the version is swapped but metadata is preserved. The extra-deps loop skips PolySharp (case-insensitive) to avoid double-emission. Verify this logic in getDependencies() around lines 930–974.
  • Legacy TFM deps: getLegacyFrameworkDependencies() suppresses bundled entries but does not re-inject the user's version into the conditional ItemGroup — the user's version is already in the unconditional group from getDependencies(). Confirm MSBuild resolves this correctly across TFMs.
  • irVersion: v2.56.6 uses irVersion: 65, matching the rest of the 2.56.x line. The jump to irVersion: 66 occurred in v2.57.0-rc.0.
  • Fixture uses PolySharp override (not OneOf): Originally the fixture overrode OneOf to a lower version, but OneOf.Extended 3.0.271 has a transitive dependency on OneOf >= 3.0.271, causing NuGet error NU1605. Switched to overriding PolySharp which has no transitive dependency conflicts.
  • Existing extra-dependencies fixture (non-conflicting deps) was regenerated with zero diff, confirming no regression.

Human Review Checklist

  • Verify PolySharp in seed/csharp-sdk/imdb/extra-dependencies-override/src/SeedApi/SeedApi.csproj is at version 1.14.0 with <IncludeAssets> and <PrivateAssets> metadata preserved (not a simple self-closing tag)
  • Confirm bundled deps (OneOf, OneOf.Extended, System.Net.Http, System.Text.RegularExpressions) remain at their bundled versions since they are not overridden in this fixture
  • Verify the PolySharp version lookup loop (lines 932–937) and the skip in the extra-deps loop (lines 970–972) both use case-insensitive matching consistently
  • Confirm irVersion: 65 is correct for a 2.56.x patch release
  • Spot-check that overriding a non-PolySharp bundled dep (e.g. OneOf) would also work — the pushIfNotOverridden helper skips correctly but there is no seed fixture exercising that path

Testing

  • Seed snapshot test: new extra-dependencies-override fixture validates override behavior (PolySharp 1.14.0 with metadata)
  • Seed snapshot test: existing extra-dependencies fixture regenerated with no diff (regression check)
  • All CI checks green (compile, lint, biome, test, depcheck, test-ete, all seed-test-results including csharp-sdk, Validate Changelogs, Validate versions.yml)

Link to Devin session: https://app.devin.ai/sessions/8163fd2454fe4ba3b294a07131c23d55


Open with Devin

@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@devin-ai-integration devin-ai-integration bot force-pushed the devin/FER-9451-1775256426-csharp-extra-deps-override branch from 78962f1 to fdb3981 Compare April 3, 2026 23:25
@devin-ai-integration devin-ai-integration bot marked this pull request as ready for review April 3, 2026 23:53
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration bot and others added 8 commits April 4, 2026 02:49
…bundled deps

Co-Authored-By: judah <jsklan.development@gmail.com>
Co-Authored-By: judah <jsklan.development@gmail.com>
…rridden via extra-dependencies

Co-Authored-By: judah <jsklan.development@gmail.com>
Co-Authored-By: judah <jsklan.development@gmail.com>
…rride fixture

Co-Authored-By: judah <jsklan.development@gmail.com>
…ransitive dep conflict

Co-Authored-By: judah <jsklan.development@gmail.com>
Co-Authored-By: bot_apk <apk@cognition.ai>
@devin-ai-integration devin-ai-integration bot force-pushed the devin/FER-9451-1775256426-csharp-extra-deps-override branch from 1c92fd7 to 22c8754 Compare April 4, 2026 02:49
@jsklan jsklan merged commit e6ad674 into main Apr 6, 2026
91 checks passed
@jsklan jsklan deleted the devin/FER-9451-1775256426-csharp-extra-deps-override branch April 6, 2026 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants