Problem: rustc in nixpkgs is too old #1316
Conversation
yihuang
requested review from
calvinaco and
thomas-nguy
and removed request for
a team
yihuang
force-pushed
the
nixpkgs-23-11
branch
from
1b4de2a to
2a12988
Compare
|
Removed dependencies detected. Learn more about Socket for GitHub ↗︎ 🚮 Removed packages: pypi/[email protected], pypi/[email protected] |
WalkthroughThe recent updates focus on refining build configurations and dependencies across various components, emphasizing conditional logic for better compatibility and efficiency. Key adjustments include updates for Changes
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (5)
flake.lockis excluded by:!**/*.lockgo.modis excluded by:!**/*.modintegration_tests/poetry.lockis excluded by:!**/*.lockintegration_tests/pyproject.tomlis excluded by:!**/*.tomlnix/sources.jsonis excluded by:!**/*.json
Files selected for processing (6)
- flake.nix (1 hunks)
- integration_tests/shell.nix (1 hunks)
- nix/build_overlay.nix (1 hunks)
- nix/default.nix (3 hunks)
- nix/go-ethereum.nix (1 hunks)
- nix/testenv.nix (1 hunks)
Additional comments: 7
nix/build_overlay.nix (1)
- 1-3: The changes focusing solely on
rocksdbin the build overlay simplify the configuration. Ensure that the removal ofzstdandgo_1_20does not adversely affect any dependencies or the build process that might still require them.integration_tests/shell.nix (2)
- 17-17: Commenting out
pkgs.dappandpkgs.chain-maindsuggests a reconfiguration of the testing environment. Verify that this does not impact the required integration tests or if these dependencies are being replaced or temporarily removed for troubleshooting.Also applies to: 23-23
- 17-22: Renaming executables for different
solcversions usingrenameExeappears to be a compatibility adjustment for testing with specific Solidity versions. This change seems appropriate for ensuring that tests run with the correct compiler versions.nix/testenv.nix (1)
- 10-10: Updating the
pystarportdependency to include"poetry-core"and"setuptools", and addingflake8-isortwith"hatchling"tobuildSystemsare positive changes that likely improve the Python testing environment. Verify that these updates align with the project's testing workflow and do not introduce any compatibility issues.Also applies to: 16-16
nix/go-ethereum.nix (1)
- 22-22: Renaming
vendorSha256tovendorHashaligns with a broader shift in managing vendor hashes. Ensure that the newvendorHashvalue is correct and does not negatively impact the build process.nix/default.nix (1)
- 16-16: Adding
${sources.poetry2nix}/overlay.nixand updatingcosmovisorandrlywithpkgs.buildGo120ModuleandvendorHashare significant improvements. Ensure that these changes enhance the build reproducibility and dependency management without introducing any compatibility issues.Also applies to: 47-51, 60-60
flake.nix (1)
- 3-3: Updating the NixOS/nixpkgs version to "release-23.11" is a key update aimed at addressing the outdated
rustcissue. Ensure that this update does not introduce any compatibility issues with the project's dependencies or build process.
yihuang
force-pushed
the
nixpkgs-23-11
branch
from
0f6ff8a to
cd438a3
Compare
yihuang
force-pushed
the
nixpkgs-23-11
branch
2 times, most recently
from
6d1df84 to
6fdd202
Compare
no, that one is only for the basic shell, the actual one is pinned internally. |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (5)
flake.lockis excluded by:!**/*.lockgo.modis excluded by:!**/*.modintegration_tests/poetry.lockis excluded by:!**/*.lockintegration_tests/pyproject.tomlis excluded by:!**/*.tomlnix/sources.jsonis excluded by:!**/*.json
Files selected for processing (6)
- flake.nix (1 hunks)
- integration_tests/shell.nix (1 hunks)
- nix/build_overlay.nix (1 hunks)
- nix/default.nix (3 hunks)
- nix/go-ethereum.nix (1 hunks)
- nix/testenv.nix (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- flake.nix
- integration_tests/shell.nix
- nix/build_overlay.nix
- nix/default.nix
- nix/go-ethereum.nix
- nix/testenv.nix
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1316 +/- ##
===========================================
+ Coverage 15.85% 36.31% +20.46%
===========================================
Files 80 118 +38
Lines 6249 10774 +4525
===========================================
+ Hits 991 3913 +2922
- Misses 5178 6481 +1303
- Partials 80 380 +300 |
yihuang
force-pushed
the
nixpkgs-23-11
branch
from
80347f9 to
2c174cd
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (5)
flake.lockis excluded by:!**/*.lockgo.modis excluded by:!**/*.modintegration_tests/poetry.lockis excluded by:!**/*.lockintegration_tests/pyproject.tomlis excluded by:!**/*.tomlnix/sources.jsonis excluded by:!**/*.json
Files selected for processing (6)
- flake.nix (1 hunks)
- integration_tests/shell.nix (1 hunks)
- nix/build_overlay.nix (1 hunks)
- nix/default.nix (3 hunks)
- nix/go-ethereum.nix (1 hunks)
- nix/testenv.nix (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- flake.nix
- integration_tests/shell.nix
- nix/build_overlay.nix
- nix/default.nix
- nix/go-ethereum.nix
- nix/testenv.nix
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- default.nix (1 hunks)
Additional comments: 2
default.nix (2)
- 46-50: The conditional logic in
CGO_LDFLAGSis well-structured and accounts for different scenarios: whenrocksdbis not null, and based on thestaticflag and the platform (Windows or others). However, it's important to ensure that the libraries specified in the flags (-lrocksdb,-pthread,-lstdc++,-ldl,-lzstd,-lsnappy,-llz4,-lbz2,-lz) are compatible with the updatednixpkgsversion and that they are available in the expected versions. This is crucial for maintaining the build's stability and avoiding potential runtime issues due to library incompatibilities.- 52-53: The
postFixupscript for Darwin platforms correctly updates theinstall_nameforlibrocksdb.8.dylibto point to therocksdbpackage's library path. This ensures that the dynamic library is correctly located at runtime on macOS. It's important to verify that the specific version oflibrocksdb(in this case,.8.dylib) matches the version provided by the updatednixpkgsand that this version is indeed required by the application. If the version oflibrocksdbhas changed in the newnixpkgsrelease, this script may need to be updated to reflect the correct library version.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- nix/bundle-win-exe.nix (1 hunks)
Additional comments: 1
nix/bundle-win-exe.nix (1)
- 23-25: The updates to the paths for
libmcfgthread-1.dll,libgcc_s_seh-1.dll, andlibstdc++-6.dllare crucial for ensuring compatibility with the newnixpkgs release-23.11. It's important to verify that these changes align with the available libraries in the updatednixpkgsversion and that they do not introduce any runtime issues for the Windows executable. If not already done, consider testing the application on a Windows environment to ensure these DLLs are correctly loaded and the application functions as expected.
|
@mmsqe a blocker: NixOS/nixpkgs#273442, and the draft fix: NixOS/nixpkgs#256590 |
|
on the other hand, we don't need to do cross-compiling for macos finally: https://github.blog/2023-10-02-introducing-the-new-apple-silicon-powered-m1-macos-larger-runner-for-github-actions/ |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
go.modis excluded by:!**/*.mod
Files selected for processing (4)
- .github/workflows/build.yml (1 hunks)
- .github/workflows/lint.yml (1 hunks)
- .github/workflows/release.yml (2 hunks)
- scripts/release.sh (1 hunks)
Additional comments: 5
scripts/release.sh (1)
- 40-44: The adjustments made to the
hostsvariable for Darwin platforms are a good approach to handling cross-compilation issues between x86_64-darwin and aarch64-darwin. This change is important for ensuring proper platform support and addressing known challenges in cross-compilation.Given the complexity of cross-compilation and the specific issues mentioned (NixOS/nixpkgs#180771 and NixOS/nixpkgs#273442), it would be beneficial to verify the effectiveness of these adjustments in practice. Testing the build process on both x86_64-darwin and aarch64-darwin platforms can help ensure that the changes achieve the desired outcome without introducing new issues.
.github/workflows/release.yml (2)
- 51-53: The introduction of a matrix strategy for macOS runners is a positive change that enhances the flexibility and coverage of the CI/CD process. This approach allows for testing and building on multiple macOS versions, ensuring broader compatibility and catching potential issues early.
Consider adding comments to explain the choice of macOS versions in the matrix, especially if there are specific reasons for including both
macos-latestandmacos-14. This can help maintain clarity and make future updates to the workflow more straightforward.
- 92-96: Updating the checksum file naming logic to include the architecture (
checksums-darwin-$(uname -p).txt) is a thoughtful improvement. It enhances the organization and clarity of build artifacts, making it easier to identify and use the correct files for each architecture.This change aligns well with the goal of supporting multiple architectures and simplifies the process of verifying the integrity of the binaries. It's a good practice to ensure that artifact names reflect their contents or target platforms, especially in projects with diverse target environments.
.github/workflows/lint.yml (1)
- 20-20: Changing the operating system for the
golangcijob tomacos-latestcould offer benefits in terms of compatibility or performance for linting operations. It's important, however, to monitor the impact of this change on CI performance and costs, as macOS runners can be more resource-intensive than their Ubuntu counterparts.Ensure that all dependencies and tools used in the linting process are fully compatible with macOS to avoid any unexpected issues. This change should also be reflected in documentation or CI/CD guidelines to maintain clarity and consistency across the project's workflows.
.github/workflows/build.yml (1)
- 19-19: Including
macos-14in the matrix configuration for operating systems is a proactive step towards ensuring compatibility with the latest macOS version and hardware. This addition helps in early detection of potential compatibility issues and ensures that the project remains accessible to users on the newest platforms.Consider adding comments within the workflow file to explain the rationale behind including
macos-14, especially if there are specific compatibility considerations or testing goals. Monitoring the impact on CI performance and resource usage is also advisable, as testing on multiple macOS versions can increase the overall CI run time and resource consumption.
yihuang
force-pushed
the
nixpkgs-23-11
branch
from
78bcf01 to
6ea2308
Compare
Solution: - update nixpkgs to release-23.11 re-enable dapp update pystarport fix chain-main fix chain-main update chain-main fix build fix win bundle fix release.sh on apple silicon try our luck on mac Problem: cross-compiling macos binaries is problematic in recent nixpkgs Solution: - use the github macos-14 arm mac runner to build arm mac binaries
yihuang
force-pushed
the
nixpkgs-23-11
branch
from
6ea2308 to
db166c4
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (5)
flake.lockis excluded by:!**/*.lockgo.modis excluded by:!**/*.modintegration_tests/poetry.lockis excluded by:!**/*.lockintegration_tests/pyproject.tomlis excluded by:!**/*.tomlnix/sources.jsonis excluded by:!**/*.json
Files selected for processing (12)
- .github/workflows/build.yml (1 hunks)
- .github/workflows/lint.yml (1 hunks)
- .github/workflows/release.yml (2 hunks)
- default.nix (1 hunks)
- flake.nix (1 hunks)
- integration_tests/shell.nix (1 hunks)
- nix/build_overlay.nix (1 hunks)
- nix/bundle-win-exe.nix (1 hunks)
- nix/default.nix (3 hunks)
- nix/go-ethereum.nix (1 hunks)
- nix/testenv.nix (1 hunks)
- scripts/release.sh (1 hunks)
Files skipped from review as they are similar to previous changes (11)
- .github/workflows/lint.yml
- .github/workflows/release.yml
- default.nix
- flake.nix
- integration_tests/shell.nix
- nix/build_overlay.nix
- nix/bundle-win-exe.nix
- nix/default.nix
- nix/go-ethereum.nix
- nix/testenv.nix
- scripts/release.sh
Additional comments: 1
.github/workflows/build.yml (1)
- 19-19: Adding
macos-14to the OS matrix is a good move for ensuring compatibility with the latest macOS version. However, it's important to verify that all steps in thebuildjob, especially those involving external actions or dependencies, are compatible withmacos-14. This includes actions likeactions/checkout@v3,cachix/install-nix-action@v23, and any scripts or commands run during the job. Compatibility issues could lead to build failures or unexpected behavior on this new OS version.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (5)
flake.lockis excluded by:!**/*.lockgo.modis excluded by:!**/*.modintegration_tests/poetry.lockis excluded by:!**/*.lockintegration_tests/pyproject.tomlis excluded by:!**/*.tomlnix/sources.jsonis excluded by:!**/*.json
Files selected for processing (10)
- .github/workflows/lint.yml (1 hunks)
- default.nix (1 hunks)
- flake.nix (1 hunks)
- integration_tests/shell.nix (1 hunks)
- nix/build_overlay.nix (1 hunks)
- nix/bundle-win-exe.nix (1 hunks)
- nix/default.nix (3 hunks)
- nix/go-ethereum.nix (1 hunks)
- nix/testenv.nix (1 hunks)
- scripts/release.sh (1 hunks)
Files skipped from review as they are similar to previous changes (10)
- .github/workflows/lint.yml
- default.nix
- flake.nix
- integration_tests/shell.nix
- nix/build_overlay.nix
- nix/bundle-win-exe.nix
- nix/default.nix
- nix/go-ethereum.nix
- nix/testenv.nix
- scripts/release.sh
Solution:
👮🏻👮🏻👮🏻 !!!! 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