Update RepoToolset to 1.0.0-beta2-62804-01 #2178

tmat · 2018-04-25T00:46:36Z

No description provided.

livarcocc · 2018-04-25T19:42:46Z

@tannergooding can you review this please?

livarcocc · 2018-04-25T19:43:04Z

@mmitche to give the clear on when this can be merged.

tannergooding · 2018-04-25T19:48:03Z

nuget.config

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="utf-8"?>


This can potentially impact tests as nuget.config are automatically imported.

You mean the test projects in src\Assets pull in nuget.config? Should we add an empty nuget.config to that directory then? Alternatively, I think that setting RestoreSources property causes restore to ignore nuget.config, so perhaps the test projects should set that property?

In reply to: 184184799 [](ancestors = 184184799)

I think we are already creating a nuget.config, but we should make sure that is the case, so that we don't start taking an accidental dependency on the root nuget.config getting auto-imported.

@dsplaisted, do you know if we are creating one for all tests, or just for the perf tests?

As of #2157 a NuGet.config is generated for all tests (it goes in the root test execution folder)

tannergooding · 2018-04-25T19:48:41Z

src/Directory.Build.props

@@ -3,11 +3,10 @@
 <Project>

  <PropertyGroup>
-    <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileDirectory)..\Directory.Build.props</MSBuildAllProjects>
+    <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>


This is incorrect, Directory.Build.props are added to MSBuildAllProjects by the file that imports them, they do not add themselves. #Resolved

I'm not sure I follow why that would be the case. Shouldn't every file be adding itself?

In reply to: 184184946 [](ancestors = 184184946)

Normally, yes. However, MSBuild does something "smart" with Directory.Build.props/targets since they are auto-imported...

 <PropertyGroup Condition="'$(ImportDirectoryBuildProps)' == 'true' and '$(DirectoryBuildPropsPath)' == ''"> <_DirectoryBuildPropsFile Condition="'$(_DirectoryBuildPropsFile)' == ''">Directory.Build.props</_DirectoryBuildPropsFile> <_DirectoryBuildPropsBasePath Condition="'$(_DirectoryBuildPropsBasePath)' == ''">$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), '$(_DirectoryBuildPropsFile)'))</_DirectoryBuildPropsBasePath> <DirectoryBuildPropsPath Condition="'$(_DirectoryBuildPropsBasePath)' != '' and '$(_DirectoryBuildPropsFile)' != ''">$([System.IO.Path]::Combine('$(_DirectoryBuildPropsBasePath)', '$(_DirectoryBuildPropsFile)'))</DirectoryBuildPropsPath> </PropertyGroup> <PropertyGroup Condition="'$(ImportDirectoryBuildProps)' == 'true' and exists('$(DirectoryBuildPropsPath)')"> <MSBuildAllProjects>$(MSBuildAllProjects);$(DirectoryBuildPropsPath)</MSBuildAllProjects> </PropertyGroup>

Because of this, Directory.Build.props/targets shouldn't add themselves, they instead add any additional Directory.Build.props/targets that they import, from further up in the chain (hence why this was $(MSBuildThisFileDirectory)..\Directory.Build.props). #Resolved

I see. Will fix. Thanks for pointing it out!

In reply to: 184193805 [](ancestors = 184193805)

tannergooding · 2018-04-25T19:50:35Z

.gitignore

@@ -9,6 +9,9 @@ obj/
 # Visual Studio
 ###############################################################################
 .vs/
+.dotnet/


Why? We had them under artifacts, explicitly, in order to help prevent pollution of the root directory... #WontFix

Because it's are not under artifacts anymore. Artifacts are meant for artifacts produced by the build, not by tools the build uses.

In reply to: 184185423 [](ancestors = 184185423)

These are artifacts produced by the build. They may not be artifacts produced directly by our own source code, but they are artifacts that exist solely to support the build and that are not checked into the source tree. Having it under a single folder makes it very convenient to manage and clean up things, as you have a single location to track, rather than 4-5.

git clean -xfd cleans everything if that's what you want to do.

Cleaning up the dotnet installation is not common. Deleting artifacts without deleting dotnet is more common. Downloading and installing dotnet takes long time and is only needed when the version changes.

In reply to: 184195237 [](ancestors = 184195237)

tannergooding · 2018-04-25T19:51:02Z

Directory.Build.props

@@ -3,31 +3,11 @@
 <Project>

  <PropertyGroup>
-    <Configuration Condition="'$(Configuration)' == ''">Debug</Configuration>
+    <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>


This is incorrect, Directory.Build.props are added to MSBuildAllProjects by the importing file, they do not add themselves. #Resolved

@tannergooding I think all .props and .targets should add themselves to MSBuildAllProjects. It doesn't hurt to have the same file added twice, and it's much simpler to always do it rather than have to reason about whether the importer is doing it for you.

tannergooding · 2018-04-25T19:51:49Z

Directory.Build.props

-
-  <PropertyGroup>
-    <ArtifactsDir>$(DOTNET_SDK_ARTIFACTS_DIR)</ArtifactsDir>
-    <ArtifactsDir Condition="'$(ArtifactsDir)' == ''">$(RepoRoot)artifacts\</ArtifactsDir>


Is this set by RepoToolset now? #Resolved

DOTNET_SDK_ARTIFACTS_DIR is no longer needed.

In reply to: 184185770 [](ancestors = 184185770)

What do you mean, it is still used by our tests for dogfooding.

That still needs to be cleaned up (I'll follow up on that later). It's used as an optional override of the artifacts directory path here:

sdk/src/Tests/Microsoft.NET.TestFramework/TestContext.cs

Lines 100 to 104 in 9cf7788

string artifactsDir = Environment.GetEnvironmentVariable("DOTNET_SDK_ARTIFACTS_DIR");

if (string.IsNullOrEmpty(artifactsDir))

{

artifactsDir = Path.Combine(repoRoot, "artifacts");

}

Based on discussion with @dsplaisted the scenario this was needed for was to switch to a different version of dotnet SDK. This can be done by setting DOTNET_INSTALL_DIR instead. So DOTNET_SDK_ARTIFACTS_DIR is not needed anymore.

In reply to: 184195832 [](ancestors = 184195832)

tannergooding · 2018-04-25T19:52:00Z

Directory.Build.props

-    <ArtifactsDir>$(DOTNET_SDK_ARTIFACTS_DIR)</ArtifactsDir>
-    <ArtifactsDir Condition="'$(ArtifactsDir)' == ''">$(RepoRoot)artifacts\</ArtifactsDir>
-    <ArtifactsDir>$([MSBuild]::EnsureTrailingSlash($(ArtifactsDir)))</ArtifactsDir>
-    <DOTNET_INSTALL_DIR Condition="'$(DOTNET_INSTALL_DIR)' == ''">$(RepoRoot)artifacts\.dotnet\$(DotNetCliVersion)\</DOTNET_INSTALL_DIR>


Same for this? #Resolved

Yes, the install dir environment variable is set by build.ps1, if not set already.

In reply to: 184185819 [](ancestors = 184185819)

Doesn't this break the scenario of building the solution from the command line, directly, rather than going through the build scripts?

tannergooding · 2018-04-25T19:52:22Z

build.sh

@@ -8,4 +8,4 @@ while [ -h "$SOURCE" ]; do # resolve $SOURCE until the file is no longer a symli
 done
 ScriptRoot="$( cd -P "$( dirname "$SOURCE" )" && pwd )"

-. "$ScriptRoot/build/build.sh" --build --restore --log "$@"
+. "$ScriptRoot/build/build.sh" --build --restore "$@"


Why was the log command removed? #Resolved

It is not a valid arg in the new version of the repo toolset and caused source-build builds to fail. #Resolved

The build now always produces binlog.

In reply to: 184189056 [](ancestors = 184189056)

tannergooding · 2018-04-25T19:52:40Z

build/NuGet.props

-
-  <PropertyGroup>
-    <!-- Respect environment variable for the NuGet Packages Root if set; otherwise, use the current default location -->
-    <NuGetPackageRoot Condition="'$(NuGetPackageRoot)' == ''">$(NUGET_PACKAGES)</NuGetPackageRoot>


Is this part of RepoToolset now? #Resolved

Yes.

In reply to: 184186017 [](ancestors = 184186017)

tannergooding · 2018-04-25T19:52:57Z

build/Versions.props

@@ -12,18 +12,6 @@
    <PreReleaseVersionLabel>rc1</PreReleaseVersionLabel>
  </PropertyGroup>

-  <!-- Repo Toolset Features -->
-  <PropertyGroup>
-    <UsingToolMicrosoftNetCompilers>false</UsingToolMicrosoftNetCompilers>


Is this part of RepoToolset now?

dsplaisted · 2018-04-25T21:32:05Z

...t.NET.Build.Extensions.Tasks.UnitTests/Microsoft.NET.Build.Extensions.Tasks.UnitTests.csproj

@@ -12,7 +12,7 @@
    <OutDirName>Tests\$(MSBuildProjectName)</OutDirName>
  </PropertyGroup>

-  <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />
+  <Import Project="Sdk.props" Sdk="RoslynTools.RepoToolset" />


Personally I really don't like that all projects have to be updated to use a different Sdk. Also, Sdks don't compose if done this way- there's no way to use both RepoToolset and Microsoft.NET.Sdk.Web for example.

Would it be possible instead to:

Not have the RepoToolset SDK import Microsoft.NET.Sdk

Have projects that use RepoToolset do so via Sdk imports in Directory.Build.props and Directory.Build.targets

Leave the Sdk imports in project files as they were (ie Microsoft.NET.Sdk or Microsoft.NET.Sdk.Web)

We don't need to change this before we update the sdk repo to the new repotoolset, but I would like to consider a change like this for the future.

This is the current design. We can discuss changes in future.

mmitche · 2018-04-26T16:25:09Z

I'm good with the changes.

* Update RepoToolset to 1.0.0-beta2-62804-01 * Update dotnet SDK to 2.1.300-preview3-008616 (dotnet#2180) * Remove --log argument from build.sh * Exclude test projects from source build (dotnet#2181) * Fix paths in MSBuildAllProjects (dotnet#2182)

Looks like it was broken by dotnet#2178

tmat and others added 3 commits April 6, 2018 13:02

Update RepoToolset to 1.0.0-beta2-62804-01

dd10bc7

Merge branch 'release/2.1.3xx' into release/2.1.3xx-src

9bdc1c5

Update dotnet SDK to 2.1.300-preview3-008616 (#2180)

b48cad2

tmat mentioned this pull request Apr 25, 2018

Update RepoToolset to 1.0.0-beta2-62804-01 #2113

Closed

tmat added 2 commits April 25, 2018 11:27

Remove --log argument from build.sh

ad52646

Exclude test projects from source build (#2181)

97595d0

livarcocc added this to the 2.1.3xx milestone Apr 25, 2018

tannergooding reviewed Apr 25, 2018

View reviewed changes

Fix paths in MSBuildAllProjects (#2182)

765c4dc

tannergooding approved these changes Apr 25, 2018

View reviewed changes

dsplaisted reviewed Apr 25, 2018

View reviewed changes

dagood mentioned this pull request Apr 25, 2018

Add sdk to 2.1 RC known-good dotnet/source-build#472

Closed

tmat mentioned this pull request Apr 25, 2018

Fix paths in MSBuildAllProjects #2182

Merged

mmitche approved these changes Apr 26, 2018

View reviewed changes

tmat merged commit 363351b into release/2.1.3xx Apr 26, 2018

johnbeisner mentioned this pull request May 24, 2018

Putting back the 'perf' property switch #2273

Merged

sbomer added a commit to sbomer/sdk that referenced this pull request Sep 19, 2018

Fix dogfood in build.sh

9d2fdbc

Looks like it was broken by dotnet#2178

sbomer mentioned this pull request Sep 19, 2018

Fix dogfood in build.sh #2542

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update RepoToolset to 1.0.0-beta2-62804-01 #2178

Update RepoToolset to 1.0.0-beta2-62804-01 #2178

tmat commented Apr 25, 2018

livarcocc commented Apr 25, 2018

livarcocc commented Apr 25, 2018

tannergooding Apr 25, 2018

tmat Apr 25, 2018

tannergooding Apr 25, 2018

dsplaisted Apr 25, 2018

tannergooding Apr 25, 2018 •

edited by tmat

Loading

tmat Apr 25, 2018

tannergooding Apr 25, 2018 •

edited by tmat

Loading

tmat Apr 25, 2018

tannergooding Apr 25, 2018 •

edited by tmat

Loading

tmat Apr 25, 2018

tannergooding Apr 25, 2018

tmat Apr 25, 2018

tannergooding Apr 25, 2018 •

edited by tmat

Loading

dsplaisted Apr 25, 2018

tannergooding Apr 25, 2018 •

edited by tmat

Loading

tmat Apr 25, 2018

tannergooding Apr 25, 2018

tmat Apr 25, 2018 •

edited

Loading

tannergooding Apr 25, 2018 •

edited by tmat

Loading

tmat Apr 25, 2018

tannergooding Apr 25, 2018

tannergooding Apr 25, 2018 •

edited by tmat

Loading

dagood Apr 25, 2018 •

edited by tmat

Loading

tmat Apr 25, 2018

tannergooding Apr 25, 2018 •

edited by tmat

Loading

tmat Apr 25, 2018

tannergooding Apr 25, 2018

dsplaisted Apr 25, 2018

tmat Apr 25, 2018

mmitche commented Apr 26, 2018

	string artifactsDir = Environment.GetEnvironmentVariable("DOTNET_SDK_ARTIFACTS_DIR");
	if (string.IsNullOrEmpty(artifactsDir))
	{
	artifactsDir = Path.Combine(repoRoot, "artifacts");
	}

Update RepoToolset to 1.0.0-beta2-62804-01 #2178

Update RepoToolset to 1.0.0-beta2-62804-01 #2178

Conversation

tmat commented Apr 25, 2018

livarcocc commented Apr 25, 2018

livarcocc commented Apr 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding Apr 25, 2018 • edited by tmat Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding Apr 25, 2018 • edited by tmat Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding Apr 25, 2018 • edited by tmat Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding Apr 25, 2018 • edited by tmat Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding Apr 25, 2018 • edited by tmat Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmat Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

tannergooding Apr 25, 2018 • edited by tmat Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding Apr 25, 2018 • edited by tmat Loading

Choose a reason for hiding this comment

dagood Apr 25, 2018 • edited by tmat Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding Apr 25, 2018 • edited by tmat Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmitche commented Apr 26, 2018

tannergooding Apr 25, 2018 •

edited by tmat

Loading

tannergooding Apr 25, 2018 •

edited by tmat

Loading

tannergooding Apr 25, 2018 •

edited by tmat

Loading

tannergooding Apr 25, 2018 •

edited by tmat

Loading

tannergooding Apr 25, 2018 •

edited by tmat

Loading

tmat Apr 25, 2018 •

edited

Loading

tannergooding Apr 25, 2018 •

edited by tmat

Loading

tannergooding Apr 25, 2018 •

edited by tmat

Loading

dagood Apr 25, 2018 •

edited by tmat

Loading

tannergooding Apr 25, 2018 •

edited by tmat

Loading