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

Bump target frameworks of C# programs from netcoreapp1.0 to netcoreapp2.1 #5838

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ pkgconfigdir = $(libdir)/pkgconfig
pkgconfig_DATA = protobuf.pc protobuf-lite.pc

csharp_EXTRA_DIST= \
global.json \
csharp/.gitignore \
csharp/CHANGES.txt \
csharp/Google.Protobuf.Tools.targets \
Expand All @@ -58,7 +59,7 @@ csharp_EXTRA_DIST= \
csharp/build_tools.sh \
csharp/buildall.sh \
csharp/generate_protos.sh \
csharp/global.json \
csharp/install_dotnet_sdk.ps1 \
csharp/keys/Google.Protobuf.public.snk \
csharp/keys/Google.Protobuf.snk \
csharp/keys/README.md \
Expand Down
2 changes: 1 addition & 1 deletion conformance/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ conformance-java-lite: javac_middleman_lite
conformance-csharp: $(other_language_protoc_outputs)
@echo "Writing shortcut script conformance-csharp..."
@echo '#! /bin/sh' > conformance-csharp
@echo 'dotnet ../csharp/src/Google.Protobuf.Conformance/bin/Release/netcoreapp1.0/Google.Protobuf.Conformance.dll "$$@"' >> conformance-csharp
@echo 'dotnet ../csharp/src/Google.Protobuf.Conformance/bin/Release/netcoreapp2.1/Google.Protobuf.Conformance.dll "$$@"' >> conformance-csharp
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why do we call the dll directly instead of dotnet run --no-build?

Copy link
Contributor

Choose a reason for hiding this comment

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

I personally feel more confident that this will really only run the pre-built dll as opposed to to running a dotnet command that can potentially do something extra. No reason to change this in this PR.

@chmod +x conformance-csharp

conformance-php:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFrameworks>net451;netcoreapp1.0</TargetFrameworks>
<TargetFrameworks>net451;netcoreapp2.1</TargetFrameworks>
<AssemblyOriginatorKeyFile>../../keys/Google.Protobuf.snk</AssemblyOriginatorKeyFile>
<SignAssembly>true</SignAssembly>
<PublicSign Condition=" '$(OS)' != 'Windows_NT' ">true</PublicSign>
Expand All @@ -24,7 +24,7 @@
- Visual Studio.
-->
<PropertyGroup Condition="'$(OS)' != 'Windows_NT'">
<TargetFrameworks>netcoreapp1.0</TargetFrameworks>
<TargetFrameworks>netcoreapp2.1</TargetFrameworks>
</PropertyGroup>

</Project>
22 changes: 7 additions & 15 deletions csharp/compatibility_tests/v3.0.0/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@

function run_test() {
# Generate test proto files.
./protoc_1 -Iprotos/src -I../../../src/ --csharp_out=src/Google.Protobuf.Test \
$1 -Iprotos/src -I../../../src/ --csharp_out=src/Google.Protobuf.Test \
--csharp_opt=base_namespace=Google.Protobuf \
protos/src/google/protobuf/unittest_import_proto3.proto \
protos/src/google/protobuf/unittest_import_public_proto3.proto \
protos/src/google/protobuf/unittest_well_known_types.proto

./protoc_1 -Iprotos/csharp --csharp_out=src/Google.Protobuf.Test \
$1 -Iprotos/csharp --csharp_out=src/Google.Protobuf.Test \
--csharp_opt=base_namespace=UnitTest.Issues \
protos/csharp/protos/unittest_issues.proto

./protoc_2 -Iprotos/src --csharp_out=src/Google.Protobuf.Test \
$2 -Iprotos/src --csharp_out=src/Google.Protobuf.Test \
--csharp_opt=base_namespace=Google.Protobuf \
protos/src/google/protobuf/unittest_proto3.proto \
protos/src/google/protobuf/map_unittest_proto3.proto
Expand All @@ -22,7 +22,7 @@ function run_test() {
dotnet restore src/Google.Protobuf.Test/Google.Protobuf.Test.csproj
dotnet build -c Release src/Google.Protobuf/Google.Protobuf.csproj
dotnet build -c Release src/Google.Protobuf.Test/Google.Protobuf.Test.csproj
dotnet run -c Release -f netcoreapp1.0 -p src/Google.Protobuf.Test/Google.Protobuf.Test.csproj
dotnet run -c Release -f netcoreapp2.1 -p src/Google.Protobuf.Test/Google.Protobuf.Test.csproj
}

set -ex
Expand Down Expand Up @@ -79,26 +79,18 @@ cp ../../keys . -r
# Test A.1:
# proto set 1: use old version
# proto set 2 which may import protos in set 1: use old version
cp old_protoc protoc_1
cp old_protoc protoc_2
run_test
run_test "./old_protoc" "./old_protoc"

# Test A.2:
# proto set 1: use new version
# proto set 2 which may import protos in set 1: use old version
cp ../../../src/protoc protoc_1
cp old_protoc protoc_2
run_test
run_test "../../../src/protoc" "./old_protoc"

# Test A.3:
# proto set 1: use old version
# proto set 2 which may import protos in set 1: use new version
cp old_protoc protoc_1
cp ../../../src/protoc protoc_2
run_test
run_test "./old_protoc" "../../../src/protoc"

rm protoc_1
rm protoc_2
rm old_protoc
rm keys -r
rm src/Google.Protobuf -r
5 changes: 0 additions & 5 deletions csharp/global.json

This file was deleted.

21 changes: 21 additions & 0 deletions csharp/install_dotnet_sdk.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/usr/bin/env powershell
# Install dotnet SDK based on the SDK version from global.json

Set-StrictMode -Version 2
$ErrorActionPreference = 'Stop'

# avoid "Unknown error on a send" in Invoke-WebRequest
[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12

$InstallScriptUrl = 'https://dot.net/v1/dotnet-install.ps1'
$InstallScriptPath = Join-Path "$env:TEMP" 'dotnet-install.ps1'
$GlobalJsonPath = Join-Path $PSScriptRoot '..' | Join-Path -ChildPath 'global.json'

# Resolve SDK version from global.json file
$GlobalJson = Get-Content -Raw $GlobalJsonPath | ConvertFrom-Json
$SDKVersion = $GlobalJson.sdk.version

# Download install script
Write-Host "Downloading install script: $InstallScriptUrl => $InstallScriptPath"
Invoke-WebRequest -Uri $InstallScriptUrl -OutFile $InstallScriptPath
&$InstallScriptPath -Version $SDKVersion
2 changes: 1 addition & 1 deletion csharp/src/AddressBook/AddressBook.csproj
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netcoreapp1.0</TargetFramework>
<TargetFramework>netcoreapp2.1</TargetFramework>
<OutputType>Exe</OutputType>
<StartupObject>Google.Protobuf.Examples.AddressBook.Program</StartupObject>
<IsPackable>False</IsPackable>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netcoreapp1.0</TargetFramework>
<TargetFramework>netcoreapp2.1</TargetFramework>
<OutputType>Exe</OutputType>
<IsPackable>False</IsPackable>
</PropertyGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netcoreapp1.0</TargetFramework>
<TargetFramework>netcoreapp2.1</TargetFramework>
<OutputType>Exe</OutputType>
<IsPackable>False</IsPackable>
</PropertyGroup>
Expand Down
4 changes: 2 additions & 2 deletions csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>net451;netcoreapp1.0</TargetFrameworks>
<TargetFrameworks>net451;netcoreapp2.1</TargetFrameworks>
<AssemblyOriginatorKeyFile>../../keys/Google.Protobuf.snk</AssemblyOriginatorKeyFile>
<SignAssembly>true</SignAssembly>
<PublicSign Condition=" '$(OS)' != 'Windows_NT' ">true</PublicSign>
Expand All @@ -24,7 +24,7 @@
- Visual Studio.
-->
<PropertyGroup Condition="'$(OS)' != 'Windows_NT'">
<TargetFrameworks>netcoreapp1.0</TargetFrameworks>
<TargetFrameworks>netcoreapp2.1</TargetFrameworks>
</PropertyGroup>

<ItemGroup>
Expand Down
5 changes: 5 additions & 0 deletions global.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"sdk": {
"version": "2.1.504"
}
}
3 changes: 2 additions & 1 deletion kokoro/linux/csharp/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
# Change to repo root
cd $(dirname $0)/../../..

export DOCKERFILE_DIR=kokoro/linux/64-bit
export DOCKERHUB_ORGANIZATION=protobuftesting
export DOCKERFILE_DIR=kokoro/linux/dockerfile/test/csharp
export DOCKER_RUN_SCRIPT=kokoro/linux/pull_request_in_docker.sh
export OUTPUT_DIR=testoutput
export TEST_SET="csharp"
Expand Down
36 changes: 36 additions & 0 deletions kokoro/linux/dockerfile/test/csharp/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
FROM debian:stretch
Copy link
Contributor Author

@ObsidianMinor ObsidianMinor Mar 7, 2019

Choose a reason for hiding this comment

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

Original was debian:latest, why change it to stretch? Could this cause the issues we're seeing with compatibility testing?

Copy link
Contributor

Choose a reason for hiding this comment

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

currently latest = stretch, but meaning of debian:latest can change over time, and the idea of the docker image is to provide a stable runtime environment, so pinning to a specific version seems as a logical choice.
In the past, when debian:lastest changed from jessie to streche, that broke many of our dockerfiles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only difference I can see between the last successful run I can find (22 days ago) and this one is that the base image was updated from Debian 9.7 to 9.8 but I don't know if that's the issue (I highly doubt it is).

We could just change the script to use path variables rather than copying files. I believe it'd easier to understand and manage that way in the long run.


# Install dependencies. We start with the basic ones require to build protoc
# and the C++ build
RUN apt-get update && apt-get install -y \
autoconf \
autotools-dev \
build-essential \
bzip2 \
ccache \
curl \
gcc \
git \
libc6 \
libc6-dbg \
libc6-dev \
libgtest-dev \
libtool \
make \
parallel \
time \
wget \
&& apt-get clean

# dotnet SDK prerequisites
RUN apt-get update && apt-get install -y libunwind8 libicu57 && apt-get clean

# Install dotnet SDK via install script
RUN wget -q https://dot.net/v1/dotnet-install.sh && \
chmod u+x dotnet-install.sh && \
./dotnet-install.sh --version 2.1.504 && \
ln -s /root/.dotnet/dotnet /usr/local/bin

RUN wget -q www.nuget.org/NuGet.exe -O /usr/local/bin/nuget.exe

ENV DOTNET_SKIP_FIRST_TIME_EXPERIENCE true
5 changes: 5 additions & 0 deletions kokoro/release/csharp/windows/build_nuget.bat
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,9 @@
cd /d %~dp0\..\..\..\..

cd csharp

@rem Install dotnet SDK
powershell -File install_dotnet_sdk.ps1
set PATH=%LOCALAPPDATA%\Microsoft\dotnet;%PATH%

call build_packages.bat