From 2c1e4ea76906f1d1cbb5332506228802b9cba24f Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Wed, 7 Mar 2018 08:56:08 -0800 Subject: [PATCH 1/6] Log an error if MSBuildProjectExtensionsPath is modified after it is used --- src/Tasks/Microsoft.Common.CurrentVersion.targets | 10 ++++++++++ src/Tasks/Microsoft.Common.props | 7 +++++++ src/Tasks/Resources/Strings.resx | 2 +- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/Tasks/Microsoft.Common.CurrentVersion.targets b/src/Tasks/Microsoft.Common.CurrentVersion.targets index f85d2936ee7..e0df99c2e03 100644 --- a/src/Tasks/Microsoft.Common.CurrentVersion.targets +++ b/src/Tasks/Microsoft.Common.CurrentVersion.targets @@ -766,6 +766,16 @@ Copyright (C) Microsoft Corporation. All rights reserved. x86 + + + + <_InitialMSBuildProjectExtensionsPath Condition=" '$(ImportProjectExtensionProps)' == 'true' ">$(MSBuildProjectExtensionsPath) diff --git a/src/Tasks/Resources/Strings.resx b/src/Tasks/Resources/Strings.resx index cadec20e0a0..adc1db629d5 100644 --- a/src/Tasks/Resources/Strings.resx +++ b/src/Tasks/Resources/Strings.resx @@ -2681,7 +2681,7 @@ MSB3501 - MSB3510 Task: ReadLinesFromFile MSB3511 - MSB3520 Task: Message MSB3521 - MSB3530 Task: Warning - MSB3531 - MSB3540 Task: Error + MSB3531 - MSB3540 Task: Error // MSB3540 is used in Microsoft.Common.CurrentVersion.targets to log an error, do no reuse MSB3541 - MSB3550 Task: FindUnderPath MSB3551 - MSB3580 Task: GenerateResource // MSB3561, MSB3571 not used but already shipped, do not reuse MSB3581 - MSB3600 Task: ResolveNonMSBuildProjectOutput From 7397e1b45cfc753d6cf06304847a8b75550b28ba Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Tue, 13 Mar 2018 08:37:49 -0700 Subject: [PATCH 2/6] Add warning for BaseIntermediateOutputPath changing Add fwlink --- .../Microsoft.Common.CurrentVersion.targets | 17 ++++++++++++++++- src/Tasks/Microsoft.Common.props | 8 ++------ src/Tasks/Resources/Strings.resx | 2 +- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/Tasks/Microsoft.Common.CurrentVersion.targets b/src/Tasks/Microsoft.Common.CurrentVersion.targets index e0df99c2e03..af7f365289e 100644 --- a/src/Tasks/Microsoft.Common.CurrentVersion.targets +++ b/src/Tasks/Microsoft.Common.CurrentVersion.targets @@ -774,8 +774,23 @@ Copyright (C) Microsoft Corporation. All rights reserved. --> + + + obj\ $(BaseIntermediateOutputPath)\ + <_InitialBaseIntermediateOutputPath>$(BaseIntermediateOutputPath) + $(BaseIntermediateOutputPath) <_InitialMSBuildProjectExtensionsPath Condition=" '$(ImportProjectExtensionProps)' == 'true' ">$(MSBuildProjectExtensionsPath) diff --git a/src/Tasks/Resources/Strings.resx b/src/Tasks/Resources/Strings.resx index adc1db629d5..c3aeffe2ab6 100644 --- a/src/Tasks/Resources/Strings.resx +++ b/src/Tasks/Resources/Strings.resx @@ -2681,7 +2681,7 @@ MSB3501 - MSB3510 Task: ReadLinesFromFile MSB3511 - MSB3520 Task: Message MSB3521 - MSB3530 Task: Warning - MSB3531 - MSB3540 Task: Error // MSB3540 is used in Microsoft.Common.CurrentVersion.targets to log an error, do no reuse + MSB3531 - MSB3540 Task: Error // MSB3539, MSB3540 is used in Microsoft.Common.CurrentVersion.targets, do no reuse MSB3541 - MSB3550 Task: FindUnderPath MSB3551 - MSB3580 Task: GenerateResource // MSB3561, MSB3571 not used but already shipped, do not reuse MSB3581 - MSB3600 Task: ResolveNonMSBuildProjectOutput From 5b8e22fbcc99048af6c18bb939d6f299346d478e Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Tue, 13 Mar 2018 12:26:11 -0700 Subject: [PATCH 3/6] Address comments --- src/Tasks/Microsoft.Common.CurrentVersion.targets | 4 ++-- src/Tasks/Resources/Strings.resx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Tasks/Microsoft.Common.CurrentVersion.targets b/src/Tasks/Microsoft.Common.CurrentVersion.targets index af7f365289e..7f29228ca6a 100644 --- a/src/Tasks/Microsoft.Common.CurrentVersion.targets +++ b/src/Tasks/Microsoft.Common.CurrentVersion.targets @@ -770,7 +770,7 @@ Copyright (C) Microsoft Corporation. All rights reserved. diff --git a/src/Tasks/Resources/Strings.resx b/src/Tasks/Resources/Strings.resx index c3aeffe2ab6..7b093ca9009 100644 --- a/src/Tasks/Resources/Strings.resx +++ b/src/Tasks/Resources/Strings.resx @@ -2681,7 +2681,7 @@ MSB3501 - MSB3510 Task: ReadLinesFromFile MSB3511 - MSB3520 Task: Message MSB3521 - MSB3530 Task: Warning - MSB3531 - MSB3540 Task: Error // MSB3539, MSB3540 is used in Microsoft.Common.CurrentVersion.targets, do no reuse + MSB3531 - MSB3540 Task: Error // MSB3539, MSB3540 is used in Microsoft.Common.CurrentVersion.targets, do not reuse MSB3541 - MSB3550 Task: FindUnderPath MSB3551 - MSB3580 Task: GenerateResource // MSB3561, MSB3571 not used but already shipped, do not reuse MSB3581 - MSB3600 Task: ResolveNonMSBuildProjectOutput From 835980b6d29e1714cf18efe9ef8e3a8c5ffe88e2 Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Mon, 19 Mar 2018 08:57:09 -0700 Subject: [PATCH 4/6] Add ErrorIfChangedInBodyOfProject test --- .../ProjectExtensionsImportTestBase.cs | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs b/src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs index 38dc1447331..18e21d5cc24 100644 --- a/src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs +++ b/src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs @@ -3,6 +3,8 @@ using Microsoft.Build.Evaluation; using System; using System.IO; +using System.Linq; +using Shouldly; using Xunit; namespace Microsoft.Build.UnitTests @@ -148,5 +150,30 @@ public void ImportsProjectIfExists() Assert.Equal("true", project.GetPropertyValue(PropertyNameToEnableImport), StringComparer.OrdinalIgnoreCase); Assert.Equal("true", project.GetPropertyValue(PropertyNameToSignalImportSucceeded), StringComparer.OrdinalIgnoreCase); } + + /// + /// Ensures that an error is logged if MSBuildProjectExtensionsPath is modified after it was set by Microsoft.Common.props. + /// + [Fact] + public void ErrorIfChangedInBodyOfProject() + { + Project project = ObjectModelHelpers.LoadProjectFileInTempProjectDirectory(ObjectModelHelpers.CreateFileInTempProjectDirectory(_projectRelativePath, @" + + + + + foo + + + + + ")); + + MockLogger logger = new MockLogger(); + + project.Build(new[] {logger}).ShouldBeFalse(); + + logger.Errors.Select(i => i.Code).FirstOrDefault().ShouldBe("MSB3540"); + } } -} \ No newline at end of file +} From ef58539a3002dc8cc156c284551ded0455c173ac Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Mon, 19 Mar 2018 09:33:25 -0700 Subject: [PATCH 5/6] Unit test for BaseIntermediateOutputPath --- .../ProjectExtensionsImportTestBase.cs | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs b/src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs index 18e21d5cc24..214abc5e8d2 100644 --- a/src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs +++ b/src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs @@ -171,9 +171,36 @@ public void ErrorIfChangedInBodyOfProject() MockLogger logger = new MockLogger(); - project.Build(new[] {logger}).ShouldBeFalse(); + project.Build("_CheckForInvalidConfigurationAndPlatform", new[] {logger}).ShouldBeFalse(); logger.Errors.Select(i => i.Code).FirstOrDefault().ShouldBe("MSB3540"); } + + /// + /// Ensures that an error is logged if BaseIntermediateOutputPath is modified after it was set by Microsoft.Common.props and + /// EnableBaseIntermediateOutputPathMismatchWarning is 'true'. + /// + [Fact] + public void WarningIfBaseIntermediateOutputPathIsChangedInBodyOfProject() + { + Project project = ObjectModelHelpers.LoadProjectFileInTempProjectDirectory(ObjectModelHelpers.CreateFileInTempProjectDirectory(_projectRelativePath, @" + + + + + true + foo + + + + + ")); + + MockLogger logger = new MockLogger(); + + project.Build("_CheckForInvalidConfigurationAndPlatform", new[] { logger }).ShouldBeTrue(); + + logger.Warnings.Select(i => i.Code).FirstOrDefault().ShouldBe("MSB3539"); + } } } From 4b5f6c32910ece213ea27249052ce31c41ac9715 Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Mon, 19 Mar 2018 09:08:49 -0700 Subject: [PATCH 6/6] Migrate other test cases to Shouldly --- .../ProjectExtensionsImportTestBase.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs b/src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs index 214abc5e8d2..e72c228d93d 100644 --- a/src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs +++ b/src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs @@ -59,10 +59,10 @@ public void DoesNotImportProjectIfNotExist() string projectExtensionsPath = project.GetPropertyValue("MSBuildProjectExtensionsPath"); - Assert.True(!String.IsNullOrWhiteSpace(projectExtensionsPath), "The property 'MSBuildProjectExtensionsPath' should not be empty during project evaluation."); - Assert.True(!Directory.Exists(projectExtensionsPath), $"The project extension directory '{projectExtensionsPath}' should not exist."); - Assert.Equal("true", project.GetPropertyValue(PropertyNameToEnableImport), StringComparer.OrdinalIgnoreCase); - Assert.Equal(String.Empty, project.GetPropertyValue(PropertyNameToSignalImportSucceeded), StringComparer.OrdinalIgnoreCase); + projectExtensionsPath.ShouldNotBeNullOrWhiteSpace(); + Directory.Exists(projectExtensionsPath).ShouldBeFalse(); + project.GetPropertyValue(PropertyNameToEnableImport).ShouldBe("true"); + project.GetPropertyValue(PropertyNameToSignalImportSucceeded).ShouldBeEmpty(); } /// @@ -94,10 +94,10 @@ public void DoesNotImportProjectWhenDisabled() string projectExtensionsDirectory = Path.Combine(ObjectModelHelpers.TempProjectDir, Path.GetDirectoryName(ImportProjectPath)); - Assert.Equal("false", project.GetPropertyValue(PropertyNameToEnableImport), StringComparer.OrdinalIgnoreCase); - Assert.Equal(String.Empty, project.GetPropertyValue(PropertyNameToSignalImportSucceeded), StringComparer.OrdinalIgnoreCase); - Assert.True(Directory.Exists(projectExtensionsDirectory), $"The directory '{projectExtensionsDirectory}' should exist but doesn't."); - Assert.Equal($@"{projectExtensionsDirectory}{Path.DirectorySeparatorChar}", project.GetPropertyValue("MSBuildProjectExtensionsPath")); + project.GetPropertyValue(PropertyNameToEnableImport).ShouldBe("false"); + project.GetPropertyValue(PropertyNameToSignalImportSucceeded).ShouldBeEmpty(); + Directory.Exists(projectExtensionsDirectory).ShouldBeTrue(); + project.GetPropertyValue("MSBuildProjectExtensionsPath").ShouldBe($@"{projectExtensionsDirectory}{Path.DirectorySeparatorChar}"); } /// @@ -123,8 +123,8 @@ public void ImportsProjectIfCustomPath() ")); - Assert.Equal("true", project.GetPropertyValue(PropertyNameToEnableImport), StringComparer.OrdinalIgnoreCase); - Assert.Equal("true", project.GetPropertyValue(PropertyNameToSignalImportSucceeded), StringComparer.OrdinalIgnoreCase); + project.GetPropertyValue(PropertyNameToEnableImport).ShouldBe("true"); + project.GetPropertyValue(PropertyNameToSignalImportSucceeded).ShouldBe("true"); } /// @@ -147,8 +147,8 @@ public void ImportsProjectIfExists() ")); - Assert.Equal("true", project.GetPropertyValue(PropertyNameToEnableImport), StringComparer.OrdinalIgnoreCase); - Assert.Equal("true", project.GetPropertyValue(PropertyNameToSignalImportSucceeded), StringComparer.OrdinalIgnoreCase); + project.GetPropertyValue(PropertyNameToEnableImport).ShouldBe("true"); + project.GetPropertyValue(PropertyNameToSignalImportSucceeded).ShouldBe("true"); } ///