ARROW-4839: [C#] Add NuGet package metadata and instructions. #3891
Conversation
- Add csharp version update to the prepare script. To do this correctly, I decided to refactor the .csproj and .props files so the common msbuild properties are in a root Directory.Build.props file. - Adjust the Description of the Apache.Arrow library. - Add the new xml files to the rat_exclude list.
- Use latest serviced version for dependencies. - Remove one unnecessary dependency.
Codecov Report
@@ Coverage Diff @@
## master #3891 +/- ##
==========================================
+ Coverage 87.78% 87.85% +0.06%
==========================================
Files 727 726 -1
Lines 87892 89340 +1448
Branches 1252 1252
==========================================
+ Hits 77158 78490 +1332
- Misses 10616 10736 +120
+ Partials 118 114 -4
Continue to review full report at Codecov.
|
| rm -f Directory.Build.props.bak | ||
| git add Directory.Build.props | ||
| cd - | ||
|
|
There was a problem hiding this comment.
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.propsWe 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,
Line 21 in 74436f0
How about using "SNAPSHOT" instead of "preview" in C# too?
There was a problem hiding this comment.
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:
- When a dev builds any given commit on their machine, they don't get a "stable" version NuGet package.
- 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.
|
The appveyor error: Is caused by NuGet/Home#7591. This has been fixed in the latest .NET Core SDK version We will either need appveyor to install the latest version - |
Work around NuGet/Home#7591 until CI gets the updated SDK version.
- Handle csharp versioning like the rest of the project using SNAPSHOT suffix for non-release commits, and no suffix for release tags. - Also, fixing a small error with the artifacts folder, AssemblyName isn't set at this time, so use MSBuildProjectName.
|
This looks OK to me at a quick glance, I'll leave to @kou to sign off and merge |
| rm -f Directory.Build.props.bak | ||
| git add Directory.Build.props | ||
| cd - | ||
|
|
| <PropertyGroup> | ||
| <Product>Apache Arrow library</Product> | ||
| <Copyright>2018 Apache Software Foundation</Copyright> | ||
| <Company>Apache</Company> |
There was a problem hiding this comment.
"Apache Software Foundation" may be better.
If we need to change this, we can work on this as a follow-up task.
There was a problem hiding this comment.
yes the formal name of the organization is "The Apache Software Foundation"
There was a problem hiding this comment.
OK.
@eerhardt Could you create a pull request for this?
There was a problem hiding this comment.
Will do. Can I use the same JIRA?
There was a problem hiding this comment.
It can be a simple PR without a JIRA since it's not notable for the changelog
This change adds the necessary changes for creating the
Apache.Arrownuget package..csprojwith what I think is the correct package metadata. Please take a look and give any feedback on what I got wrong.-previewversion of the package. You need to explicitly opt into creating a "stable" version by passing in-p:VersionSuffix=''./cc @stephentoub @ericstj @pgovind @chutchinson @wesm