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

ARROW-4839: [C#] Add NuGet package metadata and instructions. #3891

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ci/appveyor-csharp-build.bat
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,6 @@ pushd csharp

dotnet test || exit /B

dotnet pack -c Release || exit /B

popd
Binary file added csharp/ApacheArrow.snk
Binary file not shown.
42 changes: 42 additions & 0 deletions csharp/Directory.Build.props
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<Project>

<!-- Common repo directories -->
<PropertyGroup>
<RepoRoot>$(MSBuildThisFileDirectory)../</RepoRoot>
<CSharpDir>$(MSBuildThisFileDirectory)</CSharpDir>
<BaseOutputPath>$(CSharpDir)/artifacts/$(AssemblyName)</BaseOutputPath>
</PropertyGroup>

<!-- AssemblyInfo properties -->
<PropertyGroup>
<Product>Apache Arrow library</Product>
<Copyright>2018 Apache Software Foundation</Copyright>
<Company>Apache</Company>
Copy link
Member

Choose a reason for hiding this comment

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

"Apache Software Foundation" may be better.
If we need to change this, we can work on this as a follow-up task.

Copy link
Member

Choose a reason for hiding this comment

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

yes the formal name of the organization is "The Apache Software Foundation"

Copy link
Member

Choose a reason for hiding this comment

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

OK.
@eerhardt Could you create a pull request for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do. Can I use the same JIRA?

Copy link
Member

Choose a reason for hiding this comment

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

It can be a simple PR without a JIRA since it's not notable for the changelog

<VersionPrefix>0.13.0</VersionPrefix>
<VersionSuffix>preview</VersionSuffix>
</PropertyGroup>

<PropertyGroup>
<LangVersion>7.2</LangVersion>
<SignAssembly>true</SignAssembly>
<AssemblyOriginatorKeyFile>$(CSharpDir)ApacheArrow.snk</AssemblyOriginatorKeyFile>
</PropertyGroup>

<!-- NuGet properties -->
<PropertyGroup>
<Authors>Apache</Authors>
<PackageIconUrl>https://www.apache.org/images/feather.png</PackageIconUrl>
<PackageLicenseFile>LICENSE.txt</PackageLicenseFile>
<PackageProjectUrl>https://arrow.apache.org/</PackageProjectUrl>
<PackageTags>apache arrow</PackageTags>
<RepositoryType>git</RepositoryType>
<RepositoryUrl>https://github.com/apache/arrow</RepositoryUrl>
<IncludeSymbols>true</IncludeSymbols>
<SymbolPackageFormat>snupkg</SymbolPackageFormat>
</PropertyGroup>

<ItemGroup Condition="'$(IsPackable)' == 'true'">
<Content Include="$(RepoRoot)LICENSE.txt" Pack="true" PackagePath="" />
</ItemGroup>

</Project>
28 changes: 25 additions & 3 deletions csharp/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,17 +135,39 @@ This implementation is under development and may not be suitable for use in prod

# Build

Install the latest `.NET Core SDK` from https://dotnet.microsoft.com/download.

dotnet build

# Docker Build
## NuGet Build

To build the NuGet package run the following command to build a debug flavor, preview package into the **artifacts** folder.

dotnet pack

When building the officially released version run: (see Note below about current `git` repository)

dotnet pack -c Release -p:VersionSuffix=''

Which will build the final/stable package.

NOTE: When building the officially released version, ensure that your `git` repository has the `origin` remote set to `https://github.com/apache/arrow.git`, which will ensure Source Link is set correctly. See https://github.com/dotnet/sourcelink/blob/master/docs/README.md for more information.

There are two output artifacts:
1. `Apache.Arrow.<version>.nupkg` - this contains the exectuable assemblies
2. `Apache.Arrow.<version>.snupkg` - this contains the debug symbols files

Both of these artifacts can then be uploaded to https://www.nuget.org/packages/manage/upload.

## Docker Build

Build from the Apache Arrow project root.

docker build -f csharp/build/docker/Dockerfile .

# Testing
## Testing

dotnet test test/Apache.Arrow.Tests
dotnet test

All build artifacts are placed in the **artifacts** folder in the project root.

Expand Down
5 changes: 0 additions & 5 deletions csharp/build/Common.props

This file was deleted.

16 changes: 4 additions & 12 deletions csharp/src/Apache.Arrow/Apache.Arrow.csproj
Original file line number Diff line number Diff line change
@@ -1,28 +1,20 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="../../build/Common.props" />

<PropertyGroup>
<TargetFrameworks>netstandard1.3;netcoreapp2.1</TargetFrameworks>
<LangVersion>7.2</LangVersion>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Authors>Apache</Authors>
<Product>Apache Arrow library</Product>
<Copyright>2018 Apache Software Foundation</Copyright>
<PackageProjectUrl>https://fzcorp.visualstudio.com/digital-products</PackageProjectUrl>
<RepositoryUrl>https://fzcorp.visualstudio.com/digital-products/_git/fz-arrow</RepositoryUrl>
<RepositoryType>git</RepositoryType>
<PackageTags>apache arrow</PackageTags>
<Company>Apache</Company>
<Version>0.0.1</Version>
<DefineConstants>$(DefineConstants);UNSAFE_BYTEBUFFER;BYTEBUFFER_NO_BOUNDS_CHECK;ENABLE_SPAN_T</DefineConstants>

<Description>Apache Arrow is a cross-language development platform for in-memory data. It specifies a standardized language-independent columnar memory format for flat and hierarchical data, organized for efficient analytic operations on modern hardware.</Description>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="System.Buffers" Version="4.5.0" />
<PackageReference Include="System.Memory" Version="4.5.1" />
eerhardt marked this conversation as resolved.
Show resolved Hide resolved
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="4.5.1" />
<PackageReference Include="System.Text.Encoding" Version="4.3.0" />
eerhardt marked this conversation as resolved.
Show resolved Hide resolved

<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0-beta2-18618-05" PrivateAssets="All" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp2.1</TargetFramework>
<LangVersion>latest</LangVersion>
</PropertyGroup>

<ItemGroup>
Expand Down
3 changes: 0 additions & 3 deletions csharp/test/Apache.Arrow.Tests/Apache.Arrow.Tests.csproj
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="../../build/Common.props" />

<PropertyGroup>
<TargetFramework>netcoreapp2.1</TargetFramework>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<LangVersion>7.3</LangVersion>
</PropertyGroup>

<ItemGroup>
Expand Down
9 changes: 9 additions & 0 deletions csharp/test/Directory.Build.props
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project>

<Import Project="..\Directory.Build.props" />

<PropertyGroup>
<IsPackable>false</IsPackable>
</PropertyGroup>

</Project>
8 changes: 8 additions & 0 deletions dev/release/00-prepare.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ update_versions() {
git add configure.ac meson.build
cd -

cd "${SOURCE_DIR}/../../csharp"
sed -i.bak -E -e \
"s/^ <VersionPrefix>.+<\/VersionPrefix>/ <VersionPrefix>${version}<\/VersionPrefix>/" \
Directory.Build.props
rm -f Directory.Build.props.bak
git add Directory.Build.props
cd -

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

How about using empty VersionSuffix on release tag? If we use empty VersionSuffix on release tag, we don't need to pass -p:VersionSuffix='' to build release package.

diff --git a/csharp/Directory.Build.props b/csharp/Directory.Build.props
index fde2ea81..bc0eaa17 100644
--- a/csharp/Directory.Build.props
+++ b/csharp/Directory.Build.props
@@ -13,7 +13,7 @@
     <Copyright>2018 Apache Software Foundation</Copyright>
     <Company>Apache</Company>
     <VersionPrefix>0.13.0</VersionPrefix>
-    <VersionSuffix>preview</VersionSuffix>
+    <VersionSuffix>SNAPSHOT</VersionSuffix>
   </PropertyGroup>
 
   <PropertyGroup>
diff --git a/csharp/README.md b/csharp/README.md
index 1d8dbf83..1eeec818 100644
--- a/csharp/README.md
+++ b/csharp/README.md
@@ -147,7 +147,7 @@ To build the NuGet package run the following command to build a debug flavor, pr
 
 When building the officially released version run: (see Note below about current `git` repository)
 
-    dotnet pack -c Release -p:VersionSuffix=''
+    dotnet pack -c Release
 
 Which will build the final/stable package.
 
diff --git a/dev/release/00-prepare.sh b/dev/release/00-prepare.sh
index f8a5eddf..27101976 100755
--- a/dev/release/00-prepare.sh
+++ b/dev/release/00-prepare.sh
@@ -29,10 +29,14 @@ update_versions() {
   case ${type} in
     release)
       local version=${base_version}
+      local csharp_version_prefix=${base_version}
+      local csharp_version_suffix=
       local r_version=${base_version}
       ;;
     snapshot)
       local version=${next_version}-SNAPSHOT
+      local csharp_version_prefix=${next_version}
+      local csharp_version_suffix=SNAPSHOT
       local r_version=${base_version}.9000
       ;;
   esac
@@ -57,8 +61,9 @@ update_versions() {
   cd -
 
   cd "${SOURCE_DIR}/../../csharp"
-  sed -i.bak -E -e \
-    "s/^    <VersionPrefix>.+<\/VersionPrefix>/    <VersionPrefix>${version}<\/VersionPrefix>/" \
+  sed -i.bak -E \
+    -e "s/^    <VersionPrefix>.+<\/VersionPrefix>/    <VersionPrefix>${csharp_version_prefix}<\/VersionPrefix>/" \
+    -e "s/^    <VersionSuffix>.*<\/VersionSuffix>/    <VersionSuffix>${csharp_version_suffix}<\/VersionSuffix>/" \
     Directory.Build.props
   rm -f Directory.Build.props.bak
   git add Directory.Build.props

We don't use any suffix on release tag. For example, https://github.com/apache/arrow/blob/apache-arrow-0.12.1/cpp/CMakeLists.txt#L21 on apache-arrow-0.12.1 tag.

We use "SNAPSHOT" suffix on non release tag. For example,

set(ARROW_VERSION "0.13.0-SNAPSHOT")
on the current master.

How about using "SNAPSHOT" instead of "preview" in C# too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about using "SNAPSHOT" instead of "preview" in C# too?

Good call. I am used to using preview on my team, but since we've already established SNAPSHOT in this project, I will make the name change.

How about using empty VersionSuffix on release tag?

When I started this work, I didn't know how the versioning was handled in this project. But what you describe above makes sense to me. My concerns when I started were:

  1. When a dev builds any given commit on their machine, they don't get a "stable" version NuGet package.
  2. You shouldn't need to change source code in order to build a "stable" package.

However, I now learned that we already have a process for switching out all the version numbers right before "tagging" for a release. This approach makes sense to me. That way if someone pulls the source code for a release and just "builds", they will get a "stable" package, because they built from the tagged commit for that release. I'll make this change, thanks for the feedback.

Copy link
Member

Choose a reason for hiding this comment

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

OK.
It's resonable.

# We can enable this when Arrow JS uses the same version.
# cd "${SOURCE_DIR}/../../js"
# sed -i.bak -E -e \
Expand Down
3 changes: 2 additions & 1 deletion dev/release/rat_exclude_files.txt
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,15 @@ c_glib/doc/plasma-glib/plasma-glib-overrides.txt
c_glib/gtk-doc.make
csharp/.gitattributes
csharp/src/Apache.Arrow/Flatbuf/*
csharp/build/Common.props
csharp/Apache.Arrow.sln
csharp/Directory.Build.props
csharp/src/Apache.Arrow/Apache.Arrow.csproj
csharp/src/Apache.Arrow/Properties/Resources.Designer.cs
csharp/src/Apache.Arrow/Properties/Resources.resx
csharp/test/Apache.Arrow.Benchmarks/Apache.Arrow.Benchmarks.csproj
csharp/test/Apache.Arrow.Tests/Apache.Arrow.Tests.csproj
csharp/test/Apache.Arrow.Tests/app.config
csharp/test/Directory.Build.props
*.html
*.sgml
*.css
Expand Down