Increment version numbers on aspnetcore.dll and aspnetcorerh.dll, remove unnecessary files #601

jkotalik · 2018-02-21T20:04:43Z

The aspnetcore.dll was originally version 7.1.xxxx.0, so I needed to increment it to something larger such that the MSI will replace it. I chose 8.1.0.xxxx to somewhat match the current dotnet version.
Currently aspnetcorerh.dll matches the aspnetcore.dll version, but I think we should make the aspnetcorerh.dll match the dotnet version. I can use the VersionPrefix defined in version.props.
I wanted to get the AssemblyRevision from the internal SDK (https://github.com/aspnet/BuildTools/blob/5e70208f6d4862de72a3dab2aa74e82cb3b0481f/src/Internal.AspNetCore.Sdk/build/Common.props#L38) but I don't believe it is available when building vcxprojs. @pranavkm I didn't see a great way of adding AssemblyInfo for C++ projects (ex https://stackoverflow.com/questions/1126880/how-can-i-auto-increment-the-c-sharp-assembly-version-via-our-ci-platform-hudso is only for .net)

natemcmaster · 2018-02-21T21:05:14Z

build/Build.Settings

+  <PropertyGroup>
+    <_TwoDigitYear>$([MSBuild]::Subtract($([System.DateTime]::UtcNow.Year), 2000))</_TwoDigitYear>
+    <_ThreeDigitDayOfYear>$([System.DateTime]::UtcNow.DayOfYear.ToString().PadLeft(3, '0'))</_ThreeDigitDayOfYear>
+    <AssemblyRevision>$(_TwoDigitYear)$(_ThreeDigitDayOfYear)</AssemblyRevision>


So, when we produce more than one build in the same day, won't this lead to duplicate file revisions?

Yeah it does. Were we okay with doing this for other dlls because we use nuget package versions?

AFAIK, we use this pattern elsewhere

Where do we use this? It means our builds are not reproducible, and that two builds with different sources will have the same revision number. Although it may not be a huge problem, it's certainly not a good thing.

I think it is used for all of our dlls today... I am looking at the 2.0.0 installation on my computer and the file version of the dll is exactly this.

Other dotnet dlls have the file version matching the product version.

Ok. This is still bad for reproducible builds. But it is bad in the same way the rest of our stack computes file version, so we can just add this to the list of offenses to be cleaned up when we fix aspnet/BuildTools#452

natemcmaster · 2018-02-21T21:07:08Z

src/RequestHandler/RequestHandler.vcxproj

@@ -231,13 +231,31 @@
    <ClCompile Include="outofprocess\websockethandler.cxx" />
    <ClCompile Include="outofprocess\winhttphelper.cxx" />
  </ItemGroup>
+  <Target Name="CreateVersionHeader" BeforeTargets="PrepareForBuild">


Does this break incremental compilation? I'm not sure if this is a thing in vcsproj.

which build input variable is used in incremental build to control the build number?

This is what used to be in the ANCM repo: https://github.com/aspnet/AspNetCoreModule/blob/e714153782a9fde9a529804efb0bcbf015af20b1/src/AspNetCore/AspNetCore.vcxproj#L193. So it is probably available.

pranavkm

Seems ok to me. Get @natemcmaster's approval and make sure the licensing information is correct

pranavkm · 2018-02-21T22:24:03Z

src/RequestHandler/RequestHandler.vcxproj

+      <VersionHeaderContents Include="#define ProductVersionStr &quot;$(AspNetCoreModuleVersionMajor).$(AspNetCoreModuleVersionMinor).$(AspNetCoreModuleVersionPatch).$(AssemblyRevision)\0&quot;" />
+      <VersionHeaderContents Include="#define PlatformToolset &quot;$(PlatformToolset)\0&quot;" />
+    </ItemGroup>
+    <WriteLinesToFile File="version.h" Lines="@(VersionHeaderContents)" OverWrite="true" />


Consider adding WriteOnlyWhenDifferent="True" to this. Should avoid writing it and breaking increments when the file content hasn't changed. In addition, add the file to writes

<ItemGroup> <FileWrites Include="version.h" /> </ItemGroup>

Prevents msbuild from cleaning up the file.

pranavkm · 2018-02-21T22:24:12Z

src/RequestHandler/RequestHandler.vcxproj

  <ItemGroup>
    <ProjectReference Include="..\CommonLib\CommonLib.vcxproj">
      <Project>{55494e58-e061-4c4c-a0a8-837008e72f85}</Project>
    </ProjectReference>
    <ProjectReference Include="..\IISLib\IISLib.vcxproj">
      <Project>{4787a64f-9a3e-4867-a55a-70cb4b2b2ffe}</Project>
    </ProjectReference>
+  </ItemGroup>
+    <ItemGroup>


pranavkm · 2018-02-21T22:25:27Z

src/RequestHandler/requesthandler.rc

+            VALUE "FileDescription", "IIS ASP.NET Core Module"
+            VALUE "FileVersion", FileVersionStr
+            VALUE "InternalName", "aspnetcorer.dll"
+            VALUE "LegalCopyright", "Copyright (C) 2016"


Has @Eilon verified these?

@Eilon these are the same as the AspNetCoreModule values except I added "Request Handler" and changed the copyright to 2018.

cc/ @shirhatti @muratg for this.

Yeah this doesn't look right to me. I think it should be Copyright (c) .NET Foundation.

pranavkm · 2018-02-21T22:28:14Z

build/Build.Settings

+  <PropertyGroup>
+    <_TwoDigitYear>$([MSBuild]::Subtract($([System.DateTime]::UtcNow.Year), 2000))</_TwoDigitYear>
+    <_ThreeDigitDayOfYear>$([System.DateTime]::UtcNow.DayOfYear.ToString().PadLeft(3, '0'))</_ThreeDigitDayOfYear>
+    <AssemblyRevision>$(_TwoDigitYear)$(_ThreeDigitDayOfYear)</AssemblyRevision>


AFAIK, we use this pattern elsewhere

natemcmaster · 2018-02-21T22:49:50Z

src/RequestHandler/requesthandler.rc

+            VALUE "CompanyName", "Microsoft"
+            VALUE "FileDescription", "IIS ASP.NET Core Module"
+            VALUE "FileVersion", FileVersionStr
+            VALUE "InternalName", "aspnetcorer.dll"


Typo: aspnetcorerh.dll

pan-wang · 2018-02-21T21:25:02Z

src/RequestHandler/requesthandler.rc

+            VALUE "CompanyName", "Microsoft"
+            VALUE "FileDescription", "IIS ASP.NET Core Module"
+            VALUE "FileVersion", FileVersionStr
+            VALUE "InternalName", "aspnetcorer.dll"


this is wrong. should be aspnetcorerh.dll

pan-wang · 2018-02-21T21:25:58Z

src/RequestHandler/requesthandler.rc

+        BLOCK "040904b0"
+        BEGIN
+            VALUE "CompanyName", "Microsoft"
+            VALUE "FileDescription", "IIS ASP.NET Core Module"


we need additional decription info instead of just copy asp.netcore module

pan-wang · 2018-02-21T21:30:41Z

src/RequestHandler/RequestHandler.vcxproj

@@ -231,13 +231,31 @@
    <ClCompile Include="outofprocess\websockethandler.cxx" />
    <ClCompile Include="outofprocess\winhttphelper.cxx" />
  </ItemGroup>
+  <Target Name="CreateVersionHeader" BeforeTargets="PrepareForBuild">


which build input variable is used in incremental build to control the build number?

pan-wang · 2018-02-21T22:26:13Z

src/RequestHandler/requesthandler.rc

+            VALUE "FileDescription", "IIS ASP.NET Core Module"
+            VALUE "FileVersion", FileVersionStr
+            VALUE "InternalName", "aspnetcorer.dll"
+            VALUE "LegalCopyright", "Copyright (C) 2016"


should this one be changed to 2018 @shirhatti

Eilon · 2018-02-22T00:34:47Z

src/RequestHandler/requesthandler.rc

+            VALUE "FileDescription", "IIS ASP.NET Core Module Request Handler"
+            VALUE "FileVersion", FileVersionStr
+            VALUE "InternalName", "aspnetcorerh.dll"
+            VALUE "LegalCopyright", "Copyright (C) 2018"


I think this should be Copyright (c) .NET Foundation or Copyright (c) Microsoft Corporation. No date.

Eilon

I sign off on copyright stuff. Didn't read the code/build changes.

jkotalik added 3 commits February 21, 2018 11:02

Add version information and cleanup files

982c46c

ignore version file

b347912

cleanup

2d4469c

jkotalik requested review from pranavkm, natemcmaster and pan-wang February 21, 2018 20:04

natemcmaster reviewed Feb 21, 2018

View reviewed changes

pranavkm approved these changes Feb 21, 2018

View reviewed changes

natemcmaster reviewed Feb 21, 2018

View reviewed changes

Feedback

71f4344

pan-wang approved these changes Feb 21, 2018

View reviewed changes

Eilon reviewed Feb 22, 2018

View reviewed changes

Feedback

dd0b2f2

Eilon approved these changes Feb 22, 2018

View reviewed changes

natemcmaster approved these changes Feb 22, 2018

View reviewed changes

jkotalik merged commit 4775706 into dev Feb 22, 2018

jkotalik deleted the jkotalik/version branch February 22, 2018 18:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Increment version numbers on aspnetcore.dll and aspnetcorerh.dll, remove unnecessary files #601

Increment version numbers on aspnetcore.dll and aspnetcorerh.dll, remove unnecessary files #601

jkotalik commented Feb 21, 2018

natemcmaster Feb 21, 2018

jkotalik Feb 21, 2018

pranavkm Feb 21, 2018

natemcmaster Feb 21, 2018

jkotalik Feb 22, 2018 •

edited

Loading

jkotalik Feb 22, 2018

natemcmaster Feb 22, 2018

natemcmaster Feb 21, 2018

pan-wang Feb 21, 2018

jkotalik Feb 21, 2018

pranavkm left a comment

pranavkm Feb 21, 2018

pranavkm Feb 21, 2018

pranavkm Feb 21, 2018

jkotalik Feb 21, 2018

jkotalik Feb 21, 2018

Eilon Feb 22, 2018

pranavkm Feb 21, 2018

natemcmaster Feb 21, 2018

pan-wang Feb 21, 2018

pan-wang Feb 21, 2018

pan-wang Feb 21, 2018

pan-wang Feb 21, 2018

Eilon Feb 22, 2018

Eilon left a comment

Increment version numbers on aspnetcore.dll and aspnetcorerh.dll, remove unnecessary files #601

Increment version numbers on aspnetcore.dll and aspnetcorerh.dll, remove unnecessary files #601

Conversation

jkotalik commented Feb 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkotalik Feb 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavkm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Eilon left a comment

Choose a reason for hiding this comment

jkotalik Feb 22, 2018 •

edited

Loading