Clean up! (Fixes #21)

[Nirmal4G]

1. Added Sdk support (this only works if the MSBuildSdksPath is set or you can copy it to the Sdks folder in VS2017 Installation)
2. I separated the targets by platform along with setting the language targets. (i.e. MonoAndroid -> MonoAndroid.targets, etc...)
3. Added support for generic short frameworks of form (fx.nameNN.N-profile1<+profile2+...>)
4. Some little fixes here and there. (automagically because of restructuring and clean-up)

Other than that all of the behaviour remains the same.
Almost all private properties' (_SdkExtras...) names are changed, since they are not clear and not to the context.

[clairernovotny]

First thank you, I know this is a lot of work as there's a large surface area here. Overall, I like the improvements.

I would ask though that the private variables be restored, or at least prefixed with _SdkExtras since I was trying to avoid using a private variable that the Sdk itself might use sometime.

Also, there seem to be a bunch of extra Exists(...) checks. If the file to be imported is within this package itself, those checks shouldn't be needed since either all of the targets/props exist, or none do. For props/targets supplied elsewhere outside this package, those checks are good.

Given that these are hard to test, I just wanted to see how you've been testing these and ensuring the functionality?

Thanks again, it's just a lot of changes and will take time for me to verify as well.

[Nirmal4G]

I would ask though that the private variables be restored, or at least prefixed with _SdkExtras since I was trying to avoid using a private variable that the Sdk itself might use sometime.

I went through the Microsoft.NET.Sdk, they were not using _Sdk at all. At the end of all fixing and testing I will check-in with Sdk team to make sure.

Also, there seem to be a bunch of extra Exists(...) checks.

I ported over these from my repo and it had a lot of redundant checks. It's all fixed now!

Given that these are hard to test, I just wanted to see how you've been testing these and ensuring the functionality?

1. I'm using NuGetizer-3000 and It is very good, for a project that's in planning stages.
2. I'm also testing if we can use NuGet.Build.Tasks.Pack as a NuGet dependency rather than within.
3. I have a lot of sample projects (simple ones, one for each platform) that I've used, so I use them to test and if all of them builds and creates packages successfully then it succeeds
4. I will also push a test project for each of those project types.

[clairernovotny]

Does the NuGet workaround still work with the 1.0.0-1.0.4 SDK? What pulls down the NuGet.Build.Tasks.Pack 4.3.0 package? We cannot have a package ref within a package since NuGet only does one pass to evaluate deps. That's why I cannot just include the UWP meta-package either.

[clairernovotny]

Also, the <DebugType>full</DebugType> can be removed ... Xamarin supports all of it now.

[clairernovotny]

The biggest Q is the NuGet one to make sure the workarounds still work on older SDK's. Otherwise, it looks good -- I need to test it more, but I can make some minor clean-ups like the DebugType.

[Nirmal4G]

What pulls down the NuGet.Build.Tasks.Pack 4.3.0 package? We cannot have a package ref within a package since NuGet only does one pass to evaluate deps. That's why I cannot just include the UWP meta-package either.

It works in my setup. Also I deleted the package in my .nuget cache and tested again. I still works!
Can you tell me what is the problem you are facing?
Is the package not restoring properly?

Does the NuGet workaround still work with the 1.0.0-1.0.4 SDK? The biggest Q is the NuGet one to make sure the workarounds still work on older SDK's.

I'm still testing it. Need to setup a clean VM to test them. Will take a day or two.

[Nirmal4G]

Also, the full can be removed ... Xamarin supports all of it now.

I removed them. Thanks for the tip.

[clairernovotny]

I just don't see how the NuGet pack 4.3 targets get downloaded initially...something has to do that. I don't think it's safe to do that without knowing how it'll work.

[Nirmal4G]

I just don't see how the NuGet pack 4.3 targets get downloaded initially...something has to do that. I don't think it's safe to do that without knowing how it'll work.

Microsoft (R) Build Engine version 15.3.409.57025 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

Build started 22-09-2017 11:24:45 PM.
Project "D:\Projects\GitHub\MSBuildSdkExtras\MSBuild.Sdk.Extras.Tests\MSBuild.Sdk.Extras.Tests.csproj" on node 1 (Restore;Build;Pack target(s)).
Restore:
  Restoring packages for D:\Projects\GitHub\MSBuildSdkExtras\MSBuild.Sdk.Extras.Tests\MSBuild.Sdk.Extras.Tests.csproj...
    GET https://api.nuget.org/v3-flatcontainer/nuget.build.tasks.pack/index.json
    OK https://api.nuget.org/v3-flatcontainer/nuget.build.tasks.pack/index.json 1094ms
    GET https://api.nuget.org/v3-flatcontainer/nuget.build.tasks.pack/4.3.0/nuget.build.tasks.pack.4.3.0.nupkg
    OK https://api.nuget.org/v3-flatcontainer/nuget.build.tasks.pack/4.3.0/nuget.build.tasks.pack.4.3.0.nupkg 1273ms
  Installing NuGet.Build.Tasks.Pack 4.3.0.
  Installing MSBuild.Sdk.Extras 1.2.0.
  Committing restore...
  Generating MSBuild file D:\Projects\GitHub\MSBuildSdkExtras\MSBuild.Sdk.Extras.Tests\obj\MSBuild.Sdk.Extras.Tests.csproj.nuget.g.props.
  Generating MSBuild file D:\Projects\GitHub\MSBuildSdkExtras\MSBuild.Sdk.Extras.Tests\obj\MSBuild.Sdk.Extras.Tests.csproj.nuget.g.targets.
  Writing lock file to disk. Path: D:\Projects\GitHub\MSBuildSdkExtras\MSBuild.Sdk.Extras.Tests\obj\project.assets.json
  Restore completed in 21.33 sec for D:\Projects\GitHub\MSBuildSdkExtras\MSBuild.Sdk.Extras.Tests\MSBuild.Sdk.Extras.Tests.csproj.

See the Restore log. I didn't assume, It worked! And I knew It would work!! 🤗👍👌

Don't worry I'm testing with my personal projects. If it doesn't work I'll revert them, that's why we have git right? 😄

[clairernovotny]

When you test, make sure to also have a global.json present that sets the Sdk version to 1.0.4. That's where I think some of the issues may arise.

[Nirmal4G]

Everything works just great!

We can ship it!

[clairernovotny]

I have a better idea re the pack targets. How about this -- detect if someone tries to call pack (override the right target or add a BeforeTargets target, and if the SDK version is < 2.0, just throw an error that says there are known issues and to use the 2.0+ SDK to build. The error can be in a condition so it can be suppressed if set at the command line. Then all the NuGet workarounds can go away completely. I think it's better to tell people just to use the latest SDK rather than ship 1.x workarounds...

[Nirmal4G]

It's all fixed now. Works perfectly with all the SDKs!
Didn't you see my comment: #23 (comment)

Also please take a look at the review comments.

[clairernovotny]

Yes, I saw, but still need to test. I'm not sure how the dotnet 1.0.4 SDK would be downloading the nuget pack targets though...

[Nirmal4G]

I've tested with all the SDKs, It works perfectly fine.

No issues so far! Even if I missed out any issues, point them to me, I'd be happy to resolve them.

[clairernovotny]

Thank you -- I've merged this in and made a few changes beyond this. I've pushed out a preview package to NuGet (1.1.0-preview.2.build.6) to test this out further over the coming couple weeks.

[Nirmal4G]

Nice Changes... Building with csproj, that means can we work it with as ProjectReference?

If we can, then I can use variants of test projects to test them out, I'll refer to dotnet/sdk tests

[Nirmal4G]

One thing I don't understand is NuGet workarounds, even 1.x sdks can no longer need that except the ones exclusively for that sdk, why are you including them back?

If you don't want a dependency then I find a way to dynamically pull them in.

[clairernovotny]

I'm not sure if ProjectReference won't do that targets/props import the same way as a package, I have not tried it.

Not sure I understand the issue with the workarounds?

I definitely don't want to have a dependency on the nuget pack package, since they're only needed for the 1.x SDK. I don't want to get out-of-sync as that revs higher for a limited case. For 1.x people who can't move to the 2.0 SDK, at least there's a workaround.

[Nirmal4G]

I definitely don't want to have a dependency on the nuget pack package, since they're only needed for the 1.x SDK.

I definitely agree.

I don't want to get out-of-sync as that revs higher for a limited case.

Ok, I understand where you might got me wrong. The dependency on the package does not add it too the latest SDKs. It's only for the 1.x SDKs.

I'll try to dynamically pull them in. I did this for my internal SDKs, might be able to port over here easily.

[clairernovotny]

Can I ask why there's a need for https://www.nuget.org/packages/MSBuild.NET.Extras.Sdk? I'm happy to take PR's here, as you can see, just looking to avoid confusion in the community.

Thanks

[Nirmal4G]

That's for myself, more like a private one, It should only contain pre-release packages, even though they are stable packages. I test new things there.

Also that Package contains overrides for some defaults that I don't very well receive in the Microsoft.NET.Sdk. Instead of having to include those overrides for each one of my projects, So, I put it there.

People always use stable packages, so I thought It wouldn't confuse them. Can I can include a message saying it's private and ask them to use yours?

[clairernovotny]

If you're looking to test things, I'd suggest checking out MyGet.org -- you can get a free public feed there and that's usually where I test things out. Would that work? :)

[Nirmal4G]

I'm happy to take PR's here.

This PR is from that repo, after exhaustive testing, I might say 😇
Your project helped me a lot. I started from there dotnet/sdk#889 (comment)
I'll always push acceptable changes to you as PR.

If you're looking to test things, I'd suggest checking out MyGet.org

I'm using MyGet, only for CI and those are not stable. plus NuGet is the default provider.
I don't want to setup MyGet feed for every project.