Skip to content

[dotnet] implement Central Package Management#17563

Open
titusfortner wants to merge 3 commits into
trunkfrom
dotnet_cpm
Open

[dotnet] implement Central Package Management#17563
titusfortner wants to merge 3 commits into
trunkfrom
dotnet_cpm

Conversation

@titusfortner
Copy link
Copy Markdown
Member

While working with Renovate I realized that we have dotnet versions specified in 6 different places, and Renovate can't help with all of them, and versions are different between them, especially transitive versions.

💥 What does this PR do?

  1. Use CPM for webdriver csproj files
  2. Dynamically generate nuspec versions for users

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s):
    • What was generated:
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

I have a few follow-on PRs for dynamically generating paket.dependencies from CPM, but this seems like the easiest first step.

🔄 Types of changes

  • Cleanup (formatting, renaming)

@selenium-ci selenium-ci added C-dotnet .NET Bindings B-build Includes scripting, bazel and CI integrations B-support Issue or PR related to support classes labels May 24, 2026
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Centralize .NET package versions using Central Package Management

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Centralize .NET package versions using Central Package Management (CPM)
• Create Directory.Packages.props with all package version declarations
• Generate nuspec dependencies dynamically from CPM and csproj files
• Remove hardcoded package versions from all csproj files
Diagram
flowchart LR
  CPM["Directory.Packages.props<br/>Central Versions"]
  CSPROJ["csproj Files<br/>PackageReferences"]
  GENERATOR["cpm_nuspec_gen.py<br/>Generator Tool"]
  NUSPEC["Selenium.WebDriver.nuspec<br/>Generated Dependencies"]
  
  CPM -- "version lookup" --> GENERATOR
  CSPROJ -- "package list" --> GENERATOR
  GENERATOR -- "renders" --> NUSPEC

Loading

File Changes

1. dotnet/private/cpm_nuspec_gen.py ✨ Enhancement +107/-0

New Python tool for generating nuspec dependencies

dotnet/private/cpm_nuspec_gen.py


2. dotnet/private/nuget_pack.bzl ✨ Enhancement +51/-1

Add CPM-based dependency rendering to nuget_pack rule

dotnet/private/nuget_pack.bzl


3. dotnet/private/BUILD.bazel ⚙️ Configuration changes +7/-0

Register cpm_nuspec_gen as py_binary target

dotnet/private/BUILD.bazel


View more (8)
4. dotnet/BUILD.bazel ⚙️ Configuration changes +1/-0

Export Directory.Packages.props file

dotnet/BUILD.bazel


5. dotnet/Directory.Packages.props ✨ Enhancement +20/-0

New central package version management file

dotnet/Directory.Packages.props


6. dotnet/src/webdriver/BUILD.bazel ⚙️ Configuration changes +6/-0

Configure webdriver-pack with CPM settings

dotnet/src/webdriver/BUILD.bazel


7. dotnet/src/webdriver/Selenium.WebDriver.csproj ✨ Enhancement +5/-4

Remove inline versions, use CPM for packages

dotnet/src/webdriver/Selenium.WebDriver.csproj


8. dotnet/src/webdriver/Selenium.WebDriver.nuspec ✨ Enhancement +1/-17

Replace hardcoded dependencies with template token

dotnet/src/webdriver/Selenium.WebDriver.nuspec


9. dotnet/test/remote/Selenium.WebDriver.Remote.Tests.csproj ✨ Enhancement +4/-3

Remove inline versions, use CPM for packages

dotnet/test/remote/Selenium.WebDriver.Remote.Tests.csproj


10. dotnet/test/support/Selenium.Support.Tests.csproj ✨ Enhancement +6/-5

Remove inline versions, use CPM for packages

dotnet/test/support/Selenium.Support.Tests.csproj


11. dotnet/test/webdriver/Selenium.WebDriver.Tests.csproj ✨ Enhancement +7/-6

Remove inline versions, use CPM for packages

dotnet/test/webdriver/Selenium.WebDriver.Tests.csproj


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0)

Grey Divider


Remediation recommended

1. Misleading CPM error file 🐞 Bug ⚙ Maintainability ⭐ New
Description
cpm_nuspec_gen.py hard-codes "Directory.Packages.props" in its missing-package-version error, even
though the CPM file is provided via the dynamic --cpm argument. If nuget_pack is configured with a
different CPM file path/name, the build fails with an error that points at the wrong file, slowing
diagnosis.
Code

dotnet/private/cpm_nuspec_gen.py[62]

Evidence
The exit message is hard-coded to a specific file name, but the Bazel rule passes an arbitrary CPM
file path via --cpm, so the message can be inaccurate for legitimate configurations.

dotnet/private/cpm_nuspec_gen.py[56-63]
dotnet/private/nuget_pack.bzl[24-57]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The generator exits with an error that always references `Directory.Packages.props`, even though the CPM file is passed in via `--cpm`. This makes failures misleading when the caller provides a different CPM file.

### Issue Context
`nuget_pack.bzl` passes the CPM file path dynamically (`ctx.file.cpm_file.path`). The Python tool should report the actual path it was given.

### Fix Focus Areas
- dotnet/private/cpm_nuspec_gen.py[56-63]

### Implementation notes
- Pass the CPM path (e.g., `args.cpm`) into `build_dependencies_block(...)` or format the error in `main()` where `args.cpm` is available.
- Update the `sys.exit(...)` message to include the CPM path (and optionally the csproj path), e.g. `... not declared in {cpm_path}`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Empty compat TFMs drops deps 🐞 Bug ≡ Correctness
Description
When nuget_pack is configured with cpm_file/csproj_file but compat_tfms is left at its default empty
list, the generator emits empty <group> blocks for every TFM and the resulting nuspec declares no
dependencies even though the csproj has PackageReferences.
Code

dotnet/private/nuget_pack.bzl[R208-211]

Evidence
The rule sets compat_tfms default to [] and passes it directly to the generator; the generator
only adds dependencies for TFMs present in compat_tfms, otherwise emitting an empty <group>
element, so compat_tfms=[] yields no dependency entries in the resulting nuspec.

dotnet/private/nuget_pack.bzl[24-55]
dotnet/private/nuget_pack.bzl[200-211]
dotnet/private/cpm_nuspec_gen.py[56-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`nuget_pack`’s CPM/nuspec rendering path passes `compat_tfms` into the generator, but the rule defaults `compat_tfms` to `[]`. With an empty list, `cpm_nuspec_gen.py` generates empty dependency groups for all TFMs, producing a NuGet package that declares no dependencies even when the csproj references packages.

### Issue Context
- `nuget_pack_impl` always passes `--compat-tfms` from `ctx.attr.compat_tfms`.
- `compat_tfms` defaults to an empty list.
- The generator only emits dependencies when `tfm in compat_tfms`.

### Fix Focus Areas
- dotnet/private/nuget_pack.bzl[24-57]
- dotnet/private/nuget_pack.bzl[200-211]
- dotnet/private/cpm_nuspec_gen.py[56-78]

### Suggested fix
In `nuget_pack_impl`, when CPM rendering is enabled:
1. Compute `all_tfms`.
2. If `ctx.attr.compat_tfms` is empty, set `compat_tfms_to_use = all_tfms` (or alternatively `fail()` to force explicit configuration).
3. Pass `compat_tfms_to_use` to the generator.

Optionally add validation to fail fast if only one of `cpm_file`/`csproj_file` is set.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

3. Unescaped XML attribute values 🐞 Bug ☼ Reliability
Description
cpm_nuspec_gen.py builds nuspec XML via string interpolation, so any unexpected XML-sensitive
characters in package IDs/versions will produce malformed nuspec output.
Code

dotnet/private/cpm_nuspec_gen.py[R66-68]

Evidence
The dependency element is created by directly interpolating name and versions[name] into XML
attribute positions, with no escaping/quoting helper, so correctness depends on those strings
containing only XML-safe characters.

dotnet/private/cpm_nuspec_gen.py[56-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The nuspec dependency XML is currently produced using f-strings, without XML escaping for attribute values. This makes the generator fragile and can output malformed XML if an ID/version contains characters requiring escaping.

### Issue Context
`cpm_nuspec_gen.py` already parses XML with `xml.etree.ElementTree`, but renders dependencies as raw strings.

### Fix Focus Areas
- dotnet/private/cpm_nuspec_gen.py[56-78]

### Suggested fix
Prefer one of:
1) Construct the `<dependencies>` / `<group>` / `<dependency>` elements using `xml.etree.ElementTree` and serialize them, OR
2) Escape attribute values (e.g., using `xml.sax.saxutils.quoteattr` / `escape`) before embedding them into strings.

Keep the output formatting (indentation/newlines) stable to avoid churn in generated nuspecs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit 8bdd98f

Results up to commit d2955a1


🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)


Remediation recommended
1. Empty compat TFMs drops deps 🐞 Bug ≡ Correctness
Description
When nuget_pack is configured with cpm_file/csproj_file but compat_tfms is left at its default empty
list, the generator emits empty <group> blocks for every TFM and the resulting nuspec declares no
dependencies even though the csproj has PackageReferences.
Code

dotnet/private/nuget_pack.bzl[R208-211]

Evidence
The rule sets compat_tfms default to [] and passes it directly to the generator; the generator
only adds dependencies for TFMs present in compat_tfms, otherwise emitting an empty <group>
element, so compat_tfms=[] yields no dependency entries in the resulting nuspec.

dotnet/private/nuget_pack.bzl[24-55]
dotnet/private/nuget_pack.bzl[200-211]
dotnet/private/cpm_nuspec_gen.py[56-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`nuget_pack`’s CPM/nuspec rendering path passes `compat_tfms` into the generator, but the rule defaults `compat_tfms` to `[]`. With an empty list, `cpm_nuspec_gen.py` generates empty dependency groups for all TFMs, producing a NuGet package that declares no dependencies even when the csproj references packages.

### Issue Context
- `nuget_pack_impl` always passes `--compat-tfms` from `ctx.attr.compat_tfms`.
- `compat_tfms` defaults to an empty list.
- The generator only emits dependencies when `tfm in compat_tfms`.

### Fix Focus Areas
- dotnet/private/nuget_pack.bzl[24-57]
- dotnet/private/nuget_pack.bzl[200-211]
- dotnet/private/cpm_nuspec_gen.py[56-78]

### Suggested fix
In `nuget_pack_impl`, when CPM rendering is enabled:
1. Compute `all_tfms`.
2. If `ctx.attr.compat_tfms` is empty, set `compat_tfms_to_use = all_tfms` (or alternatively `fail()` to force explicit configuration).
3. Pass `compat_tfms_to_use` to the generator.

Optionally add validation to fail fast if only one of `cpm_file`/`csproj_file` is set.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments
2. Unescaped XML attribute values 🐞 Bug ☼ Reliability
Description
cpm_nuspec_gen.py builds nuspec XML via string interpolation, so any unexpected XML-sensitive
characters in package IDs/versions will produce malformed nuspec output.
Code

dotnet/private/cpm_nuspec_gen.py[R66-68]

Evidence
The dependency element is created by directly interpolating name and versions[name] into XML
attribute positions, with no escaping/quoting helper, so correctness depends on those strings
containing only XML-safe characters.

dotnet/private/cpm_nuspec_gen.py[56-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The nuspec dependency XML is currently produced using f-strings, without XML escaping for attribute values. This makes the generator fragile and can output malformed XML if an ID/version contains characters requiring escaping.

### Issue Context
`cpm_nuspec_gen.py` already parses XML with `xml.etree.ElementTree`, but renders dependencies as raw strings.

### Fix Focus Areas
- dotnet/private/cpm_nuspec_gen.py[56-78]

### Suggested fix
Prefer one of:
1) Construct the `<dependencies>` / `<group>` / `<dependency>` elements using `xml.etree.ElementTree` and serialize them, OR
2) Escape attribute values (e.g., using `xml.sax.saxutils.quoteattr` / `escape`) before embedding them into strings.

Keep the output formatting (indentation/newlines) stable to avoid churn in generated nuspecs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

@selenium-ci
Copy link
Copy Markdown
Member

Thank you, @titusfortner for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Persistent review updated to latest commit 8bdd98f

@titusfortner titusfortner changed the title Dotnet cpm [dotnet] implement CPM May 24, 2026
@titusfortner titusfortner changed the title [dotnet] implement CPM [dotnet] implement Central Package Management May 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations B-support Issue or PR related to support classes C-dotnet .NET Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants