Port GenerateDepsFile and GenerateRuntimeConfigurationFiles

[eerhardt]

This change adds the tasks and targets from the dotnet/cli repo that will generate the .deps.json and runtimeconfig.json files.

I ported the tests as well. The tests build, but there is currently no way to execute the tests.

@davkean @natidea @333fred @brthor @livarcocc @piotrpMSFT

[333fred]

File a tracking bug for this.

[333fred]

We don't want to include project.lock.json?

[333fred]

Rather than comment on every TODO, just make sure we file tracking bugs for them.

[TheRealPiotrP]

@333fred are you asking if we want to check-in the lock file?

[333fred]

Yeah. You included src/Tasks/Microsoft.DotNet.Core.Build.Tasks.UnitTests/simple.dependencies.project.lock.json, do we actually want it included? That's a giant file.

[TheRealPiotrP]

I think the intent of that file is to be an Unit Test input in which case we do want to check it in so that updates/changes in NuGet do not cause false alarms in our test bed.

In general, however, we don't want to check in the lock files as they are RID-specific and can be computed as a function of the project.json and the package sources. We've also seen checked-in lock files cause issues in some contexts e.g. TFS wherein they cannot be updated until checked out.

We do avoid * versioning in the PJ to get the same benefit as of checking in the lock file.

[natidea]

_ProjectLockFilePath [](start = 5, length = 20)

In the package dependency task, we also address the case where the project file is $(MSBuildProjectName).project.json, setting the lock file in those cases to $(MSBuildProjectName).project.lock.json. Is this possible in .NET Core, or is this just legacy we should get rid of?

[eerhardt]

Eventually, I think it should be the NuGet team that defines this property. And all the places that need to read the lock file use the property defined by the NuGet .props file.

/cc @emgarten to get his thoughts.

[eerhardt]

In the current project.json-based project system, the lock file is always named project.lock.json. See https://github.com/dotnet/cli/blob/04f40f906dce2678d80fb9787e68de76ee6bf57e/src/Microsoft.DotNet.ProjectModel/Graph/LockFile.cs#L13

[natidea]

👍 Once this is in, I will stage my changes on it and resolve merge conflicts

[natidea]

Target and Props need copyright headers. Mine also include the standard warnings about not modifying them

[eerhardt]

Target and Props need copyright headers. Mine also include the standard warnings about not modifying them

@natidea can you shoot me an example of how it should look that I can copy?

[natidea]

https://github.com/NuGet/NuGet.BuildTasks/blob/dev/src/Microsoft.NuGet.Build.Tasks/Microsoft.NuGet.targets

[eerhardt]

do we actually want project.lock.json included? That's a giant file.

@piotrpMSFT is correct here. The input into the system being tested is the project.lock.json file. And so the tests need a lock file to pass into the component under test. We could try to generate the project.lock.json file during the test, but that would involve calling "restore", which could hit the internet. And in my opinion, unit tests should try limiting their dependencies as much as possible, so the test doesn't break for some un-related reason.

The lock files I'm checking in were generated manually using "dotnet new" for the "dotnet.new" test. And then I added two simple dependencies for the "simple.dependency" test.

I'm open to other ideas on how to test these components that need lock files. But I don't want our unit tests hitting the internet. @natidea - how do you plan on testing your task/targets that need lock files as an input?

[natidea]

@eerhardt I'll use sample project.json files checked in as well

[333fred]

For the ci builds, is there any reason for us to build a different platform, or should we just leave it as is?

[eerhardt]

The build fails without setting this property. I'm not sure if it is because our .sln file is configured incorrectly or not.

Feel free to fix it.

[davkean]

It's because you don't have a default set in the targets. Usually this lives in the project file itself, but as part of the cleanup we'll pull it out into a targets.

[eerhardt]

This is the error I get when I don't set this property:

F:\core-sdk\core-sdk.sln.metaproj : error MSB4126: The specified solution configuration "Debug|x64" is invalid. Please specify a valid solution configuration using the Configuration and Platform properties (e.g. MSBuild.exe Solution.sln /p:Configuration=Debug /p:Platform="Any CPU") or leave those properties blank to use the default solution configuration. [F:\core-sdk\core-sdk.sln]

[eerhardt]

Looks like it is a problem with dotnet build3. See https://github.com/dotnet/cli/blob/feature/msbuild/src/dotnet/commands/dotnet-build3/MSBuildForwardingApp.cs#L41

It is explicitly setting the current architecture as the Platform. That's why this is failing.

[brthor]

Shouldn't it set some form of default platform? What is the alternative here?

[eerhardt]

I think the alternative here is that the .csproj (or the Sdk .props/.targets) declares its default. And the tool running MSBuild doesn't specify it.

[eerhardt]

OK, I've responded to all feedback.

[333fred]

What is the point of this? Doesn't it just set VersionSuffix to empty string if it's empty string?

[eerhardt]

It does. The point of this is to declare VersionSuffix as a property, and give it a default value. That way the next line doesn't materialize the property "out of thin air". This gives a little more structure to what properties are available, because how else are you going to know that $(VersionSuffix) can be set?

[davkean]

I don't understand what that means. No dev is going to be looking through the targets looking for properties to set, they are going to be looking at docs.

[eerhardt]

Well, I'm a dev. And I went looking through the CSharp .targets and that's how I found about about the $(TargetExt) property. I'm not sure which docs I should have been looking in to find out about it.

If this is not a conventional pattern, feel free to remove it.