Set IncludeBuildOutput false when assembly is empty

[lodejard]

• No reason to have extra dll on disk.
• Also adds a compile-time target to advise when a project is
  including an empty build output.

[jtkech]

LGTM

The only restriction i see is for module / theme projects which potentially may have no compile items but we still need their assemblies at runtime to retrieve the attributes we generate, and the assets files we embed on building. Maybe we will also precompile views at the module level which outputs a 2nd assembly.

Normally there is at least a Manifest.cs file but, if not provided, on building we create a default attribute. So, a module which only provides static files, views, recipes, maybe some razor pages ... will not be packed correctly because a module / theme package should contain its assembly even without compile items.

Otherwise good to use this property for all other libraries.

I didn't know this property, thanks.

[lodejard]

Yeah, I was wondering if "Embedded" should also be a clue that silences the warning. Or the warning could be removed altogether to be honest if it gets false positives. The only time it lit up we're for the ".Target" packages, and you could tell from looking at them that the assembly wasn't strictly necessary.

[jtkech]

Hmm, as you want, but i think the warning is a good idea.

You're right for empty ones. And in OC.Module.Targets there is only one marker class, i'm quite sure that we could get rid of this file, so this project would become a candidate and the warning would be useful ;)

So maybe we just need to not import the new OC.Commons.targets in the Directory.Build.targets file of the 2 following folders src/OrchardCore.Modules and src/OrchardCore.Themes. This for the special case of module / theme projects which anyway need their assembly to be packed.

[lodejard]

@jtkech okay, leaving warning in place sounds good to me.

Though we might want to leave the oc.c.targets in all the d.b.targets --- the next PR about version management adds to that file, so the import would need to be brought back anyway if you'd like to merge that one.

[sebastienros]

Did I break this PR by merging dev?

[lodejard]

Nope, this change was actually closed over by the other one, which added to the new .targets file. So this one can be closed because it's correctly reporting there are now zero diffs if you apply it.

[jtkech]

Yes it was included in #1777.

@lodejard just read your last comment, okay no problem.

So, as a last suggestion, in place of only checking @(Compile) we could also check @(EmbeddedResource). Maybe there are other candidates, but at least it would be ok for the special case of modules.

[lodejard]

Okay, sent along #1780 based on that suggestion