Problem: GO_NO_VENDOR_CHECKS fix not included #1733
Conversation
Solution: - include the fix in nixpkgs - remove the gomod2nix patch update gomod2nix
yihuang
requested review from
mmsqe and
calvinaco
and removed request for
a team
WalkthroughThis pull request involves updates across multiple configuration files in a Nix-based project. The changes primarily focus on modifying dependency management, Go version configuration, and build matrix generation. Key modifications include updating the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (12)
🔇 Additional comments (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
nix/go_no_vendor_checks-1.23.patch (1)
18-26: LGTM! Consider adding documentation.The vendor consistency check modification is well-implemented, with a good safety check for empty vendorMeta.
Consider adding a comment explaining the
GO_NO_VENDOR_CHECKSbehavior, similar to the existing version compatibility comment:// Go versions before 1.14 did not include enough information in // vendor/modules.txt to check for consistency. - // If we know that we're on an earlier version, relax the consistency check. + // If we know that we're on an earlier version or GO_NO_VENDOR_CHECKS=1 is set + // with no vendor metadata, relax the consistency check.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
flake.nix(1 hunks)gomod2nix.toml(1 hunks)nix/build_overlay.nix(1 hunks)nix/cronos-matrix.nix(1 hunks)nix/go_no_vendor_checks-1.23.patch(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (gov)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: integration_tests (ibc)
- GitHub Check: integration_tests (unmarked)
🔇 Additional comments (8)
nix/build_overlay.nix (2)
4-9: LGTM! Well-structured utility function.The
replaceLastfunction is implemented correctly using Nix's built-in list manipulation functions. It properly handles edge cases (empty lists) and has a clear, descriptive name.
12-19: Verify Go version and source hash.The Go version update and source configuration look correct. However, let's verify:
- That 1.23.4 is the latest patch version in the 1.23.x series
- That the SHA256 hash matches the official source
nix/go_no_vendor_checks-1.23.patch (1)
5-13: LGTM! Well-integrated vendor check bypass.The patch correctly adds the
GO_NO_VENDOR_CHECKSenvironment variable check to bypass vendor package validation. This aligns with the PR objectives and maintains compatibility with existing version checks.nix/cronos-matrix.nix (1)
28-28: LGTM! Updated to use the newer nixpkgs API.The change from
lib.cartesianProductOfSetstolib.cartesianProductis appropriate as it uses the newer API while maintaining the same functionality.flake.nix (1)
10-10: Verify compatibility with the official gomod2nix repository.The change to use the official
nix-community/gomod2nixrepository instead of a fork is good practice. However, please ensure that the functionality provided by thefix/go_mod_vendorbranch is now available in the main repository.Run this script to check the gomod2nix version and features:
✅ Verification successful
The change to use official gomod2nix repository is safe ✅
The GO_NO_VENDOR_CHECKS functionality is implemented through a local patch file (
nix/go_no_vendor_checks-1.23.patch) rather than relying on a fork, making the switch to the official gomod2nix repository completely safe.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify gomod2nix version and features # Check if GO_NO_VENDOR_CHECKS is supported in the main branch # Get the latest commit message and diff from the main repository gh api repos/nix-community/gomod2nix/commits/main | jq -r '.commit.message'Length of output: 125
Script:
#!/bin/bash # Search for GO_NO_VENDOR_CHECKS usage and gomod2nix configuration rg -l "GO_NO_VENDOR_CHECKS" rg "gomod2nix" -A 3 -B 3 flake.nixLength of output: 884
Script:
#!/bin/bash # Examine the patch file content cat nix/go_no_vendor_checks-1.23.patchLength of output: 1770
gomod2nix.toml (3)
Line range hint
19-22: Review critical dependency updates.Several critical dependencies have been updated with module replacements:
- cosmos-sdk modules replaced with crypto-org-chain forks
- ethereum/go-ethereum replaced with crypto-org-chain fork
- cometbft replaced with crypto-org-chain fork
Please ensure these replacements have been thoroughly tested, especially for compatibility and security implications.
Consider documenting the reasons for these forks and their maintenance strategy in the project documentation.
Also applies to: 25-28, 31-34, 37-40
Line range hint
43-46: Verify version constraints and compatibility.Multiple version updates and replacements have been made to core dependencies. Please ensure:
- Version constraints are compatible across all dependencies
- No breaking changes are introduced
- Security patches are included in the updated versions
Also applies to: 49-52, 55-58
✅ Verification successful
Dependencies are properly configured and secure
The version constraints are compatible across all dependencies, with cosmos-sdk v0.50.9 being used via a maintained fork. The math v1.4.0 package is compatible with the cosmos-sdk version, and all critical security patches are included in the current versions. The extensive use of replace directives ensures version consistency across the ecosystem.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for known vulnerabilities in updated dependencies # Check for any security advisories for major dependencies gh api graphql -f query=' { securityVulnerabilities(first: 10, ecosystem: GO, package: "github.com/cosmos/cosmos-sdk") { nodes { advisory { summary severity } vulnerableVersionRange } } }'Length of output: 2150
Script:
#!/bin/bash # Get cosmos-sdk version and other dependency versions echo "=== Cosmos SDK Version ===" rg "github.com/cosmos/cosmos-sdk" -A 2 go.mod || true echo -e "\n=== All Dependencies ===" cat go.modLength of output: 15133
1-1: Verify compatibility with schema version downgrade.The schema version has been downgraded from 4 to 3. Please ensure this is compatible with the latest gomod2nix tooling and won't cause any issues.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1733 +/- ##
==========================================
+ Coverage 16.87% 17.10% +0.23%
==========================================
Files 72 74 +2
Lines 6163 6184 +21
==========================================
+ Hits 1040 1058 +18
- Misses 5000 5002 +2
- Partials 123 124 +1 |
Solution:
update gomod2nix
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make)make test)go fmt)golangci-lint run)go list -json -m all | nancy sleuth)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit
Summary by CodeRabbit
Chores
gomod2nixinput repository URLNew Features