Skip to content

Add suppressWarnings option for per-package csc.rsp generation#4

Merged
klumhru merged 1 commit intomainfrom
feat/suppress-warnings
Mar 28, 2026
Merged

Add suppressWarnings option for per-package csc.rsp generation#4
klumhru merged 1 commit intomainfrom
feat/suppress-warnings

Conversation

@klumhru
Copy link
Copy Markdown
Owner

@klumhru klumhru commented Mar 28, 2026

Summary

  • New suppressWarnings config field generates csc.rsp files with -nowarn directives
  • For packages with existing .asmdef files (git-unity, archive-unity): discovers all asmdefs and places csc.rsp next to each
  • For packages where we generate the .asmdef (git-raw, archive-raw): places csc.rsp next to the generated asmdef in Runtime/
  • No-op when suppressWarnings is empty or omitted

Example usage:

{
  "name": "com.google.protobuf",
  "type": "git-raw",
  "suppressWarnings": ["0618", "0649"]
}

Test plan

  • TestWriteCscRsp — single and multiple warnings, empty list produces no file
  • TestWriteCscRspForAsmdefs — discovers multiple asmdefs, places csc.rsp next to each
  • CI passes

🤖 Generated with Claude Code

Generates csc.rsp files with -nowarn directives next to .asmdef files.
For git-raw/archive-raw packages, placed next to the generated asmdef.
For git-unity/archive-unity packages, discovers all existing asmdefs
and places csc.rsp next to each one.

Example config: "suppressWarnings": ["0618", "0649"]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 28, 2026 11:31
@klumhru klumhru merged commit 6496f15 into main Mar 28, 2026
3 checks passed
@klumhru klumhru deleted the feat/suppress-warnings branch March 28, 2026 11:32
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a per-package suppressWarnings configuration option to generate Unity csc.rsp files containing -nowarn directives, and wires it into git/archive packaging flows so warning suppressions can be applied per generated/copied .asmdef.

Changes:

  • Introduces internal/unity helpers to write csc.rsp and to discover .asmdef files and place csc.rsp alongside them.
  • Adds suppressWarnings to config.PackageSpec and integrates csc.rsp generation into git and archive packager paths.
  • Adds unit tests covering csc.rsp content and multi-asmdef placement behavior.

Reviewed changes

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

Show a summary per file
File Description
internal/unity/cscrsp.go New helpers for writing csc.rsp and walking for .asmdef files.
internal/unity/cscrsp_test.go New unit tests for csc.rsp creation and placement.
internal/packager/git.go Hooks suppressWarnings into git packaging flows.
internal/packager/archive.go Hooks suppressWarnings into archive packaging flows.
internal/config/config.go Adds suppressWarnings to the package spec JSON schema.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +60 to +61
os.MkdirAll(runtimeDir, 0755)
os.MkdirAll(editorDir, 0755)
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

Test setup ignores errors from os.MkdirAll. If directory creation fails, later assertions will fail in a harder-to-debug way. Please handle these errors (e.g., if err := os.MkdirAll(...); err != nil { t.Fatal(...) }).

Suggested change
os.MkdirAll(runtimeDir, 0755)
os.MkdirAll(editorDir, 0755)
if err := os.MkdirAll(runtimeDir, 0755); err != nil {
t.Fatalf("failed to create runtimeDir: %v", err)
}
if err := os.MkdirAll(editorDir, 0755); err != nil {
t.Fatalf("failed to create editorDir: %v", err)
}

Copilot uses AI. Check for mistakes.
Comment on lines +63 to +65
os.WriteFile(filepath.Join(runtimeDir, "com.example.runtime.asmdef"), []byte("{}"), 0644)
os.WriteFile(filepath.Join(editorDir, "com.example.editor.asmdef"), []byte("{}"), 0644)

Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

Test setup ignores errors from os.WriteFile when creating the .asmdef fixtures. Please check the returned errors so failures are reported clearly and the test doesn't proceed with missing/partial inputs.

Copilot uses AI. Check for mistakes.
Comment on lines +89 to +91
dir := t.TempDir()
os.WriteFile(filepath.Join(dir, "test.asmdef"), []byte("{}"), 0644)

Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

Test setup ignores the error from os.WriteFile here. Please check it and fail the test if writing the .asmdef fixture fails, to avoid false/unclear failures later in the test.

Copilot uses AI. Check for mistakes.
Comment thread internal/unity/cscrsp.go
return nil
}
content := "-nowarn:" + strings.Join(warnings, ",") + "\n"
return os.WriteFile(filepath.Join(dir, "csc.rsp"), []byte(content), 0644)
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

WriteCscRsp unconditionally overwrites csc.rsp if it already exists. For git-unity/archive Unity packages (or raw sources that already include a csc.rsp), this can clobber existing compiler response directives. Consider detecting an existing file and either merging/appending the -nowarn directive or skipping with a clear error/message so existing settings aren’t lost silently.

Suggested change
return os.WriteFile(filepath.Join(dir, "csc.rsp"), []byte(content), 0644)
path := filepath.Join(dir, "csc.rsp")
f, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0644)
if err != nil {
return err
}
defer f.Close()
_, err = f.WriteString(content)
return err

Copilot uses AI. Check for mistakes.
Comment thread internal/unity/cscrsp.go
return nil
}
return filepath.Walk(rootDir, func(path string, info os.FileInfo, err error) error {
if err != nil || info.IsDir() {
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

WriteCscRspForAsmdefs currently swallows any filepath.Walk error by returning nil when err != nil. That can silently skip parts of the tree and report success even though csc.rsp files weren't written. Return err when it is non-nil so callers can fail fast and surface filesystem issues.

Suggested change
if err != nil || info.IsDir() {
if err != nil {
return err
}
if info.IsDir() {

Copilot uses AI. Check for mistakes.
Comment thread internal/config/config.go
NuGetVersion string `json:"nugetVersion,omitempty"`
NuGetFramework string `json:"nugetFramework,omitempty"`
Exclude []string `json:"exclude,omitempty"`
Exclude []string `json:"exclude,omitempty"`
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

This struct formatting looks like it missed gofmt (the Exclude field is misaligned vs the rest of the struct). Please run gofmt on this file so the formatting is consistent and minimizes diffs/churn going forward.

Suggested change
Exclude []string `json:"exclude,omitempty"`
Exclude []string `json:"exclude,omitempty"`

Copilot uses AI. Check for mistakes.
t.Fatalf("WriteCscRsp: %v", err)
}

data, _ := os.ReadFile(filepath.Join(dir, "csc.rsp"))
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

os.ReadFile error is ignored here (data, _ := ...). If the file read fails, the assertion error will be misleading. Please check err and t.Fatalf like the other tests in this package.

Suggested change
data, _ := os.ReadFile(filepath.Join(dir, "csc.rsp"))
data, err := os.ReadFile(filepath.Join(dir, "csc.rsp"))
if err != nil {
t.Fatalf("reading csc.rsp: %v", err)
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants