Add proposal to simplify output paths for .NET 8 #278
Conversation
dsplaisted
requested review from
richlander,
mhutch,
terrajobst,
clairernovotny and
a team
|
|
||
| ### Breaking changes | ||
|
|
||
| These changes could break things such as scripts that copy the output of the build or custom MSBuild logic that hard-codes these paths. Tieing these changes to the project TargetFramework ensures that these breaks will be encountered when the project is modified to target a new TargetFramework, not when updating to a new version of the .NET SDK. |
There was a problem hiding this comment.
Is there a way, or can we add a way, to get the paths, so scripts can programatically get these directories rather than hardcode them? For example $publishPath = & dotnet publish -f net8.0 -r linux-x64 --output-path? Is there already a target that we can use, something like $publishPath = & dotnet msbuild -t:GetPublishPath -p:TargetFramework=net8.0 -p:RuntimeIdentifer=linux-x64?
If msbuild doesn't already have a feature to output the value of a single property after an evaluation (and running a target?), maybe that would be a useful feature? $publishPath = & dotnet msbuild --output-property PublishDir -p:TargetFramework=net8.0 -p:RuntimeIdentifer=linux-x64
There was a problem hiding this comment.
I don't think there's a direct way for MSBuild to return a value to a script that calls it.
Targets can return properties or items, and this is used by Visual Studio to get information from the build. Maybe it would be an interesting feature to support some way of doing that from the command line (that would be separate from this proposal though). @rainersigwald, what do you think?
There was a problem hiding this comment.
dotnet/msbuild#3911 tracks some options there, which I'm not opposed to but I don't see a nice elegant solution for it at the moment, so it hasn't gotten a ton of traction. Feedback welcome!
| - `bin\<Configuration>` - Build output path for single targeted project | ||
| - `bin\<Configuration>\<TargetFramework>` - Build output for multi-targeted project | ||
| - `bin\publish` - Publish output for single targeted project | ||
| - `bin\publish\<TargetFramework>` - Publish output for multi-targeted project |
There was a problem hiding this comment.
Having dotnet build use a Configuration sub-folder while dotnet publish doesn't seems like a bizarre inconsistency to me. I have to imagine many will find that confusing.
I almost feel like a new pub\<Configuration>[\<TargetFramework>] scheme would make more sense than this. It would mean a .gitignore update, but it would achieve the goal of avoiding deeply nested output folders, and it would more clearly separate publish output from build output.
There was a problem hiding this comment.
I agree, another point is that these will overwrite each other: dotnet publish -c MyRelease and dotnet publish -c YourRelease - I believe it's pretty well known that if you want two independent builds, you use two different configurations, but that won't work anymore.
There was a problem hiding this comment.
I've clarified that the publish path would only change for the Release configuration, and added a section explaining the motivation behind putting publish at the same level as Configuration.
| - `bin\<Configuration>` - Build output path for single targeted project | ||
| - `bin\<Configuration>\<TargetFramework>` - Build output for multi-targeted project | ||
| - `bin\publish` - Publish output for single targeted project | ||
| - `bin\publish\<TargetFramework>` - Publish output for multi-targeted project |
There was a problem hiding this comment.
If a RID is given the proposed location should be included in this proposal too.
There was a problem hiding this comment.
The proposal is that the RID isn't included in the output path even if specified unless AppendRuntimeIdentifierToOutputPath. Per rolf's comment this would probably automatically be set to true for iOS apps.
| - `AppendRuntimeIdentifierToOutputPath` will default to true only when targeting .NET 7 or lower. | ||
| - To be discussed: Should we also default it to true if `RuntimeIdentifiers` is set? | ||
|
|
||
| For publish, we would change the publish output path only when targeting .NET 8 or higher and when the Configuration is set to Release (which we [plan to make the default](https://github.com/dotnet/sdk/issues/10357#issuecomment-1208564539) for .NET 8). In that case we would set the publish output path to `$(BaseOutputPath)\publish` (by default `bin\publish`), and append the `TargetFramework` and `RuntimeIdentifier` to the path based on `AppendTargetFrameworkToOutputPath` and `AppendRuntimeIdentifierToOutputPath`. |
There was a problem hiding this comment.
change the publish output path only when ... Configuration is set to Release
So for Debug the same deep nesting as now would be done, thus having a inconsistent behavior in locations.
I'd publish with release always (so disable publish for Debug). At least emit a warning if published as Debug, but write to the same consistent publish-location.
|
|
||
| ### Capitalization | ||
|
|
||
| Should the default publish output path be `bin\publish` or `bin\Publish`? Uppercase matches the Configuration values of `Debug` and `Release`, which the publish folder will be a sibling of. However, the current publish folder capitalization is lowercase. |
There was a problem hiding this comment.
Keep it lowercase, as it's different from a build-config (Debug, Release) so to indicate that difference here too.
There was a problem hiding this comment.
We have been discussing making Publish be a build-config instead of what it currently is. But I like the lowercase p since it matches the rest of the folder conventions.
|
|
||
| ## Proposed behavior | ||
|
|
||
| Projects targeting .NET 8 and higher will by default not include the `TargetFramework` in the output path unless the project is multi-targeted. Similarly, the `RuntimeIdentifier` will not by default be part of the output path. Finally, the publish output path will be `bin\publish` by default instead of a `publish` subfolder inside of the output path. |
There was a problem hiding this comment.
Similarly, the
RuntimeIdentifierwill not by default be part of the output path.
Would it make sense to skip the RuntimeIdentifier by default only if the project is published for native SDK RID (ie only if the project is not cross-compiled)?
There was a problem hiding this comment.
Perhaps, but I think we should try to avoid having one publish folder inside of another. I think in many cases you're going to be copying the entire publish folder, so if the cross-compiled publish outputs are in subfolders of the native RID publish folder, then when you go to copy that folder you'll also accidentally get all of the other RIDs you published for.
I think maybe it's better for projects that want to go back and forth between publishing different RIDs to set the AppendRuntimeIdentifierToOutputPath property to true. It sounds like the iOS SDK would probably want to do this by default, for example.
|
|
||
| ## Proposed behavior | ||
|
|
||
| Projects targeting .NET 8 and higher will by default not include the `TargetFramework` in the output path unless the project is multi-targeted. Similarly, the `RuntimeIdentifier` will not by default be part of the output path. Finally, the publish output path will be `bin\publish` by default instead of a `publish` subfolder inside of the output path. |
There was a problem hiding this comment.
For mobile TFMs (-ios, -tvos) we need the RuntimeIdentifier in the path, because this is very common:
- Build & test in the simulator (using a simulator RID)
- At some pointer later, want to test on device (using a device RID).
Simulator and device builds are quite different, so not much can be reused between the RIDs (it would be a rather big task to figure out what can safely be re-used and make sure nothing else is). This means that changing between device and simulator when they build into the same location would have to invalidate the entire build (and then when you switch back the same thing would happen).
Another problem is that for multi-rid builds (say we're building for both osx-arm64 and osx-x64), we build first for one RID, then the other RID, and at the end we merge the results into a universal app bundle. This wouldn't work if both RIDs built into the same directory.
So we'd probably have to set AppendRuntimeIdentifierToOutputPath to true always for any SDK for Apple platforms.
Another solution would be if the RID was only excluded from the output path of the project didn't specify a RuntimeIdentifier/RuntimeIdentifiers.
There was a problem hiding this comment.
I think that with this design, the iOS SDK would set the AppendRuntimeIdentifierToOutputPath to true.
In the future, I hope we can support multi-targeting over RuntimeIdentifiers. For example you could set TargetFrameworks to something like net9.0-macos/osx-x64;net9.0-macos/osx-arm64 and it would build both for you: dotnet/sdk#9363 (comment)
There was a problem hiding this comment.
Maybe I am just being overly negative here, but if a whole set of platforms has to set AppendRuntimeIdentifierToOutputPath=true by default anyway, while others don't, it seems like this proposal in its current form adds very little value relative to the inconsistency and confusion it would cause for users.
Once again, I would point to the pub\... scheme I proposed above. It achieves the goals this proposal sets out to solve without any of that inconsistency and confusion.
|
|
||
| Some factors influencing this: | ||
|
|
||
| - It is rarely correct to publish with the Debug configuration, as publish is used to create artifacts that are deployed. |
There was a problem hiding this comment.
I would just note here that dotnet publish -c Debug is a perfectly reasonable thing to do if you're providing nightly builds for a project targeted at end users.
In fact, publishing such builds in Release is more likely to be the incorrect thing, as any diagnostic information gained from testers is strictly less helpful in that configuration.
I don't think you can just turn Publish into a configuration without accounting for this.
|
Thanks for the feedback on this folks. Based on feedback from discussions of this, I've submitted a new proposal for simplified output paths here: #281 |
No description provided.