Skip to content

Include SourceRevisionId in InformationalVersion #2028

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.

Sign up for GitHub

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

Merged
dsplaisted merged 4 commits into from
Mar 16, 2018
Merged

Include SourceRevisionId in InformationalVersion #2028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aee0d72
Include SourceRevisionId in InformationalVersion
tmat Mar 15, 2018
f527b31
one more test
tmat Mar 15, 2018
890fa04
another test
tmat Mar 15, 2018
76c24ce
PR feedback
tmat Mar 15, 2018
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
16 changes: 15 additions & 1 deletion src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.GenerateAssemblyInfo.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<GenerateAssemblyTitleAttribute Condition="'$(GenerateAssemblyTitleAttribute)' == ''">true</GenerateAssemblyTitleAttribute>
<GenerateAssemblyVersionAttribute Condition="'$(GenerateAssemblyVersionAttribute)' == ''">true</GenerateAssemblyVersionAttribute>
<GenerateNeutralResourcesLanguageAttribute Condition="'$(GenerateNeutralResourcesLanguageAttribute)' == ''">true</GenerateNeutralResourcesLanguageAttribute>
<IncludeSourceRevisionInInformationalVersion Condition="'$(IncludeSourceRevisionInInformationalVersion)' == ''">true</IncludeSourceRevisionInInformationalVersion>
</PropertyGroup>

<!--
Expand All @@ -48,8 +49,21 @@ Copyright (c) .NET Foundation. All rights reserved.
DependsOnTargets="PrepareForBuild;CoreGenerateAssemblyInfo"
Condition="'$(GenerateAssemblyInfo)' == 'true'" />

<Target Name="AddSourceRevisionToInformationalVersion"
DependsOnTargets="GetAssemblyVersion;InitializeSourceControlInformation"
Condition="'$(SourceControlInformationFeatureSupported)' == 'true' and '$(IncludeSourceRevisionInInformationalVersion)' == 'true'">
<PropertyGroup Condition="'$(SourceRevisionId)' != ''">
<!-- Follow SemVer 2.0 rules -->
<_InformationalVersionContainsPlus>false</_InformationalVersionContainsPlus>
<_InformationalVersionContainsPlus Condition="$(InformationalVersion.Contains('+'))">true</_InformationalVersionContainsPlus>

<InformationalVersion Condition="!$(_InformationalVersionContainsPlus)">$(InformationalVersion)+$(SourceRevisionId)</InformationalVersion>
<InformationalVersion Condition="$(_InformationalVersionContainsPlus)">$(InformationalVersion).$(SourceRevisionId)</InformationalVersion>
</PropertyGroup>
</Target>

<Target Name="GetAssemblyAttributes"
DependsOnTargets="GetAssemblyVersion">
DependsOnTargets="GetAssemblyVersion;AddSourceRevisionToInformationalVersion">
<ItemGroup>
<AssemblyAttribute Include="System.Reflection.AssemblyCompanyAttribute" Condition="'$(Company)' != '' and '$(GenerateAssemblyCompanyAttribute)' == 'true'">
<_Parameter1>$(Company)</_Parameter1>
Expand Down
170 changes: 170 additions & 0 deletions src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToControlGeneratedAssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
using FluentAssertions;
using System.Runtime.InteropServices;
using Xunit.Abstractions;
using Microsoft.NET.TestFramework.ProjectConstruction;
using System.Xml.Linq;

namespace Microsoft.NET.Build.Tests
{
Expand Down Expand Up @@ -89,6 +91,174 @@ public void It_respects_opt_outs(string attributeToOptOut)
actualInfo.Should().Equal(expectedInfo);
}

[Fact]
public void It_does_not_include_source_revision_id_if_initialize_source_control_target_not_available()
{
TestProject testProject = new TestProject()
{
Name = "ProjectWithSourceRevisionId",
IsSdkProject = true,
TargetFrameworks = "netcoreapp2.0",
};

var testAsset = _testAssetsManager.CreateTestProject(testProject);

testAsset.Restore(Log, testProject.Name);

var command = new GetValuesCommand(Log, Path.Combine(testAsset.TestRoot, testProject.Name), testProject.TargetFrameworks, valueName: "InformationalVersion");
command.Execute().Should().Pass();

command.GetValues().ShouldBeEquivalentTo(new[] { "1.0.0" });
}

[Fact]
public void It_does_not_include_source_revision_id_if_source_revision_id_not_set()
{
TestProject testProject = new TestProject()
{
Name = "ProjectWithSourceRevisionId",
IsSdkProject = true,
TargetFrameworks = "netcoreapp2.0",
};

var testAsset = _testAssetsManager.CreateTestProject(testProject)
.WithProjectChanges((path, project) =>
{
var ns = project.Root.Name.Namespace;

project.Root.Add(
new XElement(ns + "Target",
new XAttribute("Name", "InitializeSourceControlInformation"),
new XElement(ns + "PropertyGroup",
new XElement("SourceRevisionId", ""))));

project.Root.Add(
new XElement(ns + "PropertyGroup",
new XElement("SourceControlInformationFeatureSupported", "true")));
});

testAsset.Restore(Log, testProject.Name);

var command = new GetValuesCommand(Log, Path.Combine(testAsset.TestRoot, testProject.Name), testProject.TargetFrameworks, valueName: "InformationalVersion");
command.Execute().Should().Pass();

command.GetValues().ShouldBeEquivalentTo(new[] { "1.0.0" });
}

[Fact]
public void It_does_not_include_source_revision_id_if_disabled()
{
TestProject testProject = new TestProject()
{
Name = "ProjectWithSourceRevisionId",
IsSdkProject = true,
TargetFrameworks = "netcoreapp2.0",
};

var testAsset = _testAssetsManager.CreateTestProject(testProject)
.WithProjectChanges((path, project) =>
{
var ns = project.Root.Name.Namespace;

project.Root.Add(
new XElement(ns + "Target",
new XAttribute("Name", "InitializeSourceControlInformation"),
new XElement(ns + "PropertyGroup",
new XElement("SourceRevisionId", "xyz"))));

project.Root.Add(
new XElement(ns + "PropertyGroup",
new XElement("SourceControlInformationFeatureSupported", "true"),
new XElement("IncludeSourceRevisionInInformationalVersion", "false")));
});

testAsset.Restore(Log, testProject.Name);

var command = new GetValuesCommand(Log, Path.Combine(testAsset.TestRoot, testProject.Name), testProject.TargetFrameworks, valueName: "InformationalVersion");
command.Execute().Should().Pass();

command.GetValues().ShouldBeEquivalentTo(new[] { "1.0.0" });
}

[Fact]
public void It_includes_source_revision_id_if_available__version_without_plus()
Copy link
Member

@dsplaisted dsplaisted Mar 15, 2018

Choose a reason for hiding this comment

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

Nit: Looks like there's an extra underscore between "available" and "version" in the name here.

Copy link
Member

@dsplaisted dsplaisted Mar 15, 2018

Choose a reason for hiding this comment

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

On the other hand, perhaps this was intentional to separate the different parts of the test name.

You might consider changing these tests to a Theory.

Copy link
Member Author

@tmat tmat Mar 15, 2018

Choose a reason for hiding this comment

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

The tests define different XML elements. I'd rather leave them separate.

{
TestProject testProject = new TestProject()
{
Name = "ProjectWithSourceRevisionId",
IsSdkProject = true,
TargetFrameworks = "netcoreapp2.0",
};

var testAsset = _testAssetsManager.CreateTestProject(testProject)
.WithProjectChanges((path, project) =>
{
var ns = project.Root.Name.Namespace;

project.Root.Add(
new XElement(ns + "Target",
new XAttribute("Name", "_SetSourceRevisionId"),
new XAttribute("BeforeTargets", "InitializeSourceControlInformation"),
new XElement(ns + "PropertyGroup",
new XElement("SourceRevisionId", "xyz"))));

project.Root.Add(
new XElement(ns + "Target",
new XAttribute("Name", "InitializeSourceControlInformation")));

project.Root.Add(
new XElement(ns + "PropertyGroup",
new XElement("SourceControlInformationFeatureSupported", "true")));
Copy link
Member

@dsplaisted dsplaisted Mar 15, 2018

Choose a reason for hiding this comment

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

Please file a follow-up bug for us to remove the SourceControlInformationFeatureSupported property and InitializeSourceControlInformation target from all of these tests once we are using a version of MSBuild that includes them in all places where these tests run (the last one will probably be the full Framework version of MSBuild that we use on CI machines).

Copy link
Member Author

@tmat tmat Mar 15, 2018

Choose a reason for hiding this comment

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

That's an interesting point. Can we run tests against older msbuild common targets? Ideally we would test against the current and a fixed previous version that doesn't have these.

Copy link
Member

@dsplaisted dsplaisted Mar 15, 2018

Choose a reason for hiding this comment

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

I don't think we need to. Generally for customers to be using an SDK with this feature, they will be using an updated version of MSBuild. The only way they wouldn't have an updated version of MSBuild is if they were using full Framework MSBuild and installed the 2.1.300 SDK, but hadn't updated VS/MSBuild to 15.7. That combination might be blocked otherwise anyway.

Copy link
Member Author

@tmat tmat Mar 15, 2018

Choose a reason for hiding this comment

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

If that's the case wouldn't we want to remove the condition on SourceControlInformationFeatureSupported from the product as well?

Copy link
Member

@dsplaisted dsplaisted Mar 15, 2018

Choose a reason for hiding this comment

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

We need it there for now, as the updated version of MSBuild hasn't made its way into the SDK. I would leave it there in general, in order to follow the recommended pattern for hooking into the extension point (since others who use it can't depend on shipping together with MSBuild), and in order to not block the 2.1.300 SDK / MSBuild 15.6 combination just because of this.

Copy link
Member Author

@tmat tmat Mar 15, 2018

Choose a reason for hiding this comment

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

OK

Copy link
Member Author

@tmat tmat Mar 15, 2018

Choose a reason for hiding this comment

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

Filed #2051

});

testAsset.Restore(Log, testProject.Name);

var command = new GetValuesCommand(Log, Path.Combine(testAsset.TestRoot, testProject.Name), testProject.TargetFrameworks, valueName: "InformationalVersion");
command.Execute().Should().Pass();

command.GetValues().ShouldBeEquivalentTo(new[] { "1.0.0+xyz" });
}

[Fact]
public void It_includes_source_revision_id_if_available__version_with_plus()
{
TestProject testProject = new TestProject()
{
Name = "ProjectWithSourceRevisionId",
IsSdkProject = true,
TargetFrameworks = "netcoreapp2.0",
};

var testAsset = _testAssetsManager.CreateTestProject(testProject)
.WithProjectChanges((path, project) =>
{
var ns = project.Root.Name.Namespace;

project.Root.Add(
new XElement(ns + "Target",
new XAttribute("Name", "_SetSourceRevisionId"),
new XAttribute("BeforeTargets", "InitializeSourceControlInformation"),
new XElement(ns + "PropertyGroup",
new XElement("SourceRevisionId", "xyz"))));

project.Root.Add(
new XElement(ns + "Target",
new XAttribute("Name", "InitializeSourceControlInformation")));

project.Root.Add(
new XElement(ns + "PropertyGroup",
new XElement("SourceControlInformationFeatureSupported", "true"),
new XElement("InformationalVersion", "1.2.3+abc")));
});

testAsset.Restore(Log, testProject.Name);

var command = new GetValuesCommand(Log, Path.Combine(testAsset.TestRoot, testProject.Name), testProject.TargetFrameworks, valueName: "InformationalVersion");
command.Execute().Should().Pass();

command.GetValues().ShouldBeEquivalentTo(new[] { "1.2.3+abc.xyz" });
}

[WindowsOnlyTheory]
[InlineData("netcoreapp1.1")]
[InlineData("net45")]
Expand Down
2 changes: 1 addition & 1 deletion src/Tests/Microsoft.NET.TestFramework/TestAssetsManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public TestAsset CreateTestProject(
var project = projectStack.Pop();
if (!createdProjects.Contains(project))
{
project.Create(testAsset, ProjectsRoot);
project.Create(testAsset, ProjectsRoot);
createdProjects.Add(project);

foreach (var referencedProject in project.ReferencedProjects)
Expand Down