mirrors/dotnet-installer
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix 5466: explicity including a file causes a build break (#5475)

Browse source 
* fix 5466: explicity including a file causes a build break

* fix failing unit tests

* Add unit tests and apply fix also to includeFiles

* apply review feedback
This commit is contained in:
Krzysztof Wicher 2017-01-27 18:39:44 -08:00 committed by Piotr Puszkiewicz
parent ba69c88c79
commit d6d39a5c9d
6 changed files with 112 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
TestAssets/TestProjects/TestAppWithExplicitInclude/Program.cs Normal file
View file

@ -0,0 +1,9 @@
using System;
class Program
{
static void Main(string[] args)
{
Console.WriteLine("Hello World!");
}
}

24
TestAssets/TestProjects/TestAppWithExplicitInclude/project.json Normal file
View file

@ -0,0 +1,24 @@
{
"version": "1.0.0-*",
"buildOptions": {
"debugType": "portable",
"emitEntryPoint": true,
"compile": {
"include": [
"Program.cs"
]
}
},
"dependencies": {},
"frameworks": {
"netcoreapp1.0": {
"dependencies": {
"Microsoft.NETCore.App": {
"type": "platform",
"version": "1.0.1"
}
},
"imports": "dnxcore50"
}
}
}

13
src/Microsoft.DotNet.ProjectJsonMigration/Rules/MigrateBuildOptionsRule.cs
View file

@ -114,11 +114,22 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
"**/*.cs"
};
private static bool IsPlainFileName(string fileName)
{
return !fileName.Contains('/') && !fileName.Contains('\\');
}
private bool CompileFilesExcludeRule(string pattern)
{
return _compilePatternsToExclude.Contains(pattern.Replace('\\', '/'))
|| IsPlainFileName(pattern);
}
private IncludeContextTransform CompileFilesTransform =>
new IncludeContextTransform(
"Compile",
transformMappings: false,
patternsToExclude: _compilePatternsToExclude,
excludePatternsRule: CompileFilesExcludeRule,
condition: ic => ic != null,
emitBuiltInIncludes: false);

21
src/Microsoft.DotNet.ProjectJsonMigration/transforms/IncludeContextTransform.cs
View file

@ -19,16 +19,13 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Transforms
(itemName) =>
new AddItemTransform<IncludeContext>(
itemName,
includeContext => FormatGlobPatternsForMsbuild(includeContext.IncludeFiles, includeContext.SourceBasePath),
includeContext => FormatGlobPatternsForMsbuild(includeContext.IncludeFiles.OrEmptyIfNull()
.Where((pattern) => !_excludePatternRule(pattern)),
includeContext.SourceBasePath),
includeContext => FormatGlobPatternsForMsbuild(includeContext.ExcludeFiles, includeContext.SourceBasePath),
includeContext => includeContext != null
&& includeContext.IncludeFiles != null
&& includeContext.IncludeFiles.Count > 0);
private bool IsPatternExcluded(string pattern)
{
return _patternsToExclude.Contains(pattern.Replace('\\', '/'));
}
&& includeContext.IncludeFiles.Where((pattern) => !_excludePatternRule(pattern)).Count() > 0);
protected virtual Func<string, AddItemTransform<IncludeContext>> IncludeExcludeTransformGetter =>
(itemName) => new AddItemTransform<IncludeContext>(
@ -41,7 +38,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Transforms
fullIncludeSet = fullIncludeSet.Union(includeContext.BuiltInsInclude.OrEmptyIfNull());
}
fullIncludeSet = fullIncludeSet.Where((pattern) => !IsPatternExcluded(pattern));
fullIncludeSet = fullIncludeSet.Where((pattern) => !_excludePatternRule(pattern));
return FormatGlobPatternsForMsbuild(fullIncludeSet, includeContext.SourceBasePath);
},
@ -57,7 +54,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Transforms
{
return includeContext != null &&
(
(includeContext.IncludePatterns != null && includeContext.IncludePatterns.Where((pattern) => !IsPatternExcluded(pattern)).Count() > 0)
(includeContext.IncludePatterns != null && includeContext.IncludePatterns.Where((pattern) => !_excludePatternRule(pattern)).Count() > 0)
||
(_emitBuiltInIncludes &&
includeContext.BuiltInsInclude != null &&
@ -75,7 +72,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Transforms
private readonly string _itemName;
private bool _transformMappings;
private string[] _patternsToExclude;
private Func<string, bool> _excludePatternRule;
private bool _emitBuiltInIncludes;
private readonly List<ItemMetadataValue<IncludeContext>> _metadata = new List<ItemMetadataValue<IncludeContext>>();
@ -84,12 +81,12 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Transforms
bool transformMappings = true,
Func<IncludeContext, bool> condition = null,
bool emitBuiltInIncludes = true,
string[] patternsToExclude = null) : base(condition)
Func<string, bool> excludePatternsRule = null) : base(condition)
{
_itemName = itemName;
_transformMappings = transformMappings;
_emitBuiltInIncludes = emitBuiltInIncludes;
_patternsToExclude = patternsToExclude ?? Array.Empty<string>();
_excludePatternRule = excludePatternsRule ?? ((pattern) => false);
_mappingsToTransfrom = (addItemTransform, targetPath) =>
{

56
test/Microsoft.DotNet.ProjectJsonMigration.Tests/Rules/GivenThatIWantToMigrateBuildOptions.cs
View file

@ -420,12 +420,13 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
}
[Theory]
[InlineData("compile", "Compile")]
[InlineData("embed", "EmbeddedResource")]
[InlineData("copyToOutput", "Content")]
[InlineData("compile", "Compile", "")]
[InlineData("embed", "EmbeddedResource", ";rootfile.cs")]
[InlineData("copyToOutput", "Content", ";rootfile.cs")]
private void MigratingGroupIncludeExcludePopulatesAppropriateProjectItemElement(
string group,
string itemName)
string itemName,
string expectedRootFiles)
{
var testDirectory = Temp.CreateDirectory().Path;
WriteExtraFiles(testDirectory);
@ -464,12 +465,12 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
if (defaultIncludePatterns.Any())
{
item.Include.Should()
.Be(@"root\**\*;src\**\*;rootfile.cs;" + string.Join(";", defaultIncludePatterns).Replace("/", "\\"));
.Be($@"root\**\*;src\**\*{expectedRootFiles};" + string.Join(";", defaultIncludePatterns).Replace("/", "\\"));
}
else
{
item.Include.Should()
.Be(@"root\**\*;src\**\*;rootfile.cs");
.Be($@"root\**\*;src\**\*{expectedRootFiles}");
}
if (defaultExcludePatterns.Any())
@ -488,12 +489,13 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
}
[Theory]
[InlineData("compile", "Compile")]
[InlineData("embed", "EmbeddedResource")]
[InlineData("copyToOutput", "Content")]
[InlineData("compile", "Compile", "")]
[InlineData("embed", "EmbeddedResource", ";rootfile.cs")]
[InlineData("copyToOutput", "Content", ";rootfile.cs")]
private void MigratingGroupIncludeOnlyPopulatesAppropriateProjectItemElement(
string group,
string itemName)
string itemName,
string expectedRootFiles)
{
var testDirectory = Temp.CreateDirectory().Path;
WriteExtraFiles(testDirectory);
@ -522,12 +524,12 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
if (defaultIncludePatterns.Any())
{
item.Include.Should()
.Be(@"root\**\*;src\**\*;rootfile.cs;" + string.Join(";", defaultIncludePatterns).Replace("/", "\\"));
.Be($@"root\**\*;src\**\*{expectedRootFiles};" + string.Join(";", defaultIncludePatterns).Replace("/", "\\"));
}
else
{
item.Include.Should()
.Be(@"root\**\*;src\**\*;rootfile.cs");
.Be($@"root\**\*;src\**\*{expectedRootFiles}");
}
if (defaultExcludePatterns.Any())
@ -579,6 +581,36 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
mockProj.Items.First(i => i.ItemType == "None").Include.Should().Be("App.config");
}
[Fact]
public void MigratingCompileIncludeWithPlainFileNamesRemovesThem()
{
var mockProj = RunBuildOptionsRuleOnPj(@"
{
""buildOptions"": {
""compile"": {
""include"": [""filename1.cs"", ""filename2.cs""],
}
}
}");
mockProj.Items.Count(i => i.ItemType.Equals("Compile", StringComparison.Ordinal)).Should().Be(0);
}
[Fact]
public void MigratingCompileIncludeFilesWithPlainFileNamesRemovesThem()
{
var mockProj = RunBuildOptionsRuleOnPj(@"
{
""buildOptions"": {
""compile"": {
""includeFiles"": [""filename1.cs"", ""filename2.cs""],
}
}
}");
mockProj.Items.Count(i => i.ItemType.Equals("Compile", StringComparison.Ordinal)).Should().Be(0);
}
private static IEnumerable<string> GetDefaultExcludePatterns(string group)
{
var defaultExcludePatterns = new List<string>(group == "copyToOutput" ?

14
test/dotnet-migrate.Tests/GivenThatIWantToMigrateTestApps.cs
View file

@ -664,6 +664,20 @@ namespace Microsoft.DotNet.Migration.Tests
BuildMSBuild(projectDirectory, projectName);
}
[Fact]
public void ItMigratesAndBuildsAppWithExplicitInclude()
{
const string projectName = "TestAppWithExplicitInclude";
var projectDirectory = TestAssets.Get(projectName)
.CreateInstance()
.WithSourceFiles()
.Root;
MigrateProject(projectDirectory.FullName);
Restore(projectDirectory, projectName);
BuildMSBuild(projectDirectory, projectName);
}
[Fact]
public void ItMigratesAndBuildsAppWithExplicitIncludeGlob()
{