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

Inject into correct point in build pipeline for VS 2017 #1119

Merged
merged 5 commits into from
Feb 21, 2017

Conversation

clairernovotny
Copy link

@clairernovotny clairernovotny commented Dec 15, 2016

This is an initial attempt at implementing the .NET Desktop side of #1118.

Notably, it's a bit "whack a mole" in terms of ensuring the GetVersion target is executed early enough. Then, without trying to overwrite what the user wants, the user will have to add something like this into their project file in order to copy the generated values into the correct version variables:

  <Target Name="UpdateVersionVars" AfterTargets="GetVersion">
    <PropertyGroup>
      <Version>$(GitVersion_SemVer)</Version>
      <PackageVersion>$(GitVersion_SemVer)</PackageVersion>
    </PropertyGroup>
  </Target>

Copying to Version at that stage will ensure that the AssemblyFileVersion and AssemblyVersion are set based on that Version turns into the informational version. I'm not sure if you can use the FullSemVer there or if that'll cause the parsing to barf.... either way, the user is in control of what variables they assign to what there.

If there's a better way to do this than to have the user supply an AfterTargets, then I'm all for it. I tested this locally and it did correctly generate the file/assembly versions and the package version when using the pack command.

@clairernovotny
Copy link
Author

Related to this in terms of making it easier to extend the Version tag dotnet/sdk#504

@clairernovotny
Copy link
Author

I've made a few updates:

  1. Just works now by default in all projects. In "SDK" projects, UpdateAssemblyInfo gets defaulted to off and instead it'll set the appropriate MSBuild properties that other tasks use to generate the assembly info and nuget packaging. Of course, this can be turned off.
  2. It can optionally use the full semver 2.0 in the nuget packaging. This is off by default since many servers don't yet support it (I believe the symbolsource stuff breaks currently). You can set <UseFullSemVerForNuGet>true</UseFullSemVerForNuGet> in your project file to opt-in to using SemVer 2.0 package versions.

@clairernovotny clairernovotny force-pushed the vs2017 branch 2 times, most recently from e71e0c6 to 5083cad Compare December 21, 2016 18:33
@asbjornu
Copy link
Member

@onovotny: Is this backwards compatible with VS2015?

@clairernovotny
Copy link
Author

clairernovotny commented Dec 30, 2016 via email

@thoemmi
Copy link
Contributor

thoemmi commented Feb 9, 2017

Any news on this? I have a new VS2017 RC project cross-targeting net40 and net45:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>net40;net45</TargetFrameworks>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="GitVersionTask" Version="4.0.0-beta0009" />
  </ItemGroup>
  <!-- ... -->

The compile failes because of duplicate Assembly*VersionAttributes.

1>obj\Debug\net40\StickyWindows.AssemblyInfo.cs(17,12,17,58): error CS0579: Duplicate 'System.Reflection.AssemblyFileVersionAttribute' attribute
1>obj\Debug\net40\StickyWindows.AssemblyInfo.cs(18,12,18,67): error CS0579: Duplicate 'System.Reflection.AssemblyInformationalVersionAttribute' attribute
1>obj\Debug\net40\StickyWindows.AssemblyInfo.cs(21,12,21,54): error CS0579: Duplicate 'System.Reflection.AssemblyVersionAttribute' attribute

@clairernovotny
Copy link
Author

@thoemmi The fix to that is to remove those attributes from your AssemblyInfo file. That file is ideally completely generated now anyway by attributes in the csproj.

@thoemmi
Copy link
Contributor

thoemmi commented Feb 10, 2017

@onovotny Wisecracker 😉 Of course I already would have done that if there were an AssemblyInfo file.
It's a MSBuild "15" project, and there's no AssemblyInfo.cs.

Anyway, I think I can manage to create the correct version information. My current .csproj looks like this

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>net40;net45</TargetFrameworks>

    <UpdateAssemblyInfo>false</UpdateAssemblyInfo>
    <GeneratePackageOnBuild>True</GeneratePackageOnBuild>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="GitVersionTask" Version="4.0.0-beta0009" />
  </ItemGroup>
  <Target Name="UpdateVersionVars" AfterTargets="GetVersion">
    <Message Text="$(GitVersion_SemVer)" Importance="High" />
    <PropertyGroup>
      <FileVersion>$(GitVersion_AssemblySemVer)</FileVersion>
      <AssemblyVersion>$(GitVersion_AssemblySemVer)</AssemblyVersion>
      <PackageVersion>$(GitVersion_SemVer)</PackageVersion>
      <InformationalVersion>$(GitVersion_SemVer)</InformationalVersion>
    </PropertyGroup>
  </Target>
  <!-- ... -->

The <UpdateAssemblyInfo>false</UpdateAssemblyInfo> says GitVersion ot to create an AssemblyInfo on its own. Instead, MSBuild uses a common set of properties: FileVersion, AssemblyVersion, PackageVersion, and InformationalVersion.
This works except PackageVersion: The generated Nupgk still uses the AssemblySemVer. I'll continue exploring MSBuild's .targets files, and post an update when I have found a solution.

Copy link
Member

@pascalberger pascalberger left a comment

Choose a reason for hiding this comment

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

@onovotny I've added two comments inline. Additionally it would be great if you can update the documentation here with the new properties and targets.


<GetVersion Condition=" '$(GetVersion)' == '' ">true</GetVersion>


Copy link
Member

Choose a reason for hiding this comment

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

Remove multiple empty lines

@@ -79,9 +79,8 @@
</ItemGroup>
<ItemGroup>
<None Include="app.config" />
<None Include="NugetAssets\GitVersionTask.targets">
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the file NugetAssets\GitVersionTask.targets be deleted?

Copy link
Author

Choose a reason for hiding this comment

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

it was moved into the build folder

Copy link
Member

Choose a reason for hiding this comment

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

Ah, missed this :)

<UpdateAssemblyInfo Condition=" '$(UpdateAssemblyInfo)' == '' ">true</UpdateAssemblyInfo>

<!-- Property that enables setting of Version -->
<UpdateVersionProperties Condition=" '$(UpdateVersionProperties)' == '' ">true</UpdateVersionProperties>
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice if these properties can be documented here

Copy link
Author

Choose a reason for hiding this comment

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

will update the docs now

@clairernovotny
Copy link
Author

I've added docs - I think I've covered it?

@pascalberger
Copy link
Member

@onovotny Thanks LGTM. I'll merge it as soon as the build has passed.

@clairernovotny
Copy link
Author

Once GitVersion supports CoreCLR and linux/mac, we can include a .net core version of these as well. I think LibGit2Sharp has that support now? I think NerdBank.GitVersioning is using that and he has a coreclr task.

@pascalberger pascalberger merged commit cc322ff into GitTools:master Feb 21, 2017
@pascalberger
Copy link
Member

@onovotny It's merged now. Thanks for your contribution!

@clairernovotny clairernovotny deleted the vs2017 branch February 21, 2017 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants