[dotnet] include .snupkg in nuget_push runfiles so symbols get published

[titusfortner]

When I was looking at SourceLink things, the Agent said we were releasing wrong for the new snupkg (or at least not ideally). This is the suggested fix. @nvborisenko can you verify?

💥 What does this PR do?

nuget_pack produces both a .nupkg (main package) and a .snupkg (symbol/debug package) per target. Previously, allow_files = [".nupkg"] filtered the .snupkg out when nuget_push collected files via ctx.files.packages. That meant the symbol package never made it into nupkg_files, and therefore never made it into the runfiles tree built on line 16 (ctx.runfiles(files = nupkg_files + [dotnet])).

snupkg is now in the iteration list (because of change #1), but we don't want a separate push command for it. We let dotnet nuget push handle symbols via its built-in auto-detection

dotnet nuget push foo.nupkg automatically looks for foo.snupkg in the same directory and uploads it as the symbol package. nuget_pack declares both files with the same stem (e.g. Selenium.WebDriver.4.27.0.nupkg and Selenium.WebDriver.4.27.0.snupkg), so once the .snupkg is in runfiles, it lands right next to its .nupkg and dotnet finds it on its own.

🤖 AI assistance

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

[qodo-code-review]

Review Summary by Qodo

Include .snupkg symbol packages in nuget_push runfiles

✨ Enhancement

Walkthroughs

Description

• Include .snupkg symbol packages in nuget_push runfiles
• Filter out .snupkg files from push commands to avoid duplicate uploads
• Allow dotnet nuget push to auto-detect and upload symbols
• Enables proper symbol package publishing alongside main packages

Diagram

flowchart LR
  A["nuget_pack produces .nupkg and .snupkg"] -->|"allow_files updated"| B["Both files included in runfiles"]
  B -->|"Unix/Windows script filters"| C["Only .nupkg files get push commands"]
  C -->|"dotnet nuget push auto-detection"| D["Symbols uploaded alongside package"]

Loading

File Changes

1. dotnet/private/nuget_push.bzl ✨ Enhancement +5/-1

Filter snupkg files and update allowed file types

• Updated allow_files attribute to include both .nupkg and .snupkg file types
• Added filtering logic in Unix script to skip .snupkg files from push commands
• Added filtering logic in Windows script to skip .snupkg files from push commands
• Enables symbol packages to be included in runfiles while only pushing .nupkg files

dotnet/private/nuget_push.bzl

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0)

1. Silent no-op push 🐞 Bug ≡ Correctness

Description

nuget_push now permits .snupkg inputs but the generated scripts skip any package that is not a
.nupkg; if a caller passes only .snupkg files, the script will have no push commands and will exit
successfully without publishing anything.

Code

dotnet/private/nuget_push.bzl[R41-42]

+        if not nupkg.basename.endswith(".nupkg"):
+            continue

Evidence

The rule now allows .snupkg, but both script generators explicitly continue for any file that
does not end with .nupkg, so an all-.snupkg input yields no push commands. The implementation
uses ctx.files.packages directly as the runfiles set, so .snupkg can be supplied and will be
present, but will not be pushed unless paired with a .nupkg that triggers dotnet nuget push.

dotnet/private/nuget_push.bzl[37-46]
dotnet/private/nuget_push.bzl[78-88]
dotnet/private/nuget_push.bzl[108-115]

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_push` accepts `.snupkg` files (via `allow_files`) but both Unix and Windows script generators skip non-`.nupkg` entries. If the input list contains only `.snupkg`, the script ends up with an empty command list and exits successfully, causing a silent “published nothing” failure.

### Issue Context
This PR intentionally filters `.snupkg` out of the push commands and relies on `dotnet nuget push <pkg>.nupkg` to auto-detect and upload `<pkg>.snupkg` if present in the same directory.

### Fix Focus Areas
- dotnet/private/nuget_push.bzl[37-47]
- dotnet/private/nuget_push.bzl[78-89]

### Suggested fix
- After filtering `nupkg_files` down to `.nupkg` entries, **fail loudly** if the resulting list is empty (e.g., `fail("nuget_push: no .nupkg files provided")`).
- (Optional) Use `nupkg.extension == "nupkg"` instead of `basename.endswith(".nupkg")` for clarity.

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

---

2. Misleading packages docstring 🐞 Bug ⚙ Maintainability

Description

The packages attribute documentation still says it is 'The nupkg files to push' even though the rule
now explicitly allows .snupkg too, which can mislead callers and increases the chance of
misconfiguration.

Code

dotnet/private/nuget_push.bzl[R111-115]

        "packages": attr.label_list(
            doc = "The nupkg files to push",
            mandatory = True,
-            allow_files = [".nupkg"],
+            allow_files = [".nupkg", ".snupkg"],
        ),

Evidence

The attr doc claims only .nupkg inputs, but the attribute now allows both .nupkg and .snupkg, making
the API contract unclear to callers.

dotnet/private/nuget_push.bzl[108-115]

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 `packages` attribute docstring says it accepts only `.nupkg` files, but `allow_files` now also includes `.snupkg`.

### Issue Context
This PR intentionally includes `.snupkg` in the runfiles so that `dotnet nuget push <pkg>.nupkg` can auto-detect and publish the matching symbol package.

### Fix Focus Areas
- dotnet/private/nuget_push.bzl[111-115]

### Suggested fix
- Update the docstring to something like: `"The NuGet package files to publish (.nupkg; .snupkg is included for symbol auto-publish)"`.
- (Optional) Rename local variable `nupkg_files` to `package_files` to reflect contents.

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

---

[Copilot]

Pull request overview

Allows .snupkg symbol packages into the nuget_push rule's runfiles so that dotnet nuget push auto-detects and uploads them alongside their .nupkg siblings, while skipping explicit push commands for .snupkg files.

Changes:

• Expand allow_files on the packages attribute to include .snupkg.
• Skip non-.nupkg files when generating push commands in both Unix and Windows script generators.

[qodo-code-review]

1. Silent no-op push 🐞 Bug ≡ Correctness

nuget_push now permits .snupkg inputs but the generated scripts skip any package that is not a
.nupkg; if a caller passes only .snupkg files, the script will have no push commands and will exit
successfully without publishing anything.

Agent Prompt

### Issue description
`nuget_push` accepts `.snupkg` files (via `allow_files`) but both Unix and Windows script generators skip non-`.nupkg` entries. If the input list contains only `.snupkg`, the script ends up with an empty command list and exits successfully, causing a silent “published nothing” failure.

### Issue Context
This PR intentionally filters `.snupkg` out of the push commands and relies on `dotnet nuget push <pkg>.nupkg` to auto-detect and upload `<pkg>.snupkg` if present in the same directory.

### Fix Focus Areas
- dotnet/private/nuget_push.bzl[37-47]
- dotnet/private/nuget_push.bzl[78-89]

### Suggested fix
- After filtering `nupkg_files` down to `.nupkg` entries, **fail loudly** if the resulting list is empty (e.g., `fail("nuget_push: no .nupkg files provided")`).
- (Optional) Use `nupkg.extension == "nupkg"` instead of `basename.endswith(".nupkg")` for clarity.

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

[AutomatedTester]

I think #17467 should be used instead of this PR

[nvborisenko]

dotnet nuget push *.nupkg auto-detects *.snupk. Currently it works as expected, symbols are pushed to the remote.
Covered by #17467 - we can merge this one as minimal changes. Before merging we should "test" whether nupkg and snupkg are located in the same directory.