-
Notifications
You must be signed in to change notification settings - Fork 165
Project simplification proposal #332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Project simplification proposal #332
Conversation
|
||
This proposal is to simplify the project file while giving the user better control over breaking changes and new warnings. | ||
|
||
The approach is versioned default properties specified by a `Base` attribute in the project file, a `Directory.Build.props`, or an `Import` file. This attribute specifies a version of the file containing the default properties. The project file is further simplified by `Type` attribute on the `Project` element. The `Type` is not versioned, although versioned default properties can be added as part of the `Base` evaluation. The `Type` attribute replaces the current `SDK` and the `Type` file contains both the `SDK` name and properties that apply to the application `Type`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A worry I have about Type
is that it's likely going to create dialects of SDK projects. Essentially, are you using old school Sdk
or new Type
. I'm not sure the value of this attribute outweighs the cost of having competing dialects in the ecosystem.
I don't have the same worry about Base
. I see that as say SdkVersion
which is something that large repos will care about and will be in centralized places, much like say CPM is today. It will be a big repo property but likely won't impact most .NET development. An expert feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. I see a main value as simplifying projects for new users - ensuring everything in the project file is something they understand.
## Motivation | ||
[motivation]: #motivation | ||
|
||
The fundamental benefit of this effort is to speed up and remove friction for users that wish to adopt tooling improvements quickly, while also allowing users to delay incorporating change until it is not disruptive. To do this, we need to allow developers a friction free experience on upgrade, which means that with no more work than previously they have control over the upgrade process. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does friction free mean here though? There are always behavior differences when updating between major versions of the .NET SDK. Are we saying that all behaviors are tied to Base
, just diagnostics, etc ... Need to be clear where the line is with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be largely up to teams, but anything that would be tied to TFM today would be a good candidate. I think the proposal needs a little more specifics on this, so good point. Behavior would also need to be tied to TFM if Base wasn't set ,or the Base would need to be automatically set when the user did not. Second seems easiest.
|
||
The fundamental benefit of this effort is to speed up and remove friction for users that wish to adopt tooling improvements quickly, while also allowing users to delay incorporating change until it is not disruptive. To do this, we need to allow developers a friction free experience on upgrade, which means that with no more work than previously they have control over the upgrade process. | ||
|
||
All breaking changes and new warning are introduced because we believe the positive impact outweighs the pain of the change. If a change is not breaking and does not add new warnings, we just add the feature and they are available when the user upgrades. If the change breaks everyone, we are unlikely to make the change. This proposal affects the occasional features that may break a few projects some of the time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we hide every breaking change behind this flag what does installing a new major version of the SDK actually do when Base
points to the previous version? Is it expected to be 100% a no-op? If so then why are customers upgrading? What is the experience that they are expecting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users would be upgrading to get the new tools. We do not want to block that.
Are you thinking in terms of every change being a breaking change, and therefore if no breaking change no change?
That wasn't the intent, so more clarity is needed.
Some teams have a low tolerance for changes including new warnings or something like removing obsoleting CLI commands. Protecting teams from those kind of breaks is the intent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users would be upgrading to get the new tools.
Isn't what we already have - "leave the TFS as is, install the new SDK" though?
|
||
All breaking changes and new warning are introduced because we believe the positive impact outweighs the pain of the change. If a change is not breaking and does not add new warnings, we just add the feature and they are available when the user upgrades. If the change breaks everyone, we are unlikely to make the change. This proposal affects the occasional features that may break a few projects some of the time. | ||
|
||
The break may be a new warning. We treat new warnings as breaking changes because they can be disruptive to projects that rely on `TreatWarningsAsErrors` to guard against regressions in their own code. We want to make it easy for users to incorporate changes and warnings as quickly and smoothly as possible to achieve these benefits. An example of a new feature that created new warnings and clearly benefits almost all projects is the `NuGetAudit` flag which informs you if you are using a vulnerable package directly or transitively. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree there is value in having new warnings across our tooling stack tied to a single property. But we have that today with SdkVersion
so have to ask why is that not working but we think that Base
will?
|
||
Your approach to upgrading the `TargetFramework` of your project varies from wanting the newest features and performance improvements as quickly as possible to postponing as long as possible to ease the upgrade effort. | ||
|
||
### Motivation for `Base` attribute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does Base
need to be an attribute? Think we need to be specific about that here. Not having it as a property seems like it will be a problem:
- Cannot test changes to a repo by using
-p:Base=11.0.0
on the command line - Will not be usable in non-SDK projects who do not have these attributes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the only way this feature could work with how MSBuild functions would be if specifying the Base
attribute and Type
would lower down to something like a property set before the SDK Imports.
For example, given:
<Project Base="10.0.100" Type="Console">
<PropertyGroup>
<TargetFramework>net10.0</TargetFramework>
</PropertyGroup>
</Project>
It would be equivalent to:
<Project>
<PropertyGroup>
<ProjectSdkBase>10.0.100</ProjectSdkBase>
<ProjectSdkType>Console</ProjectSdkType>
</PropertyGroup>
<Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />
<PropertyGroup>
<TargetFramework>net10.0</TargetFramework>
</PropertyGroup>
<Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
</Project>
With this sort of design contract, you would be able to set ProjectSdkBase
with -p:ProjectSdkBase=11.0.0
and it would override the Base
attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaredpar the comment about testing is a good one.
@jkoritzinsky that would set the base value, but the intent was for it to also bring in a props file of values so behaviors could have discrete flags - think ImplicitUsings
. So, your basically correct, but it would a bit more the equivalent to:
<Project>
<Import Project="Console.props" /> <!-- Where Console.props contains the SDK and set the ProjectType, etc. -->
<Import Project="Base10.0.100.props" /> <!-- Where Base10.0.100 contains set the ProjectBase, etc -->
<PropertyGroup>
<TargetFramework>net10.0</TargetFramework>
</PropertyGroup>
<Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
</Project>
One of the reasons they are separate is because the base will often be set in Directory.Build.props or an import.
</Project> | ||
``` | ||
|
||
For users that are not ready to upgrade their `TargetFramework` but want any new warnings that may improve their application, they can use a higher version of tooling defaults: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Base
version is the same in all three examples here. Did you mean for it to change in this example (based on this text)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the text is referring to the fact that 10.0.100
is a higher tooling version than the tfm of net482
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrewlock is correct. These examples are intended to show the TFM replacing the default TFM in the Base and why you might want to do that.
|
||
In this case, `TargetFramework` should be set to `net11` because it is closer to te user. | ||
|
||
We are particularly concerned about precedence being predictable because the results may be subtle or _spooky action at a distance_ because the properties set may affect a very different part of the build. Also, debugging MSBuild via binlogs and the structured log viewer is a skill many of our users do not have because they rarely or never need to debug MSBuild logic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think you need to illustrate why this would not work if Base
were a property. also this is likely to be expert only territory so why not say that Base
must be a property and it can only be set in a Dir.Build.props file? There can be an error if it's modified anywhere else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would agree here.
Instead of allowing this and creating a fully separate evaluation model in MSBuild for just this feature, I'd prefer using something like the new MSBuild BuildChecks feature that would make specifying a Base
value "too late" (ie not in a project file or a Directory.Build.props or something imported from a Directory.Build.props) an error.
Otherwise, you end up further complicating the already confusing MSBuild evaluation mechanisms even more. Although this feature would be meant to simplify how MSBuild works, it would just make it even more difficult to understand what's going on when it doesn't meet the user's expectations (primarily for users who can read a binlog to solve very simple problems through experts in using MSBuild for enterprise-level build scripts but don't follow .NET release notes)
|
||
_The name `Type` is not final._ | ||
|
||
The `SDK` attribute on the `Project` element can be replaced by a `Type` attribute. It is not legal to have both a `Type` and an `SDK` attribute. If neither exists, the project will behave in the same manner as projects without a `SDK` attribute behave today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Type to replace Sdk think we need some very strong motivating examples cause it will create dialects of the language.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think this is any more than other features?
<Project Base="11.0.100" | ||
Type="Console"> | ||
<PropertyGroup> | ||
<TargetFramework>net10<TargetFramework> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<TargetFramework>net10<TargetFramework> | |
<TargetFramework>net10.0<TargetFramework> |
this applies throughout the doc
|
||
When a there is a breaking change or a new warning, there is a new MSBuild property (as today). For breaking changes or new warnings, the feature will be turned off if it `false` or the equivalent `disable`. The defaults specified by `Base` will turn on the features we strongly recommend. If you disagree with this for your project, you can reset the value to `false`. | ||
|
||
The key element of the `Base` design is that it is versioned and released with each SDK. This allows you to initially experiment with, and then adopt all the breaking changes and new warnings with a single value. The proposed structure of this version is the corresponding SDK version number, such as `10.0.100`, with a possible simplification to just `10`. The `Base` defaults include the `TargetFramework` so the user manages a single version number, in most cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I simplify to just "10", does that take the earliest or latest 10.x.x?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on comments later on, I understand this to mean earliest, i.e. 10.0.100
, which is the opposite of what I expected intuitively 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is an open question at the moment. Do we want an explicit gesture like *
for floating?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure floating makes sense unless we intend to make the base definitions acquirable via NuGet? What is the intended behavior if the version specified is newer than the base included in the SDK currently being used?
|
||
This proposal is to simplify the project file while giving the user better control over breaking changes and new warnings. | ||
|
||
The approach is versioned default properties specified by a `Base` attribute in the project file, a `Directory.Build.props`, or an `Import` file. This attribute specifies a version of the file containing the default properties. The project file is further simplified by `Type` attribute on the `Project` element. The `Type` is not versioned, although versioned default properties can be added as part of the `Base` evaluation. The `Type` attribute replaces the current `SDK` and the `Type` file contains both the `SDK` name and properties that apply to the application `Type`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might need a bit of additional explanation, as when I read this paragraph I wondered what type of SDK
you meant, and then I looked at an Aspire host project of mine and then the wondering deepened because there's two types of "SDK" in it:
<Project Sdk="Microsoft.NET.Sdk">
<Sdk Name="Aspire.AppHost.Sdk" />
<!-- snip -->
</Project>
I suspect it's the attribute rather than the element that's being referred to, but it's not 100% clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, the Sdk="Microsoft.NET.Sdk"
is a shorthand for inserting a <Sdk Name="Microsoft.NET.Sdk" />
element immediately under the <Project>
element (before the Aspire SDK), which is its own shorthand for another thing (which isn't important for this conversation). That's why you see two in the Aspire project.
|
||
The fundamental benefit of this effort is to speed up and remove friction for users that wish to adopt tooling improvements quickly, while also allowing users to delay incorporating change until it is not disruptive. To do this, we need to allow developers a friction free experience on upgrade, which means that with no more work than previously they have control over the upgrade process. | ||
|
||
All breaking changes and new warning are introduced because we believe the positive impact outweighs the pain of the change. If a change is not breaking and does not add new warnings, we just add the feature and they are available when the user upgrades. If the change breaks everyone, we are unlikely to make the change. This proposal affects the occasional features that may break a few projects some of the time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All breaking changes and new warning are introduced because we believe the positive impact outweighs the pain of the change. If a change is not breaking and does not add new warnings, we just add the feature and they are available when the user upgrades. If the change breaks everyone, we are unlikely to make the change. This proposal affects the occasional features that may break a few projects some of the time. | |
All breaking changes and new warnings are introduced because we believe the positive impact outweighs the pain of the change. If a change is not breaking and does not add new warnings, we just add the feature and they are available when the user upgrades. If the change breaks everyone, we are unlikely to make the change. This proposal affects the occasional features that may break a few projects some of the time. |
|
||
The break may be a new warning. We treat new warnings as breaking changes because they can be disruptive to projects that rely on `TreatWarningsAsErrors` to guard against regressions in their own code. We want to make it easy for users to incorporate changes and warnings as quickly and smoothly as possible to achieve these benefits. An example of a new feature that created new warnings and clearly benefits almost all projects is the `NuGetAudit` flag which informs you if you are using a vulnerable package directly or transitively. | ||
|
||
We realize that your capacity to accommodate change and possible changes to your code varies widely by team, time of year, business considerations, and whether your already working on the code. These changes also affect all of the projects you are working on or building. Thus, breaks and warning that occur on tooling upgrades are undesirable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We realize that your capacity to accommodate change and possible changes to your code varies widely by team, time of year, business considerations, and whether your already working on the code. These changes also affect all of the projects you are working on or building. Thus, breaks and warning that occur on tooling upgrades are undesirable. | |
We realize that your capacity to accommodate change and possible changes to your code varies widely by team, time of year, business considerations, and whether you're already working on the code. These changes also affect all of the projects you are working on or building. Thus, breaks and warnings that occur on tooling upgrades are undesirable. |
|
||
### Motivation for `Base` attribute | ||
|
||
A new `Base` attribute on the `Project` element of the project file, `Directory.Build.props`, or an `Import` file and will specify defaults for all common project properties. This means the project file will contain two attributes and the things you add, such as packages. In cases where common properties like `Nullable` have been moved to `Directory.Build.props`, or an `Import` file, that file can be simplified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new `Base` attribute on the `Project` element of the project file, `Directory.Build.props`, or an `Import` file and will specify defaults for all common project properties. This means the project file will contain two attributes and the things you add, such as packages. In cases where common properties like `Nullable` have been moved to `Directory.Build.props`, or an `Import` file, that file can be simplified. | |
A new `Base` attribute on the `Project` element of the project file, `Directory.Build.props`, or an `Import` file will specify defaults for all common project properties. This means the project file will contain two attributes and the things you add, such as packages. In cases where common properties like `Nullable` have been moved to `Directory.Build.props`, or an `Import` file, that file can be simplified. |
|
||
A new `Base` attribute on the `Project` element of the project file, `Directory.Build.props`, or an `Import` file and will specify defaults for all common project properties. This means the project file will contain two attributes and the things you add, such as packages. In cases where common properties like `Nullable` have been moved to `Directory.Build.props`, or an `Import` file, that file can be simplified. | ||
|
||
When a there is a breaking change or a new warning, there is a new MSBuild property (as today). For breaking changes or new warnings, the feature will be turned off if it `false` or the equivalent `disable`. The defaults specified by `Base` will turn on the features we strongly recommend. If you disagree with this for your project, you can reset the value to `false`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a there is a breaking change or a new warning, there is a new MSBuild property (as today). For breaking changes or new warnings, the feature will be turned off if it `false` or the equivalent `disable`. The defaults specified by `Base` will turn on the features we strongly recommend. If you disagree with this for your project, you can reset the value to `false`. | |
When there is a breaking change or a new warning, there is a new MSBuild property (as today). For breaking changes or new warnings, the feature will be turned off if it is `false` or the equivalent `disable`. The defaults specified by `Base` will turn on the features we strongly recommend. If you disagree with this for your project, you can reset the value to `false`. |
</Project> | ||
``` | ||
|
||
This approach is simple for a project file. It might be extended to complex repos by placing the `PropertyGroup` containing the new properties at the start of whichever file (project file, `Directory.Build.props` or `Import` file) the `Base` attribute appeared in. However, the command would probably be run in the same directory as the file containing the `Base` attribute and specify the file if there were multiple props files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach is simple for a project file. It might be extended to complex repos by placing the `PropertyGroup` containing the new properties at the start of whichever file (project file, `Directory.Build.props` or `Import` file) the `Base` attribute appeared in. However, the command would probably be run in the same directory as the file containing the `Base` attribute and specify the file if there were multiple props files. | |
This approach is simple for a project file. It might be extended to complex repos by placing the `PropertyGroup` containing the new properties at the start of whichever file (project file, `Directory.Build.props` or `Import` file) the `Base` attribute appeared in. However, the command would probably be run in the same directory as the file containing the `Base` attribute and specify the file if there were multiple `.props` files. |
## Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
This introduces a new concept to .NET users, and some additional complexity in the overall MSBuild evaluation. Users would see this in areas such as understanding binlogs. We think the trade off is worthwhile because users get small clean project files the ability to control new warnings/breaking changes independent of both SDK version and TargetFramework. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This introduces a new concept to .NET users, and some additional complexity in the overall MSBuild evaluation. Users would see this in areas such as understanding binlogs. We think the trade off is worthwhile because users get small clean project files the ability to control new warnings/breaking changes independent of both SDK version and TargetFramework. | |
This introduces a new concept to .NET users, and some additional complexity in the overall MSBuild evaluation. Users would see this in areas such as understanding binlogs. We think the trade off is worthwhile because users get small clean project files with the ability to control new warnings/breaking changes independent of both SDK version and `TargetFramework`. |
## Alternatives | ||
[alternatives]: #alternatives | ||
|
||
We could continue to tie new features to TFM. THe drawback is that users generally upgrade their TFM to keep their projects in support. While this avoids breaking changes/warnings occurring on tooling upgrade, it may either delay adopting desirable new features, or result in breaking changes/warnings happening when the user is just trying to stay in support. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could continue to tie new features to TFM. THe drawback is that users generally upgrade their TFM to keep their projects in support. While this avoids breaking changes/warnings occurring on tooling upgrade, it may either delay adopting desirable new features, or result in breaking changes/warnings happening when the user is just trying to stay in support. | |
We could continue to tie new features to TFM. The drawback is that users generally upgrade their TFM to keep their projects in support. While this avoids breaking changes/warnings occurring on tooling upgrade, it may either delay adopting desirable new features, or result in breaking changes/warnings happening when the user is just trying to stay in support. |
|
||
### What capacity to pin and float should we offer for `Base` versions? | ||
|
||
### What about multi-targeting? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the main question I had going in 😃. For example, in November the current plan is to ship Swashbuckle.AspNetCore multi-targeting net8.0
, net9.0
and net10.0
(plus netstandard2.0
where relevant, but I'm tempted to drop this...). It's quite a large codebase (at least compared to my own), so it would be interesting to see what the challenges (or not) of having .NET 10 "opinions" applied to the code is, particularly if there are any features enabled that don't work down-level. As an example of this, today we use #if NET
(or similar) defines in various places to make analysers happy about using newer/better APIs that aren't available to netstandard2.0
.
|
||
### Should we restrict `Type` to the project file? | ||
|
||
Stated differently, should we allow `Type` to be in `Directory.Build.props`. Since `Type` determines the SDK and that contains the targets that run most build operations, this is tricky. Also, it may be best for users to see this in the project file for clarity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stated differently, should we allow `Type` to be in `Directory.Build.props`. Since `Type` determines the SDK and that contains the targets that run most build operations, this is tricky. Also, it may be best for users to see this in the project file for clarity. | |
Stated differently, should we allow `Type` to be in `Directory.Build.props`? Since `Type` determines the SDK and that contains the targets that run most build operations, this is tricky. Also, it may be best for users to see this in the project file for clarity. |
I feel like one thing which needs to be outlined here is how does adding this actually simplify the typical project structure? From my POV this just adds an additional way to achieve what system is already capable of, while adding additional complexity in that there is yet another way to achieve the same goals. |
Something that isn't clear to me is how does From https://learn.microsoft.com/en-us/dotnet/core/project-sdk/overview#project-files
<Project Sdk="MSBuild.Sdk.Extras/2.0.54">
...
</Project> |
</Project> | ||
``` | ||
|
||
If there is a `Directory.Build.props` or `Import` file, it can contain the `Base` attribute, although it cannot contain the `Type` attribute. This is because the `Type` attribute specifies the SDK instructions for build evaluation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a Base
attribute specified in an Import
ed file retroactively affect previously imported files (including in the Microsoft.NET.Sdk)? That would be very unintuitive to how MSBuild works today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what you mean by retroactive. If you can clarify the scenario and which one you would intuit to win, that would be awesome.
There is a discussion later of whether only the first Base
is used or they are cumulative. We plan to use a conditional to only set the value if it is not already set. We also plan an attribute so the Base
file remains readable.
|
||
We could continue to tie new features to TFM. THe drawback is that users generally upgrade their TFM to keep their projects in support. While this avoids breaking changes/warnings occurring on tooling upgrade, it may either delay adopting desirable new features, or result in breaking changes/warnings happening when the user is just trying to stay in support. | ||
|
||
## Open questions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would this all work for other SDKs, say the iOS SDK?
For instance, given this project file:
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net10.0-ios</TargetFramework>
</PropertyGroup>
</Project>
How would this be in the new world? I'm guessing something like this:
<Project Base="10.0.100"
Type="iOS">
</Project>
- Where is the fact that
Type=iOS
is a valid type declared? How does this translate to letting the user know they have to install the iOS SDK if it's not already installed? - How can the iOS SDK provide their own set of defaults?
- How can these iOS defaults be versioned? The release schedule / versioning of the iOS SDK does not match the .NET SDK at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many similar scenarios will require multi-targeting, so it's a tad more complex. Especially because I think it's an error to declare both a TargetFramework
and TargetFrameworks
I added to the open question on multi-targeting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Type can supply its own defaults.
For versioned defaults, we discussed looking for a file that next to the Base that defines Type specific defaults. Like:
- Base10.0.100.props
- iOS10.0.100.props
But, that didn't make it into the proposal.
I need to review how workloads install as, at least for V1, we planned to restrict the location for Type and Base files to the .NET install repo. Hmmm.
```XML | ||
<Project Base="11.0.100" | ||
Type="Console"> | ||
<PropertyGroup Label="These properties added on upgrade from 10.0.100 to 11.0.100" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<PropertyGroup Label="These properties added on upgrade from 10.0.100 to 11.0.100" | |
<PropertyGroup Label="These properties added on upgrade from 10.0.100 to 11.0.100"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks everyone for your great comments and grammar fixes. I've added several Open Questions.
</Project> | ||
``` | ||
|
||
If there is a `Directory.Build.props` or `Import` file, it can contain the `Base` attribute, although it cannot contain the `Type` attribute. This is because the `Type` attribute specifies the SDK instructions for build evaluation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what you mean by retroactive. If you can clarify the scenario and which one you would intuit to win, that would be awesome.
There is a discussion later of whether only the first Base
is used or they are cumulative. We plan to use a conditional to only set the value if it is not already set. We also plan an attribute so the Base
file remains readable.
## Motivation | ||
[motivation]: #motivation | ||
|
||
The fundamental benefit of this effort is to speed up and remove friction for users that wish to adopt tooling improvements quickly, while also allowing users to delay incorporating change until it is not disruptive. To do this, we need to allow developers a friction free experience on upgrade, which means that with no more work than previously they have control over the upgrade process. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be largely up to teams, but anything that would be tied to TFM today would be a good candidate. I think the proposal needs a little more specifics on this, so good point. Behavior would also need to be tied to TFM if Base wasn't set ,or the Base would need to be automatically set when the user did not. Second seems easiest.
|
||
The fundamental benefit of this effort is to speed up and remove friction for users that wish to adopt tooling improvements quickly, while also allowing users to delay incorporating change until it is not disruptive. To do this, we need to allow developers a friction free experience on upgrade, which means that with no more work than previously they have control over the upgrade process. | ||
|
||
All breaking changes and new warning are introduced because we believe the positive impact outweighs the pain of the change. If a change is not breaking and does not add new warnings, we just add the feature and they are available when the user upgrades. If the change breaks everyone, we are unlikely to make the change. This proposal affects the occasional features that may break a few projects some of the time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users would be upgrading to get the new tools. We do not want to block that.
Are you thinking in terms of every change being a breaking change, and therefore if no breaking change no change?
That wasn't the intent, so more clarity is needed.
Some teams have a low tolerance for changes including new warnings or something like removing obsoleting CLI commands. Protecting teams from those kind of breaks is the intent.
|
||
Your approach to upgrading the `TargetFramework` of your project varies from wanting the newest features and performance improvements as quickly as possible to postponing as long as possible to ease the upgrade effort. | ||
|
||
### Motivation for `Base` attribute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaredpar the comment about testing is a good one.
@jkoritzinsky that would set the base value, but the intent was for it to also bring in a props file of values so behaviors could have discrete flags - think ImplicitUsings
. So, your basically correct, but it would a bit more the equivalent to:
<Project>
<Import Project="Console.props" /> <!-- Where Console.props contains the SDK and set the ProjectType, etc. -->
<Import Project="Base10.0.100.props" /> <!-- Where Base10.0.100 contains set the ProjectBase, etc -->
<PropertyGroup>
<TargetFramework>net10.0</TargetFramework>
</PropertyGroup>
<Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
</Project>
One of the reasons they are separate is because the base will often be set in Directory.Build.props or an import.
|
||
When a there is a breaking change or a new warning, there is a new MSBuild property (as today). For breaking changes or new warnings, the feature will be turned off if it `false` or the equivalent `disable`. The defaults specified by `Base` will turn on the features we strongly recommend. If you disagree with this for your project, you can reset the value to `false`. | ||
|
||
The key element of the `Base` design is that it is versioned and released with each SDK. This allows you to initially experiment with, and then adopt all the breaking changes and new warnings with a single value. The proposed structure of this version is the corresponding SDK version number, such as `10.0.100`, with a possible simplification to just `10`. The `Base` defaults include the `TargetFramework` so the user manages a single version number, in most cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is an open question at the moment. Do we want an explicit gesture like *
for floating?
<!-- 10.0.100 Base file expresses --> | ||
<Project> | ||
<PropertyGroup> | ||
<TargetFramework Condition=" '$(TargetFramework)' == '' ">net10</TargetFramework> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the simplest way of expressing this intent for clarity for folks reading the doc? Below we suggest the simplifying attribute for this common case.
|
||
We have received several requests for customization for `Base` files. Our initial response is to clarify our intent in `Base` files and how they differ from `Import` files. `Import` files are a long-standing feature of MSBuild and allow project's to incorporate MSBuild elements from another file. This feature is widely used in large repos and we have no plans for any changes to it. | ||
|
||
`Base` files are intended as a mechanism to give user's more control of the changes Microsoft to rolls out.Fundamentally, any user entered properties will overwrite any Microsoft supplied properties - you're in control of your build. With this context, we think that `Import` is a better solution for customizing at a repo level and do not plan to support custom `Base` files at this time. We are interested in hearing about any compelling scenarios. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. That is not a scenario that I had in mind. I have added it as an open question.
|
||
_The name `Type` is not final._ | ||
|
||
The `SDK` attribute on the `Project` element can be replaced by a `Type` attribute. It is not legal to have both a `Type` and an `SDK` attribute. If neither exists, the project will behave in the same manner as projects without a `SDK` attribute behave today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think this is any more than other features?
|
||
We could continue to tie new features to TFM. THe drawback is that users generally upgrade their TFM to keep their projects in support. While this avoids breaking changes/warnings occurring on tooling upgrade, it may either delay adopting desirable new features, or result in breaking changes/warnings happening when the user is just trying to stay in support. | ||
|
||
## Open questions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many similar scenarios will require multi-targeting, so it's a tad more complex. Especially because I think it's an error to declare both a TargetFramework
and TargetFrameworks
I added to the open question on multi-targeting.
|
||
We could continue to tie new features to TFM. THe drawback is that users generally upgrade their TFM to keep their projects in support. While this avoids breaking changes/warnings occurring on tooling upgrade, it may either delay adopting desirable new features, or result in breaking changes/warnings happening when the user is just trying to stay in support. | ||
|
||
## Open questions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Type can supply its own defaults.
For versioned defaults, we discussed looking for a file that next to the Base that defines Type specific defaults. Like:
- Base10.0.100.props
- iOS10.0.100.props
But, that didn't make it into the proposal.
I need to review how workloads install as, at least for V1, we planned to restrict the location for Type and Base files to the .NET install repo. Hmmm.
No description provided.