Skip to content

buildDotnetModule: support ReadyToRun and native binaries inside nuget files#233422

Merged
SuperSandro2000 merged 6 commits intoNixOS:masterfrom
mdarocha:crossgen-fix
Jun 22, 2023
Merged

buildDotnetModule: support ReadyToRun and native binaries inside nuget files#233422
SuperSandro2000 merged 6 commits intoNixOS:masterfrom
mdarocha:crossgen-fix

Conversation

@mdarocha
Copy link
Contributor

@mdarocha mdarocha commented May 22, 2023 •
edited
Loading

Description of changes

Closes #230848

Closes #181373

This adds crossgen2 nugets to the per-sdk nuget source, allowing them to be found
by the dotnet build process.

Also, we iterate over the entire .nuget folder after restore, and perform patchelf on any native binaries found, allowing them to run properly during the build process. This allows dotnet packages that use crossgen2 (PublishReadyToRun flag), and other nugets using native binaries, like protoc or minver, to work properly without any hacks.

This PR also contains small changes to how the --runtime flag is set, so that hopefully some errors can be avoided automatically (ie. running PublishReadyToRun may require the --runtime flag). The flag is now set when building a .csproj file or when selfContainedBuild is on (as before).

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

This was referenced May 22, 2023
Closed
Closed
@mdarocha mdarocha marked this pull request as draft May 22, 2023 13:54
@github-actions github-actions bot added the 8.has: documentation This PR adds or changes documentation label May 22, 2023
@mdarocha mdarocha changed the title buildDotnetModule: support ReadyToRun and native binaries inside nuget files buildDotnetModule: misc fixes, support ReadyToRun and native binaries inside nuget files May 22, 2023
@mdarocha mdarocha force-pushed the crossgen-fix branch 3 times, most recently from 28f8446 to efe9c7b Compare May 22, 2023 15:22
@mdarocha mdarocha marked this pull request as ready for review May 22, 2023 15:22
@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label May 22, 2023
@ofborg ofborg bot requested review from chuangzhu, jvanbruegge and puffnfresh May 22, 2023 16:54
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels May 22, 2023
@mdarocha mdarocha requested a review from winterqt May 22, 2023 18:07
@nixos-discourse
Copy link

nixos-discourse commented May 23, 2023

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/2257

raphaelr
raphaelr reviewed May 26, 2023
View reviewed changes
Copy link
Contributor

@raphaelr raphaelr left a comment

Choose a reason for hiding this comment

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

dotnet 8.0.nix needs to be regenerated as well (the current preview4 verison is in nixpkgs master now)

Otherwise LGTM

@mdarocha
Copy link
Contributor Author

mdarocha commented May 26, 2023

Updated, should be ready to merge

@mdarocha
Copy link
Contributor Author

mdarocha commented May 26, 2023

35 packages built:
ArchiSteamFarm BeatSaberModManager alttpr-opentracker audiobookshelf boogie btcpayserver btcpayserver-altcoins dafny denaro depotdownloader discordchatexporter-cli eventstore formula galaxy-buds-client inklecate jackett jellyfin kavita marksman msbuild nbxplorer netcoredbg omnisharp-roslyn opentabletdriver osu-lazer pinta ps3-disc-dumper roslyn ryujinx scarab slskd space-station-14-launcher tone wasabibackend xivlauncher

Result of nixpkgs-review run on x86_64-linux 1

35 packages built:
  • ArchiSteamFarm
  • BeatSaberModManager
  • alttpr-opentracker
  • audiobookshelf
  • boogie
  • btcpayserver
  • btcpayserver-altcoins
  • dafny
  • denaro
  • depotdownloader
  • discordchatexporter-cli
  • eventstore
  • formula
  • galaxy-buds-client
  • inklecate
  • jackett
  • jellyfin
  • kavita
  • marksman
  • msbuild
  • nbxplorer
  • netcoredbg
  • omnisharp-roslyn
  • opentabletdriver
  • osu-lazer
  • pinta
  • ps3-disc-dumper
  • roslyn
  • ryujinx
  • scarab
  • slskd
  • space-station-14-launcher
  • tone
  • wasabibackend
  • xivlauncher

@natsukium
Copy link
Member

natsukium commented May 28, 2023

I confirmed that R2R works fine.
Thanks!

@mdarocha mdarocha requested a review from JamieMagee May 29, 2023 10:43
@mweinelt mweinelt added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 2, 2023
@mdarocha mdarocha mentioned this pull request Jun 14, 2023
Closed
2 tasks
@mdarocha mdarocha changed the title buildDotnetModule: misc fixes, support ReadyToRun and native binaries inside nuget files buildDotnetModule: support ReadyToRun and native binaries inside nuget files Jun 19, 2023
@mdarocha mdarocha force-pushed the crossgen-fix branch 2 times, most recently from 99242cf to 0aca53f Compare June 19, 2023 16:30
@mdarocha mdarocha requested review from corngood and layus June 21, 2023 07:34
@mdarocha mdarocha removed the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 21, 2023
@nixos-discourse
Copy link

nixos-discourse commented Jun 21, 2023

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/2356

@mdarocha mdarocha added the 12.approvals: 2 This PR was reviewed and approved by two persons. label Jun 21, 2023
@SuperSandro2000 SuperSandro2000 merged commit 67f4868 into NixOS:master Jun 22, 2023
@mdarocha mdarocha deleted the crossgen-fix branch June 22, 2023 20:49
@MattSturgeon MattSturgeon mentioned this pull request Aug 22, 2024
Merged
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@corngood corngood corngood approved these changes

@IvarWithoutBones IvarWithoutBones Awaiting requested review from IvarWithoutBones

@chuangzhu chuangzhu Awaiting requested review from chuangzhu

@jvanbruegge jvanbruegge Awaiting requested review from jvanbruegge

@puffnfresh puffnfresh Awaiting requested review from puffnfresh

@winterqt winterqt Awaiting requested review from winterqt

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

@JamieMagee JamieMagee Awaiting requested review from JamieMagee

@layus layus Awaiting requested review from layus

+1 more reviewer

@raphaelr raphaelr raphaelr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

8.has: documentation This PR adds or changes documentation 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

buildDotnetModule with ReadyToRun restricts the derivation to a single platform dotnet: using nuget packages with native binaries in them

7 participants

@mdarocha @nixos-discourse @natsukium @raphaelr @corngood @SuperSandro2000 @mweinelt