Unblock having non-anycpu configurations by always setting output path #351
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| using System; | ||
|
|
||
| namespace ConsoleApplication | ||
| { | ||
| public class Program | ||
| { | ||
| public static void Main(string[] args) | ||
| { | ||
| Console.WriteLine("Hello World!"); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| <Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
| <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" /> | ||
|
|
||
| <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Release|x64'" /> | ||
| <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Debug|x64'" /> | ||
|
|
||
| <PropertyGroup> | ||
| <OutputType>Exe</OutputType> | ||
| <TargetFramework>netcoreapp1.0</TargetFramework> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Compile Include="**\*.cs" /> | ||
| <EmbeddedResource Include="**\*.resx" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.NETCore.App"> | ||
| <Version>1.0.1</Version> | ||
| </PackageReference> | ||
| <PackageReference Include="Microsoft.NET.Sdk"> | ||
| <Version>0.0.0</Version> | ||
| </PackageReference> | ||
| </ItemGroup> | ||
|
|
||
| <Target Name="CheckPlatform" BeforeTargets="Build"> | ||
| <Error Condition="'$(Platform)' != 'x64'" Text="This test project expects to be built via solution and have Platform=x64" /> | ||
| </Target> | ||
|
|
||
| <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" /> | ||
| </Project> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| Microsoft Visual Studio Solution File, Format Version 12.00 | ||
| # Visual Studio 15 | ||
| VisualStudioVersion = 15.0.25827.1 | ||
| MinimumVisualStudioVersion = 10.0.40219.1 | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "x64SolutionBuild", "x64SolutionBuild.csproj", "{544B615E-491F-4C80-9918-B10A6FB4B3CD}" | ||
| EndProject | ||
| Global | ||
| GlobalSection(SolutionConfigurationPlatforms) = preSolution | ||
| Debug|x64 = Debug|x64 | ||
| Release|x64 = Release|x64 | ||
| EndGlobalSection | ||
| GlobalSection(ProjectConfigurationPlatforms) = postSolution | ||
| {544B615E-491F-4C80-9918-B10A6FB4B3CD}.Debug|x64.ActiveCfg = Debug|x64 | ||
| {544B615E-491F-4C80-9918-B10A6FB4B3CD}.Debug|x64.Build.0 = Debug|x64 | ||
| {544B615E-491F-4C80-9918-B10A6FB4B3CD}.Release|x64.ActiveCfg = Release|Any CPU | ||
| {544B615E-491F-4C80-9918-B10A6FB4B3CD}.Release|x64.Build.0 = Release|Any CPU | ||
| EndGlobalSection | ||
| GlobalSection(SolutionProperties) = preSolution | ||
| HideSolutionNode = FALSE | ||
| EndGlobalSection | ||
| EndGlobal |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| <!-- | ||
| *********************************************************************************************** | ||
| Microsoft.NET.DefaultOutputPaths.targets | ||
| WARNING: DO NOT MODIFY this file unless you are knowledgeable about MSBuild and have | ||
| created a backup copy. Incorrect changes to this file will make it | ||
| impossible to load or build your projects from the command-line or the IDE. | ||
| Copyright (c) .NET Foundation. All rights reserved. | ||
| *********************************************************************************************** | ||
| --> | ||
| <Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
|
|
||
| <!-- | ||
| Apply the same default output paths as Microsoft.Common.targets now since we're running before them, | ||
| but need to adjust them and/or make decisions in terms of them. | ||
| Also note that common targets only set a default OutputPath if neither configuration nor | ||
| platform were set by the user. This was used to validate that a valid configuration is passed, | ||
| assuming the convention maintained by VS that every Configuration|Platform combination had | ||
| an explicit OutputPath. Since we now want to support leaner project files with less | ||
| duplication and more automatic defaults, we always set a default OutputPath and can no | ||
| longer depend on that convention for validation. Getting validation re-enabled with a | ||
| different mechanism is tracked by https://github.com/dotnet/sdk/issues/350 | ||
| --> | ||
| <PropertyGroup> | ||
| <Configuration Condition="'$(Configuration)'==''">Debug</Configuration> | ||
| <Platform Condition="'$(Platform)'==''">AnyCPU</Platform> | ||
| <PlatformName Condition="'$(PlatformName)' == ''">$(Platform)</PlatformName> | ||
|
|
||
| <BaseOutputPath Condition="'$(BaseOutputPath)' == ''">bin\</BaseOutputPath> | ||
| <BaseOutputPath Condition="!HasTrailingSlash('$(BaseOutputPath)')">$(BaseOutputPath)\</BaseOutputPath> | ||
| <OutputPath Condition="'$(OutputPath)' == '' and '$(PlatformName)' == 'AnyCPU'">$(BaseOutputPath)$(Configuration)\</OutputPath> | ||
| <OutputPath Condition="'$(OutputPath)' == '' and '$(PlatformName)' != 'AnyCPU'">$(BaseOutputPath)$(PlatformName)\$(Configuration)\</OutputPath> | ||
| <OutputPath Condition="!HasTrailingSlash('$(OutputPath)')">$(OutputPath)\</OutputPath> | ||
|
|
||
| <BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)' == ''">obj\</BaseIntermediateOutputPath> | ||
| <BaseIntermediateOutputPath Condition="!HasTrailingSlash('$(BaseIntermediateOutputPath)')">$(BaseIntermediateOutputPath)\</BaseIntermediateOutputPath> | ||
| <IntermediateOutputPath Condition=" $(IntermediateOutputPath) == '' and '$(PlatformName)' == 'AnyCPU' ">$(BaseIntermediateOutputPath)$(Configuration)\</IntermediateOutputPath> | ||
| <IntermediateOutputPath Condition=" $(IntermediateOutputPath) == '' and '$(PlatformName)' != 'AnyCPU' ">$(BaseIntermediateOutputPath)$(PlatformName)\$(Configuration)\</IntermediateOutputPath> | ||
| <IntermediateOutputPath Condition="!HasTrailingSlash('$(IntermediateOutputPath)')">$(IntermediateOutputPath)\</IntermediateOutputPath> | ||
| </PropertyGroup> | ||
|
|
||
| <!-- Set the package output path (for nuget pack target) now, before the TargetFramework is appended --> | ||
|
There was a problem hiding this comment. I don't think there's a way to solve this right now, but it seems to me that you wouldn't want the NuGet package generated in an x86 or x64 folder. There was a problem hiding this comment. I filed #329 to make sure we have settled on default output paths. There is definitely tension between traditional VS concept of |
||
| <PropertyGroup> | ||
| <PackageOutputPath Condition="'$(PackageOutputPath)' == ''">$(OutputPath)</PackageOutputPath> | ||
| </PropertyGroup> | ||
|
|
||
| </Project> | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| <!-- | ||
| *********************************************************************************************** | ||
| Microsoft.NET.Sdk.BeforeCommonCrossTargeting.targets | ||
|
|
||
| WARNING: DO NOT MODIFY this file unless you are knowledgeable about MSBuild and have | ||
| created a backup copy. Incorrect changes to this file will make it | ||
| impossible to load or build your projects from the command-line or the IDE. | ||
|
|
||
| Copyright (c) .NET Foundation. All rights reserved. | ||
| *********************************************************************************************** | ||
| --> | ||
| <Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
|
|
||
| <Import Project="$(MSBuildThisFileDirectory)Microsoft.NET.DefaultOutputPaths.targets" /> | ||
|
|
||
| </Project> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| // Copyright (c) .NET Foundation and contributors. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| using System.IO; | ||
| using System.Runtime.InteropServices; | ||
| using Microsoft.NET.TestFramework; | ||
| using Microsoft.NET.TestFramework.Assertions; | ||
| using Microsoft.NET.TestFramework.Commands; | ||
| using Xunit; | ||
| using static Microsoft.NET.TestFramework.Commands.MSBuildTest; | ||
|
|
||
| namespace Microsoft.NET.Build.Tests | ||
| { | ||
| public class GivenThatWeWantToBuildASolutionWithNonAnyCPUPlatform | ||
| { | ||
| private TestAssetsManager _testAssetsManager = TestAssetsManager.TestProjectsAssetsManager; | ||
|
|
||
| [Fact] | ||
| public void It_builds_solusuccessfully() | ||
| { | ||
| var testAsset = _testAssetsManager | ||
| .CopyTestAsset("x64SolutionBuild") | ||
| .WithSource() | ||
| .Restore(); | ||
|
|
||
|
|
||
| var buildCommand = new BuildCommand(Stage0MSBuild, testAsset.TestRoot, "x64SolutionBuild.sln"); | ||
| buildCommand | ||
| .Execute() | ||
| .Should() | ||
| .Pass(); | ||
|
|
||
| buildCommand.GetOutputDirectory("netcoreapp1.0", Path.Combine("x64", "Debug")) | ||
|
There was a problem hiding this comment. We should probably rename this parameter or otherwise refactor so that we're not passing in There was a problem hiding this comment. I'll follow up with a separate test-only change to clean this up. |
||
| .Should() | ||
| .OnlyHaveFiles(new[] { | ||
| "x64SolutionBuild.runtimeconfig.dev.json", | ||
| "x64SolutionBuild.runtimeconfig.json", | ||
| "x64SolutionBuild.deps.json", | ||
| "x64SolutionBuild.dll", | ||
| "x64SolutionBuild.pdb" | ||
| }); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| // Copyright (c) .NET Foundation and contributors. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| using System.IO; | ||
| using System.Runtime.InteropServices; | ||
| using Microsoft.DotNet.Cli.Utils; | ||
| using Microsoft.DotNet.InternalAbstractions; | ||
| using Microsoft.NET.TestFramework; | ||
| using Microsoft.NET.TestFramework.Assertions; | ||
| using Microsoft.NET.TestFramework.Commands; | ||
| using Xunit; | ||
| using static Microsoft.NET.TestFramework.Commands.MSBuildTest; | ||
|
|
||
| namespace Microsoft.NET.Pack.Tests | ||
| { | ||
| public class GivenThatWeWantToPackASimpleLibrary | ||
|
There was a problem hiding this comment. Is this test related to the OutputPath changes or is it just filling in more general test coverage? There was a problem hiding this comment. Yes, it is related to the package output path being correct. There are different import paths for x-targeted and single-targeted cases and we only tested x-targeted. My first attempt at fixing this broke the x-targeted test and then I found it prudent to test both cases to prevent fixing one while breaking the other. |
||
| { | ||
| private TestAssetsManager _testAssetsManager = TestAssetsManager.TestProjectsAssetsManager; | ||
|
|
||
| [Fact] | ||
| public void It_packs_successfully() | ||
| { | ||
| var testAsset = _testAssetsManager | ||
| .CopyTestAsset("HelloWorld") | ||
| .WithSource() | ||
| .Restore(); | ||
|
|
||
| new PackCommand(Stage0MSBuild, testAsset.TestRoot) | ||
| .Execute() | ||
| .Should() | ||
| .Pass(); | ||
|
|
||
| var outputDirectory = new DirectoryInfo(Path.Combine(testAsset.TestRoot, "bin", "Debug")); | ||
| outputDirectory.Should().OnlyHaveFiles(new[] { | ||
| "HelloWorld.1.0.0.nupkg", | ||
| "netcoreapp1.0/HelloWorld.dll", | ||
| "netcoreapp1.0/HelloWorld.pdb", | ||
| "netcoreapp1.0/HelloWorld.deps.json", | ||
| "netcoreapp1.0/HelloWorld.runtimeconfig.json", | ||
| "netcoreapp1.0/HelloWorld.runtimeconfig.dev.json", | ||
| }); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also have the Configuration and Platform defaults in
Microsoft.NET.Sdk.props, is there a reason to also put it in this targets file?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just being defensive in case the props aren't imported, though you probably have other problems. Regardless, the duplication pre-exists this change, so I'll leave de-duping as a separate exercise.