go: support overriding more pinned go module versions

[sandydoo]

Fixes #2138.

[github-actions]

🔍 Suggested Reviewers

Based on git blame analysis of the changed lines, the following contributors have significant experience with the modified code:

• @shyim - 83.3% of changed lines (5 lines)

Please consider reviewing this PR as you have authored significant portions of the code being modified. Your expertise would be valuable! 🙏

This comment was automatically generated by git-blame-auto-reviewer

Last updated: 2025-09-10T12:04:30.438Z

[claude]

Claude finished @sandydoo's task —— View job

---

PR Review Complete ✅

Reviewing PR #2142: go: support overriding more pinned go module versions

Todo List:

• Read and analyze the modified file src/modules/languages/go.nix
• Understand the context of issue latest delve from nixpkgs/nixos-unstable is not supported by languages.go #2138
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage
• Consider the regex suggestion from @marcusramberg
• Provide constructive feedback

---

Code Review Feedback

🎯 Excellent Improvement - Regex-Based Solution

The evolution from the first commit (hardcoded version checks) to the second commit (regex-based matching) is a significant improvement. The current implementation using lib.match "buildGo.*Module" is much more maintainable and future-proof.

🔧 Areas for Improvement

1. Regex Pattern Precision

# Current: src/modules/languages/go.nix:14
goModuleArgs = lib.filterAttrs (name: _: lib.match "buildGo.*Module" name != null) overrideArgs;

The regex "buildGo.*Module" might be too permissive. Consider:

• "buildGo[0-9.]*Module" - to only match version patterns
• "buildGo(Latest|[0-9]+)?Module" - to explicitly match expected patterns

This prevents false positives if nixpkgs ever introduces unrelated attributes like buildGoSomeOtherModule.

2. Performance Concern

# src/modules/languages/go.nix:15
goModuleOverrides = lib.mapAttrs (_: _: buildGoModule) goModuleArgs;

Every matched argument gets the same buildGoModule value. This is inefficient but functionally correct. Consider adding a comment explaining why this redundancy is acceptable.

3. Error Message Clarity

# src/modules/languages/go.nix:21-25
throw ''
  `languages.go` failed to override the Go version for ${pkg.pname or "unknown"}.
  Expected to find a `buildGo*Module` argument in its override function.

  Found: ${toString (lib.attrNames overrideArgs)}
'';

The error message could be more actionable. Consider:

throw ''
  `languages.go` failed to override the Go version for ${pkg.pname or "unknown"}.
  Expected to find a `buildGo*Module` argument (e.g., buildGoModule, buildGoLatestModule, buildGo123Module).
  
  Available override arguments: ${toString (lib.attrNames overrideArgs)}
  
  This package may need updates in nixpkgs to support Go version overriding.
'';

4. Test Coverage Gap
No tests found for the Go language module. Consider adding:

• Basic functionality test that verifies Go environment setup
• Test that verifies version overriding works for packages with different buildGo*Module patterns
• Error handling test for packages without Go module arguments

5. Documentation Enhancement
The helper function comment at src/modules/languages/go.nix:8-10 is good but could mention the regex matching behavior:

# A helper function to rebuild a package with the specific Go version.
# It expects the package to have a `buildGo*Module` argument in its override function.
# Uses regex matching to find all buildGo*Module variants (buildGoModule, buildGo123Module, etc).
# This will override multiple buildGo*Module arguments if they exist.

✅ What Works Well

• Future-proof design: The regex approach eliminates the need to hardcode specific Go versions
• Multiple override support: Correctly handles packages with multiple buildGo*Module arguments
• Clean error handling: Provides useful debugging information when overrides fail
• Backward compatibility: Works with existing buildGoModule and buildGoLatestModule patterns

🏁 Recommendation

This is a solid improvement over the hardcoded approach. The main areas to address are:

1. Tightening the regex pattern for better specificity
2. Adding comprehensive tests
3. Minor error message improvements

The approach addresses @marcusramberg's regex suggestion effectively and solves the maintainability issues from the initial hardcoded implementation.

---

[cloudflare-workers-and-pages]

Deploying devenv with    Cloudflare Pages

Latest commit:	2621a60
Status:	🚫  Build failed.

View logs

[domenkozar]

Looks like most ci jobs fail

[sandydoo]

Looks like most ci jobs fail

Neat

[marcusramberg]

Did you consider using matches with a regex here to make it more future-proof? (get the overrideattrs as a list and use single after matching?

[sandydoo]

Did you consider using matches with a regex here to make it more future-proof? (get the overrideattrs as a list and use single after matching?

Yes, I'm considering it.