Skip to content
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

Add proposal for simplified output paths (version 2) #281

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions INDEX.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ Use update-index to regenerate it:
| 2021 | [TFM for .NET nanoFramework](accepted/2021/nano-framework-tfm/nano-framework-tfm.md) | [Immo Landwerth](https://github.com/terrajobst), [Laurent Ellerbach](https://github.com/Ellerbach), [José Simões](https://github.com/josesimoes) |
| 2021 | [Tracking Platform Dependencies](accepted/2021/platform-dependencies/platform-dependencies.md) | [Matt Thalman](https://github.com/mthalman) |
| 2022 | [.NET 7 Version Selection Improvements](accepted/2022/version-selection.md) | [Rich Lander](https://github.com/richlander) |
| 2023 | [Simplify .NET Output Paths](accepted/2023/simplify-output-paths.md) | [Daniel Plaisted](https://github.com/dsplaisted) |

## Drafts

Expand Down
140 changes: 140 additions & 0 deletions accepted/2023/simplify-output-paths.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
# Simplify .NET Output Paths

We'd like to make some improvements to project output paths for .NET 8.

Goals:

- Support output paths in a repo or solution-specific folder, rather than each project folder having its own output paths
- Provide simple and consistent output paths
- Avoid cases where one output path is nested inside of another one
- Avoid excessively deep directory heirarchies
- Allow output path customization when needed

## Motivation

Some developers would like to put all the output for their solution or repo [under a single folder](https://github.com/dotnet/sdk/issues/867). This would make it easier to delete all of the output, as MSBuild-based clean only deletes output for a single configuration, and doesn't delete everything from the intermediate output folder, as that's where it keeps track of what was produced by the build and hence what should be deleted in a clean. It would support building with [read only](https://github.com/dotnet/sdk/issues/887) [source trees](https://github.com/dotnet/designs/pull/281#discussion_r1072949097). It could also make it easier to set up CI and deployment.

The current output paths for .NET projects are rather complicated and inconsistent. The default output path for a .NET project includes folders for the `TargetFramework` and the `RuntimeIdentifier` (if it is set). Additionally, the publish output goes in a `publish` subfolder of the output folder. The resulting paths are as follows:

- `bin\<Configuration>\<TargetFramework>` - Build output with no `RuntimeIdentifier`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a crazy idea to think about, what about changing the build output directory structure to match that of nuget packages? bin\lib\net6.0\mylib.dll, bin\lib\netstandard2.0\mylib.dll, etc. Maybe only for projects with <Packable>true?

Today, it's very difficult for package authors to test their packages that aren't simple class libraries. If their package contains contentFiles/ or build/ or buildTransitive/, they have no choice but to pack their nupkg, then use a local package source to install the package via NuGet in a test project. And then clean the global package directory every time they want to make a change in the package.

Even if we can solve how to import a project's msbuild props/targets file via ProjectReference, the different directory structure of project bin\ vs package structure would make it difficult for package authors to write props/targets that work in both PackageReference and ProjectReference.

Hence, if we can more closely align the build output directory structure to NuGet packages, maybe it will make debugging NuGet packages via ProjectReference more feasible.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not crazy at all. Co-incidentally I thought the same thing...long before...

"If you're going to pack a project into NuGet or installer or App package, why not use the same layout as the final publishable output."

My proposal for the output path structure is just one of the many changes that accommodates and compliments this idea and beyond. I do have some ideas on how we do this but the foundation needs to be sensible, intuitive and generic enough. Like the way we set the default structure, the property names and the build context. Once you set the foundation, you should not come and break that when we might need something more in the future.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way, you could technically merge publish and bin into a single final target instead of having an additional build step to copy bin to publish. That leaves with the pack folder. So, essentially we'll end up with the 4 folders instead of 5 and we can keep everything under single folder.

  • Extensions folder (where NuGet restores and other project extensions go)
  • Interim folder (where intermediate and temporary files go)
  • Final folder (where the published output goes)
  • Publish/Package folder (where packages go)

You could also have a single or multiple target folder across projects and their build contexts. That is MY light at the end of this tunnel! 💯

- `bin\<Configuration>\<TargetFramework>\<RuntimeIdentifier>` - Build output with `RuntimeIdentifier`
- `bin\<Configuration>\<TargetFramework>\publish` - Publish output with no `RuntimeIdentifier`
- `bin\<Configuration>\<TargetFramework>\<RuntimeIdentifier>\publish` - Publish output with `RuntimeIdentifier`

## Proposed behavior

We will introduce a new output path format, called the "artifacts" output format after the default folder name it will use. By convention, the artifacts folder will be located at the solution or repo level rather within each project folder.

The recommended way to opt in to the new output path format will be to set the `UseArtifactsOutput` property to `true` in a `Directory.Build.props` file.

The artifacts folder will by default be named `.artifacts`. The convention we will use to determine which folder the artifacts folder should go in will be:

- If a `Directory.Build.props` file was used in the build, put the artifacts folder in the same folder where `Directory.Build.props` was found, which will typically be at the root of a repo. Since MSBuild already probes for `Directory.Build.props`, we should be able to use this folder for the artifacts with minimal perf impact
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know how common it is to use Directory.Build.props outside the repo folder?

E.g. using it for "global" config since MSBuild will search up to the filesystem root: https://learn.microsoft.com/en-us/visualstudio/msbuild/customize-your-build?view=vs-2022#search-scope

Copy link
Member

@akoeplinger akoeplinger Mar 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also the documented pattern of importing the next Directory.Build.props file above this one (https://learn.microsoft.com/en-us/visualstudio/msbuild/customize-your-build?view=vs-2022#use-case-multi-level-merging) which we use in dotnet/runtime for example, if there are multiple files in the hierarchy which one would we pick?

Copy link

@commonsensesoftware commonsensesoftware Mar 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...Directory.Build.props ... will typically be at the root of a repo.

This is a false assumption. Many repos use nested Directory.Build.props and/or Directory.Build.targets. Other variations include putting one under the root of /src and a different one under the root of /test. The same can be said for a *.sln, if there even is one.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Determining the root of the codebase and build is definitely challenging. Directory.Build.props and Directory.Build.targets can be outside the root, but that should not be allowed. That opens things up to the same types of supply chain attacks that are possible with the nuget.config behavior.

The .git folder is an obvious choice, but the developer may not be using Git. LICENSE or LICENCE.txt are not required. There could be none or multiple README.md files at different levels. If there were a known set of root-level conventions that covers the 80%, but can be configured to cover the other 20%, that would be preferable. Most, if not all, .*/ folders are at the root as well.

There may very well be no answer to this issue, however, it should never be assumed or intentionally allowed to import something outside the root of the repo IMHO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are also monorepo builds, where a single repository contains multiple codebases that build completely independently, and shouldn't have a co-mingled artifacts folder.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's correct that there's no perfect way to find the root of the repo. The goal is for UseArtifactsPath to work well for many repos, and the ones that it doesn't work well on can set the ArtifactsPath property.

This aligns with the philosophy that the .NET SDK tries to follow of sensible defaults that can be overridden if necessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed this now... This is a very large change as the simple fact of adding a props file will now affect the build? I now have to add a props file and set a msbuild property.

One thing I do often as mentioned before is to have levels of props. I have a props in the root, in the tests and sometimes if test are grouped into sub folders.

Why would the props file be the cause of the change, I also vote that the file should not be an indicator but rather opt in via the property and not the presence of the props.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still have to opt in by setting the property. If you do set UseArtifactsOutput to true, then the default is to look for the folder where Directory.Build.props came from, which is where it is recommended to set the property in the first place.

If that doesn't work for your repo, then you can set the ArtifactsPath property instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the artifacts path now opt in? I thought it was the default for new projects?

Regardless, my point was that no matter the situation, adding a props will change where artifacts go.

- If a `Directory.Build.props` file was not found, then put the artifacts folder in the project folder.

To override the default artifacts folder path, the `ArtifactsPath` property can be used. If `ArtifactsPath` is set, then `UseArtifactsOutput` will default to `true` and it won't be necessary to set both properties. `ArtifactsPath` should normally be set in a `Directory.Build.props` file. For example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need these two separate properties? Or could we only have ArtifactsPath?

Maybe that would also make the discussion around the default folder name easier, if we use setting the
ArtifactsPath as the opt-in gesture then the user can provide the preferred name (we could still recommend some default name in docs of course).


```xml
<Project>
<PropertyGroup>
<ArtifactsPath>$(MSBuildThisFileDirectory)\output</ArtifactsPath>
</PropertyGroup>
</Project>
```

Under the artifacts folder will be nested folders for the type of the output, the project name, and the pivots, ie `<ArtifactsPath>\<Type of Output>\<Project Name>\<Pivots>`.

- Type of Output - This will be a value such as `bin`, `publish`, `obj`, or `package`. Projects will be able to override this value, for example to separate shipping and non-shipping artifacts into different folders
- `bin` will be the folder for the normal build output. Projects can override this with `ArtifactsBinOutputName`
- `publish` will be the folder for the publish output. Projects can override this with `ArtifactsPublishOutputName`
- `package` will be the folder for the package output, where .nupkg files that are created when packing the project are placed. (Or should the folder name be `packages`? Feedback welcome.) Projects can override this with `ArtifactsPackageOutputName`
- `obj` will be the folder for the intermediate output.
- The Project Name. This will ensure that each project has a separate output folder. By default this will be `$(MSBuildProjectName)`, but projects can override this with the `ArtifactsProjectName` property. If `ArtifactsPath` was not explicitly specified, and a `Directory.Build.props` was not found, then the artifacts folder will already be inside the project folder, and an additional Project Name folder will *not* be included in the output paths. Additionally, for package output, this folder won't be included in the path, so that all the `.nupkg` files that are built can be in a single folder.
- Pivots - This is used to distinguish between builds of a project for different configurations, target frameworks, runtime identifiers, or other values. It can be specified with `ArtifactsPivots`. By default, the pivot will include:
- The `Configuration` value (lower-cased, using the invariant culture)
- The `TargetFramework` if the project is multi-targeted
- The `RuntimeIdentifier`, if it was explicitly set (either in the project file or on the command line). The `RuntimeIdentifier` won't be appended if it was automatically set by the SDK, for example because `SelfContained` was set

If multiple pivot elements are used, they will be joined by the underscore (`_`) character. For example, a multi-targeted project with the RuntimeIdentifer specified could have a pivot of `debug_net8.0-windows10.0.19041.0_win-x64`. For the `package` output type, the pivots will only be the `Configuration`, and other values won't be included in the path

Some examples:

- `.artifacts\bin\debug` - The build output path for a simple project when you run `dotnet build`
drewnoakes marked this conversation as resolved.
Show resolved Hide resolved
- `.artifacts\obj\debug` - The intermediate output path for a simple project when you run `dotnet build`
- `.artifacts\bin\MyApp\debug_net8.0` - The build output path for the `net8.0` build of a multi-targeted project
- `.artifacts\publish\MyApp\release_linux-x64` - The publish path for a simple app when publishing for `linux-x64`
- `.artifacts\package\release` - The folder where the release .nupkg will be created for a project

## Customizing the output paths

The `Directory.Build.props` and `Directory.Build.targets` files can be used to [customize builds](https://learn.microsoft.com/visualstudio/msbuild/customize-your-build#directorybuildprops-and-directorybuildtargets) with logic that will apply to multiple projects. However, they do not work very well to customize the output path. This is because custom output path logic usually depends on the contents of the project file, for example the `TargetFramework` value or a custom property such as `IsShipping`. `Directory.Build.props` is imported before the body of the project file, so it can't read those properties. On the other hand, `Directory.Build.targets` is imported after common targets, so it is too late to correctly set the output paths.

There is already a `BeforeTargetFrameworkInferenceTargets` property which can be set to import a targets file after the project file is evaluated but before the `TargetFramework` is parsed. Custom output path logic may depend on the target framework, and it is better to write this logic in terms of the properties which are parsed out of the `TargetFramework` such as `TargetFrameworkIdentifier` and `TargetFrameworkVersion`. Because of this, we will add support for an `AfterTargetFrameworkInferenceTargets` property which can be used to inject a target file that will be imported after `TargetFramework` parsing.

As an example of how to use this, a project that wants to put the output for "Shipping" projects in a separate folder could set the `IsShipping` property to true in each shipping project, and put the following in `Directory.Build.props`:

```xml
<Project>
<PropertyGroup>
<UseArtifactsOutput>true</UseArtifactsOutput>
<AfterTargetFrameworkInferenceTargets>$(MSBuildThisFileDirectory)\Directory.AfterTargetFrameworkInference.targets</AfterTargetFrameworkInferenceTargets>
</PropertyGroup>
</Project>
```

The `Directory.AfterTargetFrameworkInference.targets` would include the following:

```xml
<Project>
<PropertyGroup Condition="'$(IsShipping)' == 'true'">
<ArtifactsOutputName>Shipping\bin</ArtifactsOutputName>
<ArtifactsPublishOutputName>Shipping\publish</ArtifactsPublishOutputName>
<ArtifactsPackageOutputName>Shipping\package</ArtifactsPackageOutputName>
</PropertyGroup>
</Project>
```

## Specifying the output path via command line

The .NET CLI supports an `--output` parameter to specify the output path for various commands. This [does not work well](https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/7.0/solution-level-output-no-longer-valid) for solutions, since it results in multiple projects building to the same folder. We will add an `--artifacts-path` parameter that sets the `ArtifactsPath` property. Because each project will be built into a separate subfolder, this will be a safer option to use for solution builds.

## Changing the default output path format

We are proposing this new output format because they represent an improvement over the existing behavior. If they do represent an improvement, then we would like to change the default to use the new behavior, so that new projects and new users can get the improved behavior without having to know to opt in. However, there are several obstacles to changing the default.

First of all, we don't want to affect existing projects that haven't opted in to the new behavior. To do otherwise would be a massive breaking change, as many CI setups, build scripts, custom build logic, etc. all depend on knowing where the output of a project is and would break if it changes.

Often what we do in situations like this is to only make the new behavior the default for a new target framework. For example, projects that hadn't explicitly opted in or out would only get the new behavior if they targeted .NET 8 or higher. This would work for single-targeted projects, but if a project multi-targeted to .NET 7 and .NET 8, then it could be very confusing if the output for .NET 7 was in the traditional `bin` folder, but the output for .NET 8 was in an `.artifacts` folder that wasn't even in the project folder but was at the solution level.

Ideally in that case we might want to use the new output path format by default for all target frameworks if any of them are .NET 8 or greater. Unfortunately this is not possible, as the output paths need to be determined during MSBuild evaluation, and at that point it's not possible to query the build of another target framework to see if it targets a given version of .NET or higher.

So for multi-targeted projects, we'd be left with these options:

- Default to the new output paths - too breaking
- Use different output path formats for different target frameworks - probably too confusing
- Default to the old output paths

Another issue with defaulting to the new output path format is that it can produce output paths that are longer, which would make it more likely for projects to run up against path length limitations. .NET Maui projects in particular have hit this limitation in the past with nested Java builds in the intermediate output path, and has taken steps to shorten their paths.

## Considerations

### Using `.artifacts` for the folder name

The default `.artifacts` folder name was chosen for a few reasons:

- Folders starting with `.` are already ignored by the default item globs for .NET SDK projects. This is important so that if a project switches back and forth between output formats, the output path (and especially the intermediate output path, where C# source code files are generated) won't be included in the globs.
- The `.` prefix sorts the folder separately from the rest of the source folders

There is also at least one drawback:

- Folders starting with `.` are hidden in some contexts (MacOS finder for example). This will make it harder to find the output or run the app.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a big deal to me. Do we really want build output hidden by default on macOS and Linux?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, but probably ideally we wouldn't want them hidden. However, if we go with something non-hidden like artifacts, then we will have to add that to the list of folders that are ignored in the default item globs for ALL projects (so that you don't get weird intermediate file errors when you switch back and forth between output path formats). That would introduce a higher breaking change risk (ie any project with source in an artifacts folder would now be broken).

Using a folder starting with . means that the default item globs already ignore it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also very uneasy about the .artifacts change, it sounds like something that will be annoying for Linux/macOS users but not for Windows users, I don't think that is a good precedent.

Would it be hard to detect whether we have an existing artifacts folder and issue a warning in that case? Maybe that should be a standard warning anyway if you have globs matching ArtifactsPath.

Copy link
Member

@agocke agocke Apr 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dotfiles are a wildly inappropriate use case on Unix. This would be extremely bad for Unix users. The whole point of dotfiles is that tooling goes out of their way to hide dotfiles and make them difficult to select, which is the exact opposite of what you want with build artifacts.

This signals to me that there's more work to do here. I think the SDK authors should use and review some popular Unix build tools like make, CMake, go build, npm run build, cargo build, etc. We should review whether our output is in line with these tools, or describe how others do things differently (and why we don't want to go that route).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we condition the dot based on the OS? E.g., if Windows -> .artifacts, else -> artifacts?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the really interesting thing is -- what if we named obj as .obj and stuck it inside bin? I suspect most people who have scripts today that reference obj aren't actually fetching stuff from there (they shouldn't be), they're probably just deleting it to clean up.

Is that functionally any different from the current state of the feature in Preview 3 though?

$ ls .artifacts
bin  obj  package  publish

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting .obj inside bin means you can just delete the bin directory to wipe all the build assets.

That is the critical part for me. A single top level dir for all our build assets. It's valuable enough that we've pushed that pattern into the entire .NET build stack. Want to give that goodness to our customers.

I'm starting to come around to the idea of the top level dir isn't dot prefixed but we consider dot prefixing subdirectories which are build implementation details. Essentially places the 99% customer should not be peaking into.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything we do to nest bin and/or obj underneath another directory is consuming precious characters from MAX_PATH on windows which is historically a real pain for Xamarin, and so now .NET MAUI apps. There's been many changes in Android tasks/targets specifically for example to shorten the names of things we have less control over in the native toolchain/ecosystem part of the stack (eg: hashing file/folder names and building lookup tables in obj).

I'm more supportive of the shortest name possible for this reason alone. I'd take the 6 character savings that bin/.obj affords over .artifacts/obj.

And while I understand that these paths are all customizable, what I'd like to avoid is needing to change our templates for dotnet new maui projects to ship with opinionated differences over the 'defaults' for other .NET project types just to ensure dotnet build still works out of the box on most developers' systems.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we keep the dot in the directory name, we should also set the Hidden attribute on Windows because all the arguments around dot in the name apply to the Hidden attribute.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the . folder either. One good thing about the current situation is that bin is visible and very noticeable. This is a nice property when diving into it is required. Making the build output folder invisible is adding more "magic" to the stack.

And this is no good especially for beginners. Imagine having to explain things involving ls -a in the correct folder instead of just "go to the artifacts folder".


### Finding the output path from scripts

Today, the output path is likely hardcoded in many places that interact with MSBuild (build scripts, CI pipelines, etc.). If those are going to need to be updated, it would be better if they could be updated in a way that makes them resilient to output path changes. To do this, we should provide a way for a script to call in to MSBuild and get the output path value. This will be a separate design proposal, right now we have it tracked with the following work item: https://github.com/dotnet/msbuild/issues/3911

## Looking for a .sln file in the convention

If we ever enable the new output path format by default, we would want it to work for projects without a Directory.Build.props file. In that case we would want to update the convention so that if a Directory.Build.props file isn't found, it will look for a folder with a `.sln` file in it, walking up the directory tree from the current project folder to the root. When a `.sln` file is found, the artifacts folder would be put in that same directory.

To implement this, we would probably need an additional intrinsic MSBuild function, as the `GetDirectoryNameOfFileAbove` function doesn't support wildcards for the file name