Skip to content

Better codegen: fix .props .targets & bundled packaging#42

Merged
noelex merged 2 commits intonoelex:mainfrom
ha-ves:better-codegen
Sep 25, 2025
Merged

Better codegen: fix .props .targets & bundled packaging#42
noelex merged 2 commits intonoelex:mainfrom
ha-ves:better-codegen

Conversation

@ha-ves
Copy link
Contributor

@ha-ves ha-ves commented Sep 19, 2025

Continuation from #38, this one is packaging for each supported framework.
Per-framework package will use .NET versioning:
MajorNET.MajorRclNET.MinorRclNET.PatchRclNET

Multi-target package is still supported, with detached ros2cs tooling. (I think this is the preferred one)

The ros2cs tooling will be bundled with multi-target Rcl.NET, supporting the lowest version of .NET 8 LTS.

Fix some missing MSBuild properties and provide more flexible targets for library consumers.

(CI/CD part no longer applicable in this PR, will need another PR related to #45)

Works in:

  • Visual Studio Pack [project]
  • CI (incl. github action)
    • by dotnet pack -p:IsPackingSingleTfm=true -p:TargetFramework=<net>
    • (I hadn't explored MSBuild enough to find out if there's way to split TargetFrameworks and do the single packaging from it instead)

Examples here.


Copilot: Pull Request Description

Summary

  • Major refactor and cleanup of build and packaging logic.
  • Dropped support for .NET 7.0 (now only .NET 8.0 and .NET 9.0 are targeted).
  • Simplified and consolidated MSBuild targets and props.
  • Improved CI/CD workflow for NuGet packaging and releases.
  • Decoupled ros2cs tooling from the Rcl.NET package.
  • Enhanced flexibility for consumers and library users.

Details

  • Build System:

    • Moved and consolidated MSBuild logic into Directory.Build.targets and Directory.Build.props.
    • Removed redundant and project-specific build files from build_files/.
    • Updated project files to only target .NET 8.0 and .NET 9.0.
    • Improved per-framework packaging targets.
    • Made projects packable in Visual Studio.
    • Introduced a new MSBuild target (EnsureRos2csTool) to install and use ros2cs as a .NET tool, rather than bundling it with Rcl.NET.
    • Updated Directory.Build.targets to use tool run ros2cs and allow passing custom arguments via Ros2csArgs.
    • Changed ros2cs.csproj to use a single framework property ($(Ros2csFramework)) for more flexible builds.
    • Improved error handling and messaging for missing ros2cs tool.
    • Minor warning message improvements.
    • Cleaned up and removed unnecessary MSBuild logic from project files.
  • CI/CD:

    • Overhauled .github/workflows/release.yml for more concise and flexible release handling.
    • Added workflow dispatch with release type selection.
    • Now uploads all .nupkg files to GitHub tag releases.
  • Other:

    • Removed EOL (end-of-life) .NET versions.
    • General cleanup and modernization of project and build structure.
    • No changes to core library code; all changes are related to build and tooling integration.

@noelex
Copy link
Owner

noelex commented Sep 22, 2025

I think multi-target packaging Rcl.NET is fine. My concern was bundling multi-targeted ros2cs into Rcl.NET may bloat the package if we want to support EOL versions of .NET. This shouldn't be a problem if we're not bundling ros2cs into Rcl.NET.

And I'm not sure installing additional packages during build is a good practice. I think this kind of operation should be done at user's discretion.

I think we should either ask the user to install ros2cs manually (since we can't have tool package as dependency), or bundle ros2cs into Rcl.NET.

If we're taking the bundling approach, we could target lowest supported LTS framework and enabling major roll forward will allow ros2cs to run on newer frameworks.

Or if we're concerned about binary breaking changes, we can still do multi-targeting since we will only need to package no more than 3 target frameworks if we choose to only package for supported versions of .NET.

@ha-ves
Copy link
Contributor Author

ha-ves commented Sep 22, 2025

And I'm not sure installing additional packages during build is a good practice. I think this kind of operation should be done at user's discretion.

Yes, it would have been implied when the user agreed to install Rcl.NET, since it stated explicitly depends on ros2cs.

Or if we're concerned about binary breaking changes, we can still do multi-targeting since we will only need to package no more than 3 target frameworks if we choose to only package for supported versions of .NET.

"Strong Type" breaking changes in .NET seem to have been minimal since .NET 7, but features and performance changes are highly beneficial, though only if the generator outputs files that can use those features. (e.g., LibraryImport instead of directly DllImport, which should improve function call performance by adding generated AOT code for marshalling)

For now, I think bundling the ros2cs executable on the lowest non-EOL .NET version (net8.0) is reasonable, dropping net6.0 and net7.0.

Please let me know if you have more considerations about the interface generation, then I'll close this PR and send in another PR with reduced changes and rebased properly with the main branch.

@noelex
Copy link
Owner

noelex commented Sep 22, 2025

LGTM on changes to ros2cs. Could you please:

  • create a seperate PR for ros2cs changes since these are unrelated to packaging
  • update this PR (or create a new one) to bundle ros2cs targeting net8.0 and drop per-framework packaging as we discussed

Thanks!

* move source ros2cs targets to Directory.build
* simplifies project-agnostic Directory.build
* drops EOL <= net7.0
@ha-ves ha-ves changed the title Better codegen part 2 : Packaging & Versioning Better codegen: fix .props .targets & bundled packaging Sep 23, 2025
@noelex noelex merged commit be5597d into noelex:main Sep 25, 2025
8 checks passed
@ha-ves ha-ves deleted the better-codegen branch September 25, 2025 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants