Disable packing for source build excluded projects

[chcosta]

Fixes #1183

[chcosta]

FYI @jcagme

[tmat]

This shouldn't be necessary. The entire Pack target is disabled here: https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.Arcade.Sdk/tools/Empty.targets#L15.

[dagood]

Agree, a consistent fix would be to add more disables for any other targets that are running and causing failures. (Edit: To clarify, this was assuming the emptying was working correctly, and the issue was BeforeTargets still activating.)

[tmat]

This needs to be investigated more - from the binlog attached to the issue it seems like the default implementation of Pack target was executed. Not sure why.

[chcosta]

I'm not following. Today, if I build Arcade...

./build.sh --restore -bl --pack /p:DotNetBuildFromSource=true /p:PackageProjectUrl=https://github.com/dotnet/source-build

… I get these failures …

/src/arcade-linux/.dotnet/sdk/2.1.401/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Publish.targets(280,5): error NETSDK1004: Assets file '/src/arcade-linux/artifacts/obj/Microsoft.DotNet.GenAPI/project.assets.json' not found. Run a NuGet package restore to generate this file. [/src/arcade-linux/src/Microsoft.DotNet.GenAPI/Microsoft.DotNet.GenAPI.csproj]
/src/arcade-linux/.dotnet/sdk/2.1.401/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Publish.targets(280,5): error NETSDK1004: Assets file '/src/arcade-linux/artifacts/obj/Microsoft.DotNet.ApiCompat/project.assets.json' not found. Run a NuGet package restore to generate this file. [/src/arcade-linux/src/Microsoft.DotNet.ApiCompat/Microsoft.DotNet.ApiCompat.csproj]
/src/arcade-linux/.dotnet/sdk/2.1.401/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Publish.targets(280,5): error NETSDK1004: Assets file '/src/arcade-linux/artifacts/obj/Microsoft.DotNet.GenFacades/project.assets.json' not found. Run a NuGet package restore to generate this file. [/src/arcade-linux/src/Microsoft.DotNet.GenFacades/Microsoft.DotNet.GenFacades.csproj]
/src/arcade-linux/.dotnet/sdk/2.1.401/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Publish.targets(280,5): error NETSDK1004: Assets file '/src/arcade-linux/artifacts/obj/Microsoft.DotNet.SourceRewriter/project.assets.json' not found. Run a NuGet package restore to generate this file. [/src/arcade-linux/src/Microsoft.DotNet.SourceRewriter/Microsoft.DotNet.SourceRewriter.csproj]
    0 Warning(s)
    4 Error(s)

I looked at the pp file and saw that Empty.targets gets imported before the SDK targets get imported and the SDK targets define Pack. I didn't see that Empty.targets was really doing anything here.

[tmat]

Empty.targets gets imported before the SDK targets get imported and the SDK targets define Pack.

That's the bug.

[chcosta]

The solution is to either set IsPackable=false or ImportNuGetBuildTasksPackTargetsFromSdk=false. Given that the project is of type "<Project Sdk="Microsoft.NET.Sdk">" and "Pack" is defined in the SDK, I'm not certain how you would tell it to import Empty.targets after importing Sdk.targets.

[tmat]

Yeah :(. This used to work when we imported the Arcade.Sdk directly from the the project <Project Sdk="Microsoft.DotNet.Arcade.Sdk"/> since then our targets were the last imported. It doesn't work anymore since we now import Sdk.targets from Directory.Build.targets, which happens before .NET SDK (or other SDKs the project is using) imports their Sdk.targets.

I think we should set ImportNuGetBuildTasksPackTargetsFromSdk=false in Empty.targets instead of redefining Pack target.

There doesn't seem to be a way to import our targets after all SDK targets have been imported, nor we can disable importing SDK .targets. @nguerrera any ideas?

[chcosta]

Defining ImportNuGetBuildTasksPackTagetsFromSdk in Empty.targets sounds reasonable to me. I'll test that out.

[tmat]

I'd remove <Target Name="Pack"/> line since it's not effective.
Also, I'd add to the comment that NuGet targets is imported by Microsoft.Net.Sdk.targets after this file is, so we can't override Pack target here.

[chcosta]

I can modify the comment.

Given that NuGet.Build.Tasks.Pack.targets defines the "Pack" target, I was concerned that there would be somebody in the chain dependent on that target or trying to run that target and would hit an error related to that target not being defined if we didn't preserve the empty "pack" target. It seems safer to me to just define the empty target here. Perhaps that's overly cautious?

[tmat]

Actually, you are right. We need the empty Pack target, if we won't import NuGet.Build.Tasks.Pack.targets

[crummel]

I'm still getting errors with this change and the build command:
DotNetBuildFromSource=true ./build.sh --pack /p:PackageProjectUrl=https://github.com/dotnet/arcade --configuration Release /p:EnableSourceLink=false -bl

/home/chris/arcade/.dotnet/sdk/2.1.401/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Publish.targets(280,5): error NETSDK1004: Assets file '/home/chris/arcade/artifacts/obj/Microsoft.DotNet.SourceRewriter/project.assets.json' not found. Run a NuGet package restore to generate this file. [/home/chris/arcade/src/Microsoft.DotNet.SourceRewriter/Microsoft.DotNet.SourceRewriter.csproj]
/home/chris/arcade/.dotnet/sdk/2.1.401/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Publish.targets(280,5): error NETSDK1004: Assets file '/home/chris/arcade/artifacts/obj/Microsoft.DotNet.GenFacades/project.assets.json' not found. Run a NuGet package restore to generate this file. [/home/chris/arcade/src/Microsoft.DotNet.GenFacades/Microsoft.DotNet.GenFacades.csproj]
/home/chris/arcade/.dotnet/sdk/2.1.401/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Publish.targets(280,5): error NETSDK1004: Assets file '/home/chris/arcade/artifacts/obj/Microsoft.DotNet.GenAPI/project.assets.json' not found. Run a NuGet package restore to generate this file. [/home/chris/arcade/src/Microsoft.DotNet.GenAPI/Microsoft.DotNet.GenAPI.csproj]
/home/chris/arcade/.dotnet/sdk/2.1.401/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Publish.targets(280,5): error NETSDK1004: Assets file '/home/chris/arcade/artifacts/obj/Microsoft.DotNet.ApiCompat/project.assets.json' not found. Run a NuGet package restore to generate this file. [/home/chris/arcade/src/Microsoft.DotNet.ApiCompat/Microsoft.DotNet.ApiCompat.csproj]

[chcosta]

I can do a private build and publish it if you want to validate that way? This works in my local testing but I'm not opposed to getting more validation.

[crummel]

Oh, sorry, that makes sense. Trying that now.

[chcosta]

You have to build the package with this change and then update your repo to consume this change use the SDK you just built, and then build. Have you followed those steps?

[chcosta]

@crummel , I'll wait to merge this until you've completed your validation. Let me know if you hit any issues.

[crummel]

Built this properly now and it looks good to me. Thanks!