Skip to content
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.

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

Use Arcade bootstrapping scripts #30498

Merged
merged 4 commits into from
Oct 30, 2018
Merged

Use Arcade bootstrapping scripts #30498

merged 4 commits into from
Oct 30, 2018

Conversation

tmat
Copy link
Member

@tmat tmat commented Oct 14, 2018

Use Arcade bootstrapping scripts to install dotnet cli.

@tmat tmat requested a review from a team as a code owner October 14, 2018 02:08
@tmat tmat force-pushed the DotNet branch 2 times, most recently from 60206b5 to 462054c Compare October 16, 2018 23:54
@tmat tmat changed the title WIP: Use Arcade bootstrapping scripts Use Arcade bootstrapping scripts Oct 17, 2018
@tmat
Copy link
Member Author

tmat commented Oct 17, 2018

@jaredpar PTAL

@@ -35,4 +35,23 @@
<Import Project="OptimizationData.targets"/>
<Import Project="SymStore.targets"/>

<UsingTask TaskName="Microsoft.DotNet.Arcade.Sdk.LocateDotNet" AssemblyFile="$(RoslynToolsBuildTasksAssembly)" />

<Target Name="InitializeDotNetToolPath" Condition="'$(DotNetTool)' == ''">
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean that VS based builds could pick up the dotnet install that our scripts bootstrap?

Copy link
Member Author

Choose a reason for hiding this comment

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

This target makes the DotNetTool variable available in the build no matter how it's triggered (build script, msbuild directly or from VS). The only place I'm aware now that needs this is running tests via msbuild /t:test. We can indeed use this target whenever we need to find dotnet.

Copy link
Member

Choose a reason for hiding this comment

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

So is that a yes? Or do we still have to run the .NET SDK MSI before we can build Roslyn?

Copy link
Member Author

Choose a reason for hiding this comment

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

The target will fail if it can't find the right version of .NET SDK. It won't try to install it.

global.json Show resolved Hide resolved
@jaredpar
Copy link
Member

Done with review pass iteration #2

@tmat tmat force-pushed the DotNet branch 3 times, most recently from 6e21b9a to c39ee87 Compare October 26, 2018 23:59
@tmat tmat closed this Oct 29, 2018
@tmat tmat reopened this Oct 29, 2018
@tmat
Copy link
Member Author

tmat commented Oct 29, 2018

@jaredpar PTAL. The change now includes msbuild and dotnet SDK version checking.

MSBuildMinimumDisplayVersion="MSBuild 15.7"
/>
<UsingTask TaskName="Microsoft.DotNet.Arcade.Sdk.CompareVersions" AssemblyFile="$(RoslynToolsBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.DotNet.Arcade.Sdk.SingleError" AssemblyFile="$(RoslynToolsBuildTasksAssembly)" />
Copy link
Contributor

@nguerrera nguerrera Oct 30, 2018

Choose a reason for hiding this comment

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

Why are things in Arcade with AssemblyFile == $(RoslynToolsBuildTasksAssembly)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Temporary hack.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will move these to Arcade SDK proper.

@tmat tmat merged commit 00384a0 into dotnet:master Oct 30, 2018
@tmat tmat deleted the DotNet branch October 30, 2018 01:01
wachulski added a commit to wachulski/roslyn that referenced this pull request Oct 31, 2018
…out-if-error-message

* origin/master: (1712 commits)
  User-defined operator checks should ignore differences in tuple member names (dotnet#30774)
  Attempted fix for correctly reporting error CS1069 when using implicit namespaces (dotnet#30244)
  Invert the binding order of InitializerExpressions and CreationExpressions (dotnet#30805)
  Use Arcade bootstrapping scripts (dotnet#30498)
  Ensure that the compilers produce double.NaN values in IEEE canonical form. (dotnet#30587)
  Remove properties set in BeforeCommonTargets.targets
  Fix publishing of dependent projects
  Contributing page: reference Unix build instructions
  Delete 0
  Propagate values from AbstractProject to VisualStudioProjectCreationInfo
  Fix publishing nuget info of dev16.0.x-vs-deps branch
  Revert "Add a SetProperty API for CPS to passing msbuild properties"
  Validate generic arguments in `using static` directives (dotnet#30737)
  Correct 15.9 publish data
  Enable test.
  Do not inject attribute types into .Net modules.
  Add a SetProperty API for CPS to passing msbuild properties
  Revert "add beta2 suffix to dev16 branch"
  Fix references
  Remove commit sha from package versions
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants