Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Move corefx onto the .NET Core 2.0 toolset #18035

Merged
merged 11 commits into from
Apr 10, 2017
Merged

Conversation

mellinoe
Copy link
Contributor

@mellinoe mellinoe commented Apr 6, 2017

The latest version of buildtools allows a 2.0 CLI to be used. This change pulls in that version of buildtools and updates corefx to use the 2.0 CLI for restoring nuget packages and running our shared tools.

  • All of the project.json(.template) files under external have been converted to use PackageReference items directly defined in the depproj files. The versions for these references are based on pre-existing properties defined in dependencies.props. When we take future version update PR's, we should only see changes in that single file.
  • packageresolve.targets was modified so that the paths for the MSBuild project to be restored, as well as the "project.assets.json" file (the lock file, essentially). Note that the property for the projec to be restored is still called ProjectLockJson, although it represents an MSBuild project. I'll look into cleaning this up, I just need to make sure there's no stray references to "ProjectLockJson" remaining.
  • There are a bunch of new calls to chmod here and there. This is because .NET Core 2.0 (which we now restore all packages with, and run the build on) does not preserve executable flags in Unix files, by design. We need to mark our programs as executable before we try to use them.
  • NuGet now restores all packages into folders with lower-case names. This needed to be accounted for in a variety of different places.
  • I've brought back the "hack" where we pre-build CoreFx.Tools.dll before running the full product build. This is necessary due to changes in the CLI. Technically this will go away when we move CoreFx.Tools.dll into buildtools. Never mind. We don't need that.

NOTE: This is not going to work on Alpine right now, because there is no 2.0 CLI being produced for it. Everything should be in place to do, so, though, so hopefully we can turn that on shortly.

@ericstj , @weshaggard, @joperezr

build.sh Outdated
fi

# Building CoreFx.Tools before calling build-managed.sh to ensure our tools are available before running the main build.
"$__scriptpath/Tools/msbuild.sh" "$__scriptpath/src/Tools/CoreFx.Tools/CoreFx.Tools.csproj" /v:m /m
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, Did we end up figuring out what changed on 2.0 in the last month that regressed this?
Also, don't we need to do this as well on build.cmd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this was a misunderstanding on my part. There was a configuration option in MSBuild.runtimeconfig.json (which we override) that gave us the old behavior back. I accidentally deleted that file. After un-deleting it, we don't need this hacky stuff.

@@ -171,11 +172,5 @@
<PackageId>microsoft.xunit.runner.uwp</PackageId>
<Version>$(AppXRunnerVersion)</Version>
</DependencyBuildInfo>

<!-- project.json files to update -->
<ProjectJsonFiles Include="$(MSBuildThisFileDirectory)external\**\project.json" />
Copy link
Member

Choose a reason for hiding this comment

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

I assume that msbuild /t:UpdateDependencies will still work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it does. It will only update dependencies.props now, but the properties defined in there are used in the depproj files.

@@ -104,7 +104,7 @@
<DnuRestoreCommand>$(DnuRestoreCommand) restore</DnuRestoreCommand>
<DnuRestoreCommand Condition="'$(ParallelRestore)'=='true'">$(DnuRestoreCommand) --parallel</DnuRestoreCommand>
<DnuRestoreCommand Condition="'$(UseNuGetHttpCache)'!='true'">$(DnuRestoreCommand) --no-cache</DnuRestoreCommand>
<DnuRestoreCommand>$(DnuRestoreCommand) --packages "$(PackagesDir.TrimEnd('/\'.ToCharArray()))" $(DnuRestoreSource)</DnuRestoreCommand>
<DnuRestoreCommand>$(DnuRestoreCommand) --packages "$(PackagesDir.TrimEnd('/').TrimEnd('\'))" $(DnuRestoreSource)</DnuRestoreCommand>
Copy link
Member

Choose a reason for hiding this comment

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

mm not sure I understood this change.

Copy link
Contributor Author

@mellinoe mellinoe Apr 6, 2017

Choose a reason for hiding this comment

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

For some reason, MSBuild did not want to call the TrimEnd overload we were using before. We did add a parameterless overload to this function in 2.0. I couldn't figure out the syntax to get it right, so I just made two calls to TrimEnd instead.

Copy link
Member

Choose a reason for hiding this comment

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

There was some breaking changes in MSBuild with netcoreapp2.0 caused by the new overload. See dotnet/msbuild#1634.

/cc @rainersigwald @AndyGerlicher - in case they want to know about this change for netcoreapp2.0. It feels like a bug.


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

Copy link
Member

Choose a reason for hiding this comment

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

Filed dotnet/msbuild#1966 to take a look at this

@@ -8,5 +8,8 @@
<RuntimeOS Condition="'$(PortableBuild)' == 'true' and '$(OSGroup)' == 'Linux'">linux</RuntimeOS>
-->
<NugetRuntimeIdentifier>$(RuntimeOS)-$(ArchGroup)</NugetRuntimeIdentifier>
<ContainsPackageReferences>true</ContainsPackageReferences>
<!-- We need configuration-specific assets files. -->
<RestoreOutputPath>$(BaseIntermediateOutputPath)external/$(MSBuildProjectName)/$(TargetGroup)</RestoreOutputPath>
Copy link
Member

Choose a reason for hiding this comment

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

I believe instead of TargetGroup, you could use $(Configuration) instead

Copy link
Member

Choose a reason for hiding this comment

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

Better yet just hook it off IntermedateOutputPath (https://github.com/dotnet/corefx/blob/master/dir.props#L237)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Better yet just hook it off IntermedateOutputPath

Yeah that seems simple enough. Will change.

</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.NETCore.Platforms">
<Version>2.0.0-$(CoreFxExpectedPrerelease)</Version>
Copy link
Member

@joperezr joperezr Apr 6, 2017

Choose a reason for hiding this comment

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

I have a feeling that this will eventually break us in the future. Instead we might consider using the actual nuget package version in a property, instead of just the prerelease. This is doable through dependencies.props. cc: @dagood

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, that would be better. I'm not very familiar with the format of dependencies.props, but if we can get that to work then I'm all for it.

Copy link
Member

Choose a reason for hiding this comment

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

I imagine we will do a clean-up pass of a lot of these versions once we get fully moved to the msbuild based CLI.

@@ -6,6 +6,22 @@
<NugetRuntimeIdentifier>$(RuntimeOS)-x64</NugetRuntimeIdentifier>
Copy link
Member

Choose a reason for hiding this comment

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

RuntimeOS [](start = 30, length = 9)

Where are you specifying the TFM of these guys now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's still getting pulled in through the build configuration, unless I'm misunderstanding the question.

Copy link
Member

Choose a reason for hiding this comment

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

Yes it should be.

echo "{\"frameworks\":{\"netcoreapp1.1\":{\"dependencies\":{\"Microsoft.NETCore.Runtime.CoreCLR\":\"$__CoreClrVersion\", \"Microsoft.NETCore.Platforms\": \"$__CoreClrVersion\"}}},\"runtimes\":{\"$__rid\":{}}}" > "$__pjDir/project.json"
$__dotnet restore $__pjDir/project.json --packages $__packagesDir
__crossgenInPackage=$__packagesDir/runtime.$__packageRid.Microsoft.NETCore.Runtime.CoreCLR/$__CoreClrVersion/tools/crossgen
echo "<Project Sdk=\"Microsoft.NET.Sdk\"><PropertyGroup><TargetFramework>netcoreapp2.0</TargetFramework><DisableImplicitFrameworkReferences>true</DisableImplicitFrameworkReferences><RuntimeIdentifiers>$__packageRid</RuntimeIdentifiers></PropertyGroup><ItemGroup><PackageReference Include=\"Microsoft.NETCore.Runtime.CoreCLR\" Version=\"$__CoreClrVersion\" /><PackageReference Include=\"Microsoft.NETCore.Platforms\" Version=\"1.1.0\" /></ItemGroup></Project>" > "$__pjDir/crossgen.csproj"
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this any more? Has crossgen been added to the package yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't when I was using an "older" version of the 2.0 CLI. Let me check again now that this works on a newer version.

@@ -2,7 +2,9 @@

# Restores crossgen and runs it on all tools components.

__CoreClrVersion=1.1.0
__CoreClrVersion=2.0.0-preview1-25204-02
Copy link
Member

Choose a reason for hiding this comment

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

The duplication of all these version is going to be a maintenance nightmare we need to think about ways of eliminating them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically I can pluck this version out of the string resources of libcoreclr.so. If crossgen is included with the shared framework though (which I think is the plan?) then it won't be necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

If crossgen is included with the shared framework though (which I think is the plan?).

It should now be in the the tools folder of the Microsoft.NETCore.App package (so you don't need to restore the Microsoft.NETCore.Runtime.CoreCLR package) but the current plan is to not include it as an exe in the shared framework on disk.

/cc @gkhanna79

<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
<!-- When zipped/unzipped in a package, ilasm loses its executable flag. Reset it. -->
<Target Name="MakeIlasmExecutable" AfterTargets="CopyFilesToOutputDirectory" Condition="'$(RunningOnUnix)' == 'true'">
Copy link
Member

Choose a reason for hiding this comment

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

Why wasn't this an issue before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See this bullet point:

  • There are a bunch of new calls to chmod here and there. This is because .NET Core 2.0 (which we now restore all packages with, and run the build on) does not preserve executable flags in Unix files, by design. We need to mark our programs as executable before we try to use them.

<ProjectJson Condition="Exists('$(ProjectJsonTemplate)')">$(IntermediateOutputPath)project.json</ProjectJson>
<ProjectLockJson Condition="Exists('$(ProjectJsonTemplate)')">$(IntermediateOutputPath)project.lock.json</ProjectLockJson>
</PropertyGroup>

<Import Project="..\dir.targets" />

<PropertyGroup>
<RestorePackages>true</RestorePackages>
Copy link
Member

Choose a reason for hiding this comment

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

Are these properties still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe so... RestorePackages defaults to false and is only set in this folder.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it won't be needed once we stop doing the restore ourselves and start using the nuget targets for restore.

</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.CSharp">
Copy link
Member

Choose a reason for hiding this comment

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

I assume you are keeping the Version as nested metadata because of the dependency on a new msbuild we don't want to take just yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's right. It would prevent us from using VS 2015 on Windows, and the only benefit we'd get would be that syntax sugar.

@@ -13,10 +13,16 @@
<PropertyGroup Condition="!$(NuGetTargetMoniker.StartsWith('.NETFramework'))">
<!-- For things not .NETFramework we need the net461 targeting pack to generate facades -->
<NuGetTargetMoniker>.NETFramework,Version=v4.6.1</NuGetTargetMoniker>
<NuGetTargetMonikerShort>net461</NuGetTargetMonikerShort>
Copy link
Member

Choose a reason for hiding this comment

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

Were does NugetTargetMonikerShort get consumed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It gets consumed here in packageresolve.targets. Unfortunately, passing the "full moniker" to /p:TargetFramework does not work with the 2.0 CLI / SDK.

@@ -7,18 +7,45 @@
<NugetRuntimeIdentifier Condition="'$(OSGroup)' == 'Unix' AND '$(RunningOnUnix)' != 'true'">ubuntu.14.04-x64</NugetRuntimeIdentifier>
<NugetRuntimeIdentifier Condition="'$(TargetGroup)' == 'uap'">win10-$(ArchGroup)</NugetRuntimeIdentifier>
<NugetRuntimeIdentifier Condition="'$(TargetGroup)' == 'uapaot'">win10-$(ArchGroup)-aot</NugetRuntimeIdentifier>
<AppendMSBuildRestoreArguments>true</AppendMSBuildRestoreArguments>
Copy link
Member

Choose a reason for hiding this comment

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

What is AppendMSBuildRestoreArguments and why do we need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I think this is a leftover change from some intermediate state. Will remove.

</PackageReference>
</ItemGroup>


Copy link
Member

Choose a reason for hiding this comment

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

nit: extra line

set PROJECT_JSON_FILE=%PROJECT_JSON_PATH%\project.json
set PROJECT_JSON_CONTENTS={ "dependencies": { "Microsoft.DotNet.BuildTools": "%BUILDTOOLS_VERSION%" }, "frameworks": { "netcoreapp1.0": { } } }
set PROJECT_JSON_FILE=%PROJECT_JSON_PATH%\project.csproj
set PROJECT_JSON_CONTENTS=^^^<Project Sdk=^^^"Microsoft.NET.Sdk^^^"^^^>^^^<PropertyGroup^^^>^^^<TargetFramework^^^>netcoreapp1.0^^^</TargetFramework^^^>^^^<DisableImplicitFrameworkReferences^^^>true^^^</DisableImplicitFrameworkReferences^^^>^^^</PropertyGroup^^^>^^^<ItemGroup^^^>^^^<PackageReference Include=^^^"Microsoft.DotNet.BuildTools^^^" Version=^^^"%BUILDTOOLS_VERSION%^^^" /^^^>^^^</ItemGroup^^^>^^^</Project^^^>
Copy link
Member

Choose a reason for hiding this comment

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

Similar comment (with similar response I'm sure :)) - This should be a file now that we can pass parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do that. Any opinions on what that project file should be called / where it should be?

Copy link
Member

Choose a reason for hiding this comment

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

I think I would just call it something like init-tools.proj (or perhaps init-tools.msbuild to avoid clashing with other projs) and put it right next to the init-tools scripts.

@@ -69,14 +69,6 @@ if not [%INIT_TOOLS_ERRORLEVEL%]==[0] (
exit /b %INIT_TOOLS_ERRORLEVEL%
)

echo Updating CLI NuGet Frameworks map...
Copy link
Member

Choose a reason for hiding this comment

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

Yay!

init-tools.sh Outdated
echo "Making all .sh files executable under Tools."
ls $__scriptpath/Tools/*.sh | xargs chmod +x

cp Tools-Override/* Tools
Copy link
Member

Choose a reason for hiding this comment

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

This is currently being done in run.sh https://github.com/dotnet/corefx/blob/master/run.sh#L10.

@@ -5,7 +5,6 @@
<Project Include="$(MSBuildThisFileDirectory)$(MSBuildProjectName).csproj" />
<Project Include="$(MSBuildThisFileDirectory)$(MSBuildProjectName).csproj">
Copy link
Member

Choose a reason for hiding this comment

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

Given the way we build this https://github.com/dotnet/corefx/blob/master/build.proj#L41, this entire file can probably be deleted.

@@ -4,181 +4,212 @@
<TargetGroups Include="netcore50">
<TargetingPackNugetPackageId>Microsoft.TargetingPack.Private.NetNative</TargetingPackNugetPackageId>
<NuGetTargetMoniker>.NETCore,Version=v5.0</NuGetTargetMoniker>
<NuGetTargetMonikerShort>netcore50</NuGetTargetMonikerShort>
Copy link
Member

Choose a reason for hiding this comment

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

Given you have these here why do you have them in all the project files as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only put them in the project files where the TFM was manually overridden.

@@ -5,7 +5,7 @@
<!-- Temporarily enable local build of tools -->
<CoreFxToolsDir Condition="'$(CoreFxToolsDir)' == ''">$(ToolsDir)</CoreFxToolsDir>
<CoreFxDesktopToolsDir Condition="'$(CoreFxDesktopToolsDir)' == ''">$(ToolsDir)net46/</CoreFxDesktopToolsDir>
<CoreFxToolsTaskDir Condition="'$(CoreFxToolsTaskDir)' == '' AND '$(RunningOnCore)' == 'true'">$(CoreFxToolsDir)</CoreFxToolsTaskDir>
Copy link
Member

Choose a reason for hiding this comment

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

What happen to our RunningOnCore property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure why I changed this. I'll look into it.

src/src.builds Outdated
@@ -12,7 +12,7 @@

<PropertyGroup>
<!-- TODO: We should see about generating this from scratch instead of relying on a previous deps file as a template -->
<_ToolsDotNetCliSharedFxPath>$(ToolsDir)dotnetcli\shared\Microsoft.NETCore.App\1.1.0</_ToolsDotNetCliSharedFxPath>
<_ToolsDotNetCliSharedFxPath>$(ToolsDir)dotnetcli\shared\Microsoft.NETCore.App\1.1.1</_ToolsDotNetCliSharedFxPath>
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't this 2.0.0 now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I believe it should be. This is another annoying place where versions are hardcoded...

@mellinoe
Copy link
Contributor Author

mellinoe commented Apr 7, 2017

I fixed the way some parameters were getting passed around. Hopefully this will fix those issues.

@mellinoe
Copy link
Contributor Author

mellinoe commented Apr 7, 2017

Ok, I'm hopeful that this next build will be successful. I pushed another update that fixed a problem caused by the new "mono" TargetGroup not having a NuGetTargetMonikerShort value.

@mellinoe
Copy link
Contributor Author

@karajas Can you look at the latest commit? I added some logic to GenerateDepsFile which filters out items from the "native" section of the file when they aren't contained in our test folder. I was hitting errors because on Windows, the deps file listed "DiaSymReader.dll" when we don't have that in our test folder. There were a handful of such files that are filtered out and removed with my new logic.

@@ -0,0 +1,9 @@
{
"runtimeOptions": {
"tfm": "netcoreapp1.1",
Copy link
Member

Choose a reason for hiding this comment

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

What is the tfm option? I'm not sure I've seen that before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. It is in the runtimeconfig.json file included in the ILLink nuget package, and this file is copy-pasted from there. I just updated the Microsoft.NETCore.App version below.

Copy link
Member

Choose a reason for hiding this comment

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

The new tooling puts this "tfm" in the runtimeconfig. This value is used by the host to do "shared runtime store" probing. See https://github.com/dotnet/core-setup/blob/master/Documentation/design-docs/DotNetCore-SharedPackageStore.md for more info.


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

* This is necessary because in 2.0, the packages have been collapsed,
  so everything in the deps file appears under this one package.
After moving to the deps file from the 2.0 shared framework,
there were some issues in running tests, because some files
listed in the deps file are not present in our test shared
framework. I've added some logic to the GenerateDepsJson file
which removes these invalid items.
@@ -10,8 +10,8 @@ if [%BUILDTOOLS_SOURCE%]==[] set BUILDTOOLS_SOURCE=https://dotnet.myget.org/F/do
set /P BUILDTOOLS_VERSION=< "%~dp0BuildToolsVersion.txt"
set BUILD_TOOLS_PATH=%PACKAGES_DIR%Microsoft.DotNet.BuildTools\%BUILDTOOLS_VERSION%\lib\
set PROJECT_JSON_PATH=%TOOLRUNTIME_DIR%\%BUILDTOOLS_VERSION%
set PROJECT_JSON_FILE=%PROJECT_JSON_PATH%\project.json
Copy link
Member

Choose a reason for hiding this comment

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

I'd say it is worth renaming these "PROJECT_JSON" variables.

Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

LGTM once it is green.

@mellinoe
Copy link
Contributor Author

I'm going to try to merge this in as-is and follow up with some more cleanup, so that I can stop dealing with merge conflicts with this (the auto-updater keeps giving me merge conflicts over and over).

Will wait for green.

@weshaggard
Copy link
Member

@karajas Can you look at the latest commit? I added some logic to GenerateDepsFile which filters out items from the "native" section of the file when they aren't contained in our test folder. I was hitting errors because on Windows, the deps file listed "DiaSymReader.dll" when we don't have that in our test folder. There were a handful of such files that are filtered out and removed with my new logic.

The easiest solution here is to add a reference to diasymreader package when on windows. That will also help with some of or test run diagnostics.

@mellinoe
Copy link
Contributor Author

I've removed the Alpine build definitions from pipeline.json in the latest commit. The builds are already neglected and non-functional; they would take quite a bit more work to bring them up with this.

@mellinoe mellinoe merged commit fdf96a7 into dotnet:master Apr 10, 2017
@@ -144,24 +145,20 @@ if [ ! -e $__INIT_TOOLS_DONE_MARKER ]; then

echo "Initializing BuildTools..."
echo "Running: $__BUILD_TOOLS_PATH/init-tools.sh $__scriptpath $__DOTNET_CMD $__TOOLRUNTIME_DIR" >> $__init_tools_log

chmod +x $__BUILD_TOOLS_PATH/init-tools.sh
Copy link
Member

Choose a reason for hiding this comment

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

I think these "chmod" hacks should have links to the underlying issue: NuGet/Home#4424

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll put that on my list of follow-up fixes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants