search

nirin / msbuild-sdks

MSBuild project SDKs
https://nirin.dev/projects/msbuild-sdks
Other
18 stars 2 forks source link

Using DefaultItems in PR mode does not work properly #5

Closed mkaring closed 6 years ago

mkaring commented 6 years ago

Hello,

I noticed that the default values for EnableDefaultMyAppItems and EnableMyAppCodeGenerator are set in "MSBuild.NET.DefaultItems/Build/PlatformItems/Desktop.VisualBasic.targets". How ever they are first used in "MSBuild.NET.DefaultItems/Build/PlatformItems/Desktop.VisualBasic.props". Since the "*.target" files are included later, the default values do not take effect.

Is this intended?

Nirmal4G commented 6 years ago

This SDK was supposed to be for my projects only, that's why I didn't write the docs for the properties. But hey, thanks for using the SDK!

Most of the features will soon be merged in MSBuild.Sdk.Extras. In the meantime if you need any help converting/setting up your projects, create an issue like this and I'll help you.

Nirmal4G commented 6 years ago

Is this intended?

Yes, since Items are always evaluated on the 2nd pass of the build process. It should take effect, like all other properties in the other targets.

I'll take a look and see what's wrong with the VisualBasic DefaultItems overrides.

Nirmal4G commented 6 years ago

When using DefaultItems in PackageReference mode, the Microsoft.NET.Sdk's DefaultItems are brought in during the MSBuildProjectExtensions (i.e. NuGet) imports, which resets the corresponding ones (None, Compile and EmbeddedResource items) specified in the Main SDK!

This is the issue that's causing the above mentioned problem. Since we can't control the order of the import as it's set within Common props, the only workaround is to use it as a SDK import like so...

<Project>
  <!-- Other things + Overrides -->

  <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk"/>
  <Import Project="Items.props" Sdk="MSBuild.NET.DefaultItems"/>

  <!-- Other things + Overrides -->

  <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk"/>
  <Import Project="Items.targets" Sdk="MSBuild.NET.DefaultItems"/>

  <!-- Other things + Overrides -->
</Project>

OR

<Project Sdk="MSBuild.NET.Extras.Sdk">
  <!-- Project Content -->
</Project>

Note.: The MSBuild.NET.Extras.Sdk v1.2.0 and above will include the Default Items.

mkaring commented 6 years ago

This method works starting with 1.2.0 ?

<Project Sdk="MSBuild.NET.Extras.Sdk">
  <!-- Project Content -->
</Project>

As of 1.1.0 MSBuild.NET.Extras.Sdk does not seem to reference MSBuild.NET.DefaultItems at all.

Is there any timeline for the release of 1.2.0?

Nirmal4G commented 6 years ago

As of 1.1.0 MSBuild.NET.Extras.Sdk does not seem to reference MSBuild.NET.DefaultItems at all.

It was supposed to be used as PackageReference but here this doesn't seem to work, so I'm integrating the default items within the SDK itself. But if you are using Microsoft.NET.Sdk you can use it as I mentioned above.

Is there any timeline for the release of 1.2.0?

Expecting this Sunday/Monday.

mkaring commented 6 years ago

I tried to find how you solved the problem with the DefaultItems in version 1.2.0 ... but for some reason I can't find the commits changing this in the history.

Nirmal4G commented 6 years ago

The changes are in feature/web branch.

mkaring commented 6 years ago

I see. Wouldn't the more straight forward way be to include the default items package using an include? To avoid maintaining two packages containing the same files?

mkaring commented 6 years ago

There are still some problems here. The example projects do not work properly because FrameworkProjectType is not working as expected. The reason for this is in the file "Source\MSBuild.NET.Extras.Sdk\ProjectSystem\MSBuild.NET.ProjectSystem.targets". The problem is the condition Condition="'$(FrameworkProjectType)' == 'Wpf' AND '$(TargetFrameworkIdentifier)' == '.NETFramework'" The TargetFrameworkIdentifier property is not set at this point, because this property is set in the target files of the "Microsoft.NET.Sdk". How ever this target file is included after the targets of the extras sdk. I think the order of insertions is wrong. The "Microsoft.NET.Sdk" has to be included first, followed by the ones of the extras sdk. For the *.props files is is the other way. First the extras sdk and then the microsoft sdk.

Nirmal4G commented 6 years ago

I see. Wouldn't the more straight forward way be to include the default items package using an include? To avoid maintaining two packages containing the same files?

There's reason for this, and I'll resolve them very soon, that's why they are in a feature/* branch.

Nirmal4G commented 6 years ago

The example projects do not work properly because FrameworkProjectType is not working as expected.

Thanks for reporting this, fixed in recent commit.