Refactor MSBuild project files by adding the common property file by iSazonov · Pull Request #4106 · PowerShell/PowerShell · GitHub

Reopen #3917
Related #3400

Motivation

We inherited old assembly file versions from Windows PowerShell.
Currently we shouldn't set versions of assemblies statically in AssemblyInfo.cs files, we should set versions of assemblies dynamically by MSBuild as discussed in #3690.

Fix

Remove AssemblyVersion and AssemblyFileVersionAttribute attributes from AssemblyInfo.cs files.
Put all common properties in PowerShell.Common.props including version related properties.
Import PowerShell.Common.props in all csproj files.

Follow-Up Work

In the PR we temporarily hard code assembly versions as "6.0.0-beta.3".
Later we should set a dll version based on GitCommitId.

Additional considerations

In previous PR #3917 we caught some problems and we should fix it here:

package versions now corrupted (1.0.0) (for help): [Continue to use 'VersionPrefix' tag. 'Version' will be inferred from this tag and all other versions, such as AssemblyVersion, FileVesion and PackageVersion and etc, will be inferred from 'Version']
- set PackageVersion?
- set Version?
SMA.dll backward compatibility: [Reviewed by powershell-committee and agreed to bump AssemblyVersion to 6.0.0.0]
- statically set AssemblyVersion 3.0.00 (AssemblyFileVersionAttribute?)
- dynamically set AssemblyVersion by MSBuild
- any tests?

Note

Quoted from Steve Lee (@SteveL-MSFT):

@PowerShell/powershell-committee reviewed this and is fine with taking the assembly version to 6.0.0.0 and taking it now to discover customer impact

👍 for following DRY principle.

Dongbo Wang (@daxian-dbw) Commit to get PS version from Git is ready. If you want I can add it to the PR. It is only add one new Target.

Ilya (@iSazonov) If the new Target is not huge then add it in and let's review it together.

The new target is very simple.

Also I tried to build a package by

Start-PSBuild -Clean -CrossGen -Runtime win10-x64 -Configuration Release
Start-PSPackage -Type msi

and got PowerShell-6.0.0-beta.3-win10-win2016-x64.msi.

It's good that you verified the package generation. But it's the NuGet package that I'm worried. Without <VersionPrefix>6.0.0</VersionPrefix>, you will get NuGet packages with 1.0.0 in the name.

How can I build the NuGet package?

Ilya (@iSazonov) see document here: https://github.com/PowerShell/PowerShell/blob/master/docs/maintainers/releasing.md#nuget-packages

This regular expression will fail when we finally reach the official v6.0.0 (no -beta.N part anymore).

No plans -rtm?
In any case I can make the part as optional.

The whole <PropertyGroup> block should be indented since it's part of the <Target>

We reverted the commit.

I think $(PS6AdditionalCommits) will never be an empty string, because when there is no additional commit, it should be 0.

Good catch!

Will fix in next PR.

I guess you don't need to hard code them now?

The commit was reverted.

with --long, even if it's right on a tag, you still will get the long version like v6.0.0-beta.3-0-gxxxxxxxxx. However, when we are right on a tag, we are expecting the GitCommitId to be v6.0.0-beta.3.

This is what I get from Microsoft.Win32.Registry.AccessControl.dll:

PS:156> $o | fl ProductVersion, FileVersion, FileName, ProductName


ProductVersion : 4.6.25428.01 built by: dlab-DDVSOWINAGE031. Commit Hash: 644b3ba1fec90828fc31353461b7d1a4aac7ea7f
FileVersion    : 4.6.25428.01
FileName       : C:\Users\dongbow\Downloads\Microsoft.Win32.Registry.AccessControl.dll
ProductName    : Microsoft® .NET Framework

AssemblyInformationalVersion maps to ProductVersion, while AssemblyFileVersion maps to FileVersion, maybe we should do similar thing in powershell assemblies.

I don't think [assembly: AssemblyCulture("")] is necessary. We can remove it.
As for [assembly: NeutralResourcesLanguage("en-US")] it can be replaced by <NeutralLanguage>en-US</NeutralLanguage> in csproj, and I think this tag can go in PowerShell.Common.props

Done.

Same here and in other files. So for this one, the AssemlbyInfo.cs file can be deleted probably.

The some AssemlbyInfo.cs files is empty now.
What is the probability that we will not add personal attributes?

Get the tag commit meta info from the target is good, but it raises a problem -- in our release process, we need to be able to build and package powershell core targeting a tag label, say v6.0.0-beta.4, without actually creating the tag. We added a new parameter -ReleaseTag to Start-PSBuild and Start-PSPackage so that currently we can build and package powershell without a tag but still have the correct $PSVersionTable.GitCommitId. (see https://github.com/PowerShell/PowerShell/blob/master/docs/maintainers/releasing.md#release-steps for more information)

However, with this taget change, the FileVersion, ProductVersion won't be correct without the tag being pushed, so it will break our release process.

The reason to create the tag until finishing preparing the release is that issues may come up when or after build/package, and in that case we need check in fixes. If tag is already pushed, then it's super hard to do the fix.

Given this, I suggest to separate the GetPS6VersionFromGit target out from this PR, let's get the common.props part in first.

@PowerShell/powershell-committee reviewed this and is fine with taking the assembly version to 6.0.0 and taking it now to discover customer impact

Steve Lee (@SteveL-MSFT) Could we add @PowerShell/powershell-committee argumentations to the PR description if in future customers catch any problems with the change?

Also my question is - what is a best practice to ensure assembly backward compatibility? If we follow the best practice, we can recommend the same users to avoid the problem.

Dongbo Wang (@daxian-dbw) Well, I revert last commit. We can easy pass a ReleaseTag value by ```dotnet build ... /p:"PS6BuildVersion=$ReleaseTag" in Build.psm1. I'll add this in next PR.

This reverts commit 7d2b520.

I revert last commit. We can easy pass a ReleaseTag value by ```dotnet build ... /p:"PS6BuildVersion=$ReleaseTag" in Build.psm1. I'll add this in next PR.

Thanks Ilya (@iSazonov)! Appreciate your quick action.

I think <ProductVersion> and InformationalVersion are probably not needed in this PR. The target change can bring them back.

Removed.

I think we need the <VersionPrefix>6.0.0</VersionPrefix> to represent the project version, so that the NuGet package versions are correct.

I agree.
Done.

For clarify. I only found the VersionPrefix in Microsoft.NET.DefaultAssemblyInfo.targets where it define Version property.

Currently, Version composition works as this: If Version is unset, use VersionPrefix (defaults to 1.0.0 if unset) and - if present - append VersionSuffix.
All other version are then defaulted to whatever Version is.

Quoted from StackOverflow

AssemblyVersion and FileVesion are inferred by Version by default. But if they are set, it is considered an override of the default inference.
Not only these 2 versions, ProductVersion and InformationalVersion are also inferred from Version if they are not set. (and ProductVersion is by default the same as InformationVersion)

We probably can remove this file now.

Done.

<GenerateNeutralResourcesLanguageAttribute> should be removed now.

Done.

This file can be removed now.

Done.

This tag is now needed now.

Sorry?

not needed now 😄

Removed.

This tag can be removed now.

Done.

This file can be removed now.

Done.

This file can be removed now.

Done.

This tag can be removed now.

Done.

LGTM. Ilya (@iSazonov) Thanks for your hard work on this and your patience as well.

Dongbo Wang (@daxian-dbw) Thanks for review! I am ready to continue the work in new PR.

…werShell.Common.props' (PowerShell#4106) `AssemblyVersion` and `FileVersion` are now inferred from the `Version` property, which is inferred by `VersionPrefix`. So now both `AssemblyVersion` and `FileVersion` are 6.0.0.0 for each of PowerShell assemblies, and the `ProductVersion` and `InformationalVersion` are 6.0.0.

Update assemblies versions by MSBuild

f178d17

Ilya (iSazonov) requested a review from Dongbo Wang (daxian-dbw) June 26, 2017 17:35

Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label Jun 26, 2017

Dongbo Wang (daxian-dbw) self-assigned this Jun 26, 2017

Ilya (iSazonov) force-pushed the csproj-common-props branch from eee06ac to f178d17 Compare June 26, 2017 17:43

Add GetPS6VersionFromGit target

7d2b520

Ilya (iSazonov) force-pushed the csproj-common-props branch from 2ab1735 to 7d2b520 Compare June 28, 2017 07:30

Steve Lee (SteveL-MSFT) added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Jun 28, 2017

Dongbo Wang (daxian-dbw) reviewed Jun 28, 2017

View reviewed changes

Steve Lee (SteveL-MSFT) added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Jun 28, 2017

Ilya (iSazonov) added 2 commits June 29, 2017 06:16

Revert "Add GetPS6VersionFromGit target"

9e1ba46

This reverts commit 7d2b520.

Move to '<NeutralLanguage>en-US</NeutralLanguage>'

d059267

Dongbo Wang (daxian-dbw) reviewed Jun 29, 2017

View reviewed changes

Remove unneeded files and properties

4f2e4c4

Dongbo Wang (daxian-dbw) approved these changes Jun 29, 2017

View reviewed changes

Dongbo Wang (daxian-dbw) merged commit a8e3d89 into PowerShell:master Jun 29, 2017

Dongbo Wang (daxian-dbw) changed the title Update assemblies versions by MSBuild Refactor MSBuild project files by adding the common property file Jun 29, 2017

Ilya (iSazonov) mentioned this pull request Jul 4, 2017

Refactor MSBuild project files to get PowerShell version from git tag #4182

Merged

Ilya (iSazonov) deleted the csproj-common-props branch July 20, 2017 11:30

-                  <SignAssembly>true</SignAssembly>
-                  <GenerateAssemblyFileVersionAttribute>false</GenerateAssemblyFileVersionAttribute>
-                  <GenerateAssemblyVersionAttribute>false</GenerateAssemblyVersionAttribute>
                   <GenerateNeutralResourcesLanguageAttribute>false</GenerateNeutralResourcesLanguageAttribute>

                   <NoWarn>$(NoWarn);CS1570</NoWarn>
-                  <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
-                  <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
                   <DelaySign>true</DelaySign>

-                  <SignAssembly>true</SignAssembly>
-                  <GenerateAssemblyFileVersionAttribute>false</GenerateAssemblyFileVersionAttribute>
-                  <GenerateAssemblyVersionAttribute>false</GenerateAssemblyVersionAttribute>
                   <GenerateNeutralResourcesLanguageAttribute>false</GenerateNeutralResourcesLanguageAttribute>

-                  <SignAssembly>true</SignAssembly>
-                  <GenerateAssemblyFileVersionAttribute>false</GenerateAssemblyFileVersionAttribute>
-                  <GenerateAssemblyVersionAttribute>false</GenerateAssemblyVersionAttribute>
                   <GenerateNeutralResourcesLanguageAttribute>false</GenerateNeutralResourcesLanguageAttribute>

+                  <PS6AdditionalCommits>$([System.Text.RegularExpressions.Regex]::Match($(PowerShellVersion), $(RegexGitVersion)).Groups[2].Value)</PS6AdditionalCommits>
+                  <PS6CommitSHA>$([System.Text.RegularExpressions.Regex]::Match($(PowerShellVersion), $(RegexGitVersion)).Groups[3].Value)</PS6CommitSHA>
+                  <RegexSymVer>^(\d+).(\d+).(\d+)-(.+)</RegexSymVer>

+                    <Output TaskParameter="ConsoleOutput" PropertyName="PowerShellVersion" />
+                  </Exec>
+                <PropertyGroup>

+                  <PS6PatchVersion>$([System.Text.RegularExpressions.Regex]::Match($(PS6BuildVersion), $(RegexSymVer)).Groups[3].Value)</PS6PatchVersion>
+                  <PS6LabelVersion>$([System.Text.RegularExpressions.Regex]::Match($(PS6BuildVersion), $(RegexSymVer)).Groups[4].Value)</PS6LabelVersion>
+                  <PS6FormattedVersion Condition="'$(PS6AdditionalCommits)' == ''">$(PS6BuildVersion) SHA: $(PS6CommitSHA)</PS6FormattedVersion>

+                  <AssemblyVersion>6.0.0.0</AssemblyVersion>
+                  <ProductVersion>6.0.0-beta.3</ProductVersion>
+                  <InformationalVersion>6.0.0-beta.3</InformationalVersion>

+                    <GitInfoBaseDir Condition="'$(GitInfoBaseDir)' == ''">$(MSBuildProjectDirectory)/$(GitCDUP)/.git</GitInfoBaseDir>
+                  </PropertyGroup>
+                  <Exec Command='$(GitExe) --git-dir="$(GitInfoBaseDir)" describe --abbrev=60 --long' ConsoleToMSBuild="true">

+                  <ProductVersion>$(PS6BuildVersion)</ProductVersion>
+                  <!-- PackageVersion>$(PS6BuildVersion)</PackageVersion -->
+                  <InformationalVersion>$(PS6FormattedVersion)</InformationalVersion>

-              [assembly: AssemblyVersion("3.0.0.0")]
               [assembly: AssemblyCulture("")]
               [assembly: NeutralResourcesLanguage("en-US")]

+                  <FileVersion>6.0.0.0</FileVersion>
+                  <AssemblyVersion>6.0.0.0</AssemblyVersion>
+                  <NeutralLanguage>en-US</NeutralLanguage>

		@@ -1,9 +1,2 @@
		using System.Reflection;
		using System.Resources;

		@@ -1,8 +1,2 @@
		using System.Reflection;
		using System.Resources;

Refactor MSBuild project files by adding the common property file by iSazonov · Pull Request #4106 · PowerShell/PowerShell · GitHub

Uh oh!

Refactor MSBuild project files by adding the common property file - #4106

Refactor MSBuild project files by adding the common property file#4106Dongbo Wang (daxian-dbw) merged 5 commits intoPowerShell:masterPowerShell/PowerShell:masterfrom iSazonov:csproj-common-propsiSazonov/PowerShell:csproj-common-propsCopy head branch name to clipboard

Conversation

Ilya (iSazonov) commented Jun 26, 2017 • edited by daxian-dbw Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Fix

Follow-Up Work

Additional considerations

Note

Uh oh!

Keith Hill (rkeithhill) commented Jun 26, 2017

Uh oh!

Ilya (iSazonov) commented Jun 27, 2017

Uh oh!

Dongbo Wang (daxian-dbw) commented Jun 27, 2017

Uh oh!

Ilya (iSazonov) commented Jun 28, 2017

Uh oh!

Dongbo Wang (daxian-dbw) commented Jun 28, 2017

Uh oh!

Ilya (iSazonov) commented Jun 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dongbo Wang (daxian-dbw) commented Jun 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dongbo Wang (daxian-dbw) Jun 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dongbo Wang (daxian-dbw) commented Jun 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Steve Lee (SteveL-MSFT) commented Jun 28, 2017

Uh oh!

Ilya (iSazonov) commented Jun 29, 2017

Uh oh!

Dongbo Wang (daxian-dbw) commented Jun 29, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dongbo Wang (daxian-dbw) Jun 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Refactor MSBuild project files by adding the common property file#4106
Dongbo Wang (daxian-dbw) merged 5 commits into
PowerShell:masterfrom
iSazonov:csproj-common-props

Ilya (iSazonov) commented Jun 26, 2017 •
edited by daxian-dbw
Loading

Ilya (iSazonov) commented Jun 28, 2017 •
edited
Loading

Dongbo Wang (daxian-dbw) Jun 28, 2017 •
edited
Loading

Dongbo Wang (daxian-dbw) commented Jun 28, 2017 •
edited
Loading

Dongbo Wang (daxian-dbw) Jun 29, 2017 •
edited
Loading