Too many defaults that are opt-out, and they are different from what MSbuild and the .NET Core SDK chose as defaults

[jcagme]

Aspnet has a defined set of properties which are different from the default values in Arcade so they had to put workarounds. i.e.

• https://github.com/aspnet/Extensions/blob/master/eng/Workarounds.BeforeArcade.props
• https://github.com/aspnet/Extensions/blob/master/eng/Workarounds.AfterArcade.targets
• https://github.com/aspnet/Extensions/blob/master/eng/Workarounds.AfterArcade.props

Full list of properties they had to work around were:

• Xliff tasks for resx generation.
• DebugType == embedded
• Strong name key == MicrosoftShared
• Test framework == xunit 2.4.1-pre.build.4059. (We were on 2.3 and there are breaking changes between 2.3 and 2.4)
• Using ref assemblies from a NuGet package for .NET Framework projects (Microsoft.NETFramework.ReferenceAssemblies). This breaks some ASP.NET scenarios
• MSBuild warnings as errors. Sometimes warnings are inevitable and we don’t have a way to fix the warnings.
• Projects, by default, do not produce packages when calling dotnet pack. (This is the opposite default that Microsoft.NET.Sdk chose.)
• Any project named “Tests” gets automatic references to xunit. We have some tests that use NUnit.
• The obj/ and bin/ folders are moved to the repo root. (I actually like this but…) it’s not the default MSBuild chose, and we rely on some tools which don’t work unless bin/ and obj/ are in the default MSbuild location.

Is the idea for customers to have these types of workarounds in place defining the values of the properties or we should have a better overriding story in place?

[tmat]

Xliff tasks for resx generation.

What tools are used for localization?

<UsingToolXliff>false</UsingToolXliff>

This should be in Versions.props file. This is the intended way how repos should opt-out of Xliff localization or other optional Arcade SDK features. So I'm not sure what the problem is here.

Using ref assemblies from a NuGet package for .NET Framework projects (Microsoft.NETFramework.ReferenceAssemblies). This breaks some ASP.NET scenarios
true

Ditto. Also move to Versions.props. It would be useful to know what the problem is.

DebugType == embedded

What's the problem with embedding symbols in CI builds? The reason why we do so is that in CI builds symbols are not published to symbol servers. So when investigating a crash dump that was captured during CI build the symbols are not available unless embedded to he DLLs.

Strong name key == MicrosoftShared

This is just default. It's definitely expected that repos that need different key override the default. Why is the default bad?

Test framework == xunit 2.4.1-pre.build.4059. (We were on 2.3 and there are breaking changes between 2.3 and 2.4)

Again, I don't understand what the problem is. If the default doesn't work for you setting <XUnitVersion> in Versions.props is the intended way to override.

Projects, by default, do not produce packages when calling dotnet pack. (This is the opposite default that Microsoft.NET.Sdk chose.)

I'm open for discussion about this one. I believe turning this on by default in the SDK was a mistake, because usually one needs to specify a bunch of properties (like PackageDescription, tags, etc.) in order to have a reasonable content of the package. So I think expressing the intent to produce a package is better than accidentally producing and publishing bad/useless packages (like packages for utilities, etc.).
Note that msbuild MySolution.sln /t:pack runs Pack target on all projects. Unless they specify IsPackable false they produce a package which would get published.
@livarcocc @nguerrera

MSBuild warnings as errors. Sometimes warnings are inevitable and we don’t have a way to fix the warnings.

Any problem with listing the warnings that should be suppressed in <NoWarn>?

Any project named “Tests” gets automatic references to xunit. We have some tests that use NUnit.

There are two knobs you can use:

• repo that doesn't use xunit at all can set UsingToolXUnit=false in Versions.props.
• if some projects use xunit and others nunit then the ones that use nunit can specify
  <TestRunnerName>NUnit<TestRunnerName> in the project file. This will stop XUnit references from being added. We need to add support for NUnit runner to Arcade SDK though, so that /t:Test works for NUnit. This should be fairly easy to do, the targets implemented in https://github.com/dotnet/arcade/tree/master/src/Microsoft.DotNet.Arcade.Sdk/tools/XUnit just need to be reimplemented for NUnit.

I need to document this better.

The obj/ and bin/ folders are moved to the repo root. (I actually like this but…) it’s not the default MSBuild chose, and we rely on some tools which don’t work unless bin/ and obj/ are in the default MSbuild location.

This is a proposed product feature: dotnet/sdk#867.
What tools are those?

[jcagme]

@natemcmaster @dougbu

[natemcmaster]

The main point of this feedback is not the specifics. We could debate those separately. I listed those as examples given to illustrate the larger problem. Microsoft.DotNet.Arcade.Sdk is very heavy handed about what it does with projects, and in my opinion, deviates too much from how Microsoft.NET.Sdk and MSBuild are designed. We're having trouble adopting Arcade in aspnet repos because Microsoft.DotNet.Arcade.Sdk, the YAML templates, BAR manifest generation, publishing, and more are tightly coupled. It has not been reasonable or cheap for my team to adopt Arcade. I have basically had to read every line of Microsoft.DotNet.Arcade.Sdk to make sure using it will change important aspects of the product we produce. It has been so difficult getting just a few of our repos that I am considering just not doing it at all for our larger repos, like aspnet/AspNetCore.

[dougbu]

Any project named “Tests” gets automatic references to xunit.

This doesn't help anything very much if it means tests only run in Arcade. For aspnet/Extensions, aspnet/AspNetcore, and so on, we want to be able to run tests in local builds. Will that be possible?

Also, (especially in aspnet/AspNetCore) many projects with "Tests" in the name are testassets and most of our test projects are named "….Test".

[nguerrera]

Who said tests only run in arcade? We have losts of repos with tests that run in arcade, dotnet test, VS test explorer and build -test?

[nguerrera]

Projects, by default, do not produce packages when calling dotnet pack. (This is the opposite default that Microsoft.NET.Sdk chose.)

I don't remember if this was really discussed before v1. Pack target is implemented entirely in nuget so Microsoft.NET.Sdk didn't make the choice here. cc @rrelyea I don't have a strong opinion for or against how arcade chose to do it. I think the scenario of being able to pack a sln without getting cruft is compelling, and avoiding cruft is usually easier with opt-in than opt-out.

[nguerrera]

Moving between different infrastructure is always hard.

Arcade being opinionated is a big part of its appeal to me. There are a number of things that are requirements for our infrastructure that don't apply to file -> new project. I don't think we can make it a requirement that all default behaviors are the same between those. I think having certain important things done for me by default if I follow opinionated conventions is a good approach. Historically, when we have relied on each team to do it their own way, we have duplicated effort at best and had gaps in which parts of the stack were meeting certain requirements at worst. We finally have things like source linking and symbol publishing just working without having to maintain it ourselves.

I get that if you already built the equivalent with a different approach, then migrating is hard. The dividends will still come in the future when we can share improvements across more teams with less duplicated effort.

[markwilkie]

Now that we've shipped preview 2, I'll circle back around and work towards getting this area of 'defaults' more crisp. More soon.....

[tmat]

What @nguerrera says. It is the goal of Arcade SDK to set certain standards and enforce some amount of uniformity. There is indeed a balance - we need to give repos freedom to customize certain things but not others. We can discuss each particular requirement, convention and default. But if this issue is asking for Arcade SDK to not do that then it can be closed as "won't fix, by design".

[dougbu]

Who said tests only run in arcade?

Nobody. My statement included an "if" and I asked for confirmation.

[nguerrera]

Sorry, confirming that it is totally possible.

[natemcmaster]

The original list of things @jcagme posted were given in response to the question, "what did aspnet have to workaround?" and "what were the pain points in adopting Arcade?". You have my feedback. You're welcome to dismiss it. All I wanted to let you know is that adopting Arcade is not trivial, the things it does are not always right, and in my opinion may of its behaviors are magic, clever, and fancy and a pain to demystify.

From https://github.com/dotnet/arcade/blob/master/Documentation/Overview.md

Overview / Introduction

We need well-understood and consistent mechanisms to consume, update, and share engineering infrastructure across the .NET Core team.

The primary concept is to break the infrastructure into “pay for play” components, such that one piece of functionality can be consumed, with minimal dependencies. The idea is to not force unnecessary dependencies, thus making it more reasonable to consume only what is needed.

This approach publishes what amounts to “public surface area” for the shared engineering infrastructure. These “contracts” then allow for the product teams to reason about how (or if) they participate and manage their engagement with the common infra over time. In short, the product teams “pull” what is needed, when it’s needed.

...

Principles

• Updates and changes should always be done with all the repos (not just yours) in mind. This implies compromise by all to achieve a better common goal.
• Simplicity and austerity are our friends. We want to avoid clever, magic, or fancy features.

[tmat]

"what did aspnet have to workaround?" and "what were the pain points in adopting Arcade?". You have my feedback. You're welcome to dismiss it

I am not dismissing any specific feedback. I welcome feedback on any particular setting/convention and am ready to explain the reasons for them. What I am saying above is that we need to constrain repos to some degree and disallow some customizations to get a sane system.

behaviors are magic, clever, and fancy and a pain to demystify.

Perhaps. It depends on what you consider magic and fancy. Again, this is not actionable unless we talk about specific feature.

[natemcmaster]

Would you rather I open an issue for each default that I think should be opt-in? Or have a long discussion on this thread?