Nixify ffi CI#1574
Conversation
Coverage Report for CI Build 26466302919Coverage remained the same at 85.14%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
caarloshenriq
left a comment
There was a problem hiding this comment.
tACK b3d667b on NixOS with act
Approach is clean. Just flagging that 3f76cc7 (DO NOT MERGE test) is still in the branch and left a stray test line in payjoin-ffi/README.md. Worth squashing before merging.
|
|
||
| dotnetSdk = pkgs.dotnetCorePackages.combinePackages [ | ||
| pkgs.dotnetCorePackages.sdk_10_0 | ||
| pkgs.dotnetCorePackages.runtime_8_0 |
There was a problem hiding this comment.
What happens if we stick to dotnet 10? The previous PR touching this file assumed we needed to have donnet 8 compatibility, but we're gonna upgrade BTCPay and the Plugin to use dotnet 10 from now on....
I'm wondering if this had to be brought back because of a regression?
There was a problem hiding this comment.
The error I was getting was due to the inability to find the net_8 apphost file while using the he 1.0 sdk when running ./payjoin-ffi/csharp/contrib/test.sh I think it must have to do with some build step misconfiguration on my end.
/nix/store/60wblmm28bvni14276vvmi3p77hmxw79-dotnet-sdk-10.0.203/share/dotnet/sdk/10.0.203/Microsoft.Common.CurrentVersion.targets(5395,5): error MSB3030: Could not copy the file "/home/runner/work/rust-payjoin/rust-payjoin/payjoin-ffi/csharp/obj/Debug/net8.0/apphost" because it was not found. [/home/runner/work/rust-payjoin/rust-payjoin/payjoin-ffi/csharp/Payjoin.Tests.csproj]
There was a problem hiding this comment.
There was a problem hiding this comment.
Confirmed we can avoid bringing back .NET 8 entirely. I tested the stricter path: move the C# test project to net10.0, drop runtime_8_0 from the Nix shell, and update CI to install .NET 10.
Suggested change:
diff --git a/flake.nix b/flake.nix
@@
- dotnetSdk = pkgs.dotnetCorePackages.combinePackages [
- pkgs.dotnetCorePackages.sdk_10_0
- pkgs.dotnetCorePackages.runtime_8_0
- ];
+ dotnetSdk = pkgs.dotnetCorePackages.sdk_10_0;
diff --git a/payjoin-ffi/csharp/Payjoin.Tests.csproj b/payjoin-ffi/csharp/Payjoin.Tests.csproj
@@
- <TargetFramework>net8.0</TargetFramework>
+ <TargetFramework>net10.0</TargetFramework>
diff --git a/.github/workflows/csharp.yml b/.github/workflows/csharp.yml
@@
- - name: Install .NET 8 SDK
+ - name: Install .NET 10 SDK
uses: actions/setup-dotnet@v4
with:
- dotnet-version: "8.0.x"
+ dotnet-version: "10.0.x"Local verification passes:
nix develop -c dotnet --infoshows only .NET 10 SDK/runtimenix develop -c bash payjoin-ffi/csharp/contrib/test.shpasses: 27 tests undernet10.0git diff --checknix fmt -- --ci
There was a problem hiding this comment.
Got it, the last missing step was the global.json config specifying the sdk version as 8.0.100
14c6773 to
294c783
Compare
This adds individual dev shells for the remaining ffi langs, js, csharp, and dart to match the python dev shell behavior.
This also removes the uneeded rust msrv version env in the python CI.
I realized adding the specific needs for the JS bindings into the global dev shell was mostly a time waster so I opted to refactor the recent additions from #1572 and instead use a javascript specific dev shell, the same applies for the addition of csharp and dart here
Unfortunately a windows build is not really feasibly nixify-able at this time.
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.