Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Publish target for C# builds and upgrade NuGet packages #3451

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

pepone
Copy link
Member

@pepone pepone commented Jan 30, 2025

This PR upgrades C# NuGet packages to include slice2cs compiler in all builds. The PR also adds a new MSBuild target Publish to publish the NuGet package to the local global-packages source.

The BUILDING.md has been updated to reflect this changes.


```shell
msbuild msbuild\ice.proj /t:BuildDist
```
Copy link
Member Author

@pepone pepone Jan 30, 2025

Choose a reason for hiding this comment

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

BuildDist was used before when we have separate targets for sources and tests. Now is just an alias for Build.

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather remove BuildDist and not keep it as an alias for Build.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I would in a follow up PR, need to fix CI builds that relay in all languages to provide a BuildDist target.

</PropertyGroup>

<Choose>
Copy link
Member Author

Choose a reason for hiding this comment

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

The properties below were used to find slice2cs in different platforms. With the new package we will always use the Slice compiler included in the NuGet package.

Copy link
Member

Choose a reason for hiding this comment

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

In the future when we publish to NuGet Gallery, will be include binaries for Linux, macOS, and Windows?

Copy link
Member Author

Choose a reason for hiding this comment

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

Need to add a platform subdir like we do in IceRPC.

Copy link
Member

@externl externl left a comment

Choose a reason for hiding this comment

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

Looks good. How will this work for as a multi platform package?

@pepone
Copy link
Member Author

pepone commented Jan 30, 2025

Looks good. How will this work for as a multi platform package?

I would deal with this in a follow up PR.


```shell
msbuild msbuild\ice.proj /t:BuildDist
```
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather remove BuildDist and not keep it as an alias for Build.

You can publish the package to your local `global-packages` source with the following command:

```shell
dotnet msbuild msbuild\ice.proj /t:Publish
Copy link
Member

Choose a reason for hiding this comment

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

For a follow-up PR: is there anyway we can switch to "dotnet build" and not need the msbuild/ice.proj argument?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need MSBuild for the C++ dependencies. If we build then separately it would be fine to use dotnet.

@pepone pepone merged commit 5d35cf3 into zeroc-ice:main Jan 30, 2025
22 of 23 checks passed
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.

3 participants