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

Conversation

dsplaisted
Copy link
Member

@dsplaisted dsplaisted commented Nov 18, 2022

Based on feedback from a discussion of #278, this is another attempt at simplifying output paths which should be more consistent.

Rendered view

@gfoidl
Copy link
Member

gfoidl commented Nov 19, 2022

This looks good 👍🏻

accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
Copy link

@alexrp alexrp left a comment

Choose a reason for hiding this comment

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

Big improvement over the previous proposal. Other than the minor point about renaming obj, I see no issues with this.

accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
Copy link
Member

@baronfel baronfel left a comment

Choose a reason for hiding this comment

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

Really liking this iteration! I'll try to dig into some of the open questions around how other ecosystem layout their artifacts.

accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
dsplaisted and others added 2 commits November 22, 2022 05:21
Co-authored-by: Rolf Bjarne Kvinge <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
Co-authored-by: Chet Husk <[email protected]>
@dsplaisted
Copy link
Member Author

Hi folks! I've made another update to the proposal:

  • Switch from using bin to artifacts
  • Put configurations in lower case in new output path format
  • Add details about complications with intermediate output path and multi-targeted builds

Have a look and keep the feedback coming. Thanks!

@jkotas
Copy link
Member

jkotas commented Nov 23, 2022

Switch from using bin to artifacts

This will be a major breaking change and generate work for every other project out there. We should think twice whether this cosmetic change is really worth it.


If a project is multi-targeted to `net7.0` and `net8.0` and doesn't specify the output path format, then different formats will be used for the output for the different target frameworks. The .NET 7 build output will be in `bin\Debug\net7.0`, while the .NET 8 output will be in `artifacts\bin\debug_net8.0`.

To prevent the output from one target framework to be globbed as part of the inputs to another target framework, both the `bin` and `artifacts` folder will need to be excluded from the default item globs (via the `DefaultItemExcludes` property). This means that even projects that target .NET 7 or lower could be impacted by breaking changes if they currently have source files or other assets in an `artifacts` folder.
Copy link
Member

Choose a reason for hiding this comment

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

Will this require an update to .NET 7 to not add default items from the artifacts folder?

A .NET 8 build will generate source files and put them in artifacts/intermediates, and if the project also targets .NET 7, those files will be picked up as default items unless the artifacts folder is excluded.

@dsplaisted
Copy link
Member Author

Switch from using bin to artifacts

This will be a major breaking change and generate work for every other project out there. We should think twice whether this cosmetic change is really worth it.

@jkotas Are you referring to any change we make to the output path format, or specifically this version of the proposal which would switch to using an artifacts path?

I'm not sure that switching to an artifacts path is significantly more breaking than the previous versions of the proposal which changed the output path format but still put everything under bin.

@jkotas
Copy link
Member

jkotas commented Nov 24, 2022

@jkotas Are you referring to any change we make to the output path format, or specifically this version of the proposal which would switch to using an artifacts path?

I feel that the bin and obj directories are hardcoded and will need to be updated in more places than the rest. I agree with you that the rest is also significantly breaking.

For reference, I have spent almost a day earlier this week on debugging and working around a much smaller breaking change in the directory layout in .NET 7.0.200 SDK (dotnet/sdk#29177). I expect that the cost to update for the breaking change proposed here in any larger project is going to be very non-trivial.

@nkolev92
Copy link

It'd be great to have a list of actual properties that are being changed at some point.
I understand that all of those might not be known yet, but it'd be great to have them documented.

accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
accepted/2022/simplify-output-paths.md Outdated Show resolved Hide resolved
@Nirmal4G
Copy link

Building software as a concept always needs two contexts at minimum, temporary one which can be discarded or overwritten and a permanent one which can be used, shared or published. That's why I proposed two root paths, one for building/testing and one for publishing/staging the final artifacts.

Initial Proposals:


The original structure I proposed is in dotnet/msbuild#6105. This has the following changes...

Summary

  • Introduce BuildDir as the one-stop directory for all things build (e.g., bin, obj, etc...).

  • Move MSBuildProjectExtensionsPath to $(BuildDir)\ext which frees BaseIntermediateOutputPath from common props import.

  • Promote PublishDir to root as one-stop directory for all things publish ($(PackageOutputPath) can be $(PublishDir)\$(Configuration)).

  • Use BuildDirName similar to PublishDirName to change the name of the build directory.


The pros to this structure are...

  1. The property name is intuitive and similar to existing ones like PublishDir and OutDir.

  2. Just as mentioned before, we can bring in all build tools folders under a common build directory. We can also make it as a symbolic link or junction to a common publish directory. We can also point it to a server share.

  3. We can have a custom folder for our custom workflow without needing to worry about collision. Since, we can append build parameters like TargetFramework to build directory itself which I observed many like to do.

  4. For Docker builds, caching NuGet restore is an important optimization. This gives us a way to separate those Assets from obj folder into a new one ext under build and thus can be cached in a single command step.

  5. We can also bring IntDir to close the gap from C++ project system. This is currently an issue in MSBuild.

  6. The changes can go into MSBuild which can be applied to all projects if they choose to and still provide previous structure for compatibility.

The only con against this is having two directories instead of one. But I'm sure you can solve this with a simple property switch to make the publish come under build if users want it that way.

Ultimately, I recommend this way if we want to have a structure that works for all projects and still have the ability to customize it when required.

@ChristopherHaws
Copy link

ChristopherHaws commented Apr 11, 2023

@alexrp I think that since you are not following the standard naming convention it would make sense for you to configure ArtifactsProjectName in your projects directly (or in Directory.Build.props). I also don't follow the standard conventions, but I don't have duplicate project names in my solution. I don't know whether this should be something handled out of the box or not since the default behavior of .NET is that the PackageId is based on the project name which should be unique (you can't publish two packages with the same PackageId to nuget).

@dsplaisted Would it make sense for the artifact folder name to default to the PackageId property if it exists instead since this property is defined as being unique? This property is not used by all project types so there should probably be fallback logic such as ArtifactsProjectName -> PackageId -> AssemblyName -> ProjectName?

@alexrp
Copy link

alexrp commented Apr 11, 2023

I may be breaking with common .NET convention, but:

This would also be more in line with how many other build tools work, incidentally.

This point still stands. If you look at virtually any build tool that supports out-of-tree builds, they all replicate the relative project directory structure in the build output directory. If memory serves, this is true of Autotools, Meson, and CMake just off the top of my head. So I think it's reasonable for there to at least be a property in the SDK that I can flip to get this behavior, rather than having to roll it myself.

Would it make sense for the artifact folder name to default to the PackageId property if it exists instead since this property is defined as being unique? This property is not used by all project types so there should probably be fallback logic such as ArtifactsProjectName -> PackageId -> AssemblyName -> ProjectName?

The issue is that the project name is added to the artifacts path in Sdk.props. That's far too early for anything like AssemblyName or PackageId to be usable. 🙁

@eli-schwartz
Copy link

This point still stands. If you look at virtually any build tool that supports out-of-tree builds, they all replicate the relative project directory structure in the build output directory. If memory serves, this is true of Autotools, Meson, and CMake just off the top of my head. So I think it's reasonable for there to at least be a property in the SDK that I can flip to get this behavior, rather than having to roll it myself.

It's actually crucial to do this in many cases, because often, you'll have data that isn't the output of a compiler, and must be hierarchical in order to function (usually but not always at deploy time). Languages such as python are a good example of this. When this is the case, you need hierarchical build outputs as well, to:

  • support running it live from the build directory for testing
  • avoid conflicting on files with the same name, such as __init__.py or generally anything called "utils" or "config" or similar names that may appear multiple times and be identified by the directory they reside in -- these cannot all write to the same output file
  • function correctly at build time, for e.g. C/C++ headers that are built and used internally as well as installed, and need to follow a particular #include subdirectory structure

For meson in particular, we "support" both a flat and a mirror layout, but the flat one is considered a failed experiment, is full of bugs, and has been deprecated (we will no longer accept bugfixes for it).

There are a lot of routine things people do in a build system that depend on exact structure, and generally very little reason not to respect this by default.

@alexrp
Copy link

alexrp commented Apr 11, 2023

The issue is that the project name is added to the artifacts path in Sdk.props. That's far too early for anything like AssemblyName or PackageId to be usable. 🙁

Took a closer look at the code and it seems the logic is repeated in targets/Microsoft.NET.DefaultOutputPaths.targets. So @ChristopherHaws's fallback suggestion might be workable after all. 🤔

@tannergooding
Copy link
Member

I responded via the survey as well, but wanted to share some thoughts here....

.artifacts is a con. .name in general represents a "hidden folder" on many systems and your build artifacts are anything but. They are frequently a set of files you explicitly want to be able to browse and trivially access, unlike say your .git folder.

The _ in <Pivots>, rather than just separating as folders, is a con as this greatly hinders auto-complete on some systems. That is, some systems allow you to specify a short prefix and then hit tab 2+ times to cycle through the available options. Other systems require you to specify the shortest non-conflicting prefix before tab completion works. If there are conflicts it will explicitly list the options but still expects you to type it out. Thus, debug_net8.0 in a multi-tfm setup requires you to type 10 characters before tab-complete works. Where-as debug/net8.0 allows you to type d<tab>net8<tab> and get a much faster set of completion in the worst case.

For my own setup, I have <repoRoot>/artifacts/*, where * is:

  • bin
  • obj
  • pub - not publish, just for consistency
  • dotnet - optional folder used to contain downloaded .NET SDK (via dotnet-install) for CI
  • log - binlog output folder
  • pkg - *.nupkg output folder
  • tst - *.trx output folder

For pkg, log, and tst there is no need to differentiate "per project" since each project only has a single output, so I simply have the */<config>/*.* files. For example, artifacts/pkg/Debug/TerraFX.nupkg

For bin, obj, and pub I mirror the general folder structure and thus have:

  • samples
  • sources
  • tests

Under these folders there is then */<project>/<config>/<tfm>/*.*, so for example, artifacts/bin/samples/TerraFX.DirectX/Debug/net8.0/TerraFX.DirectX.dll

This setup ensures no overall conflicts, helps separate different types of shipping vs non-shipping files, makes it easy for artifact collection for different publishing steps/scenarios, and makes it trivial for users to find the relevant output files for manual copying, testing, or other considerations.

@hamarb123
Copy link

Haven't tried it out in depth yet, but I also think that the . should not be in the artifacts folder name (by default at least), the default should just be artifacts.
I'm pretty sure I'll have to change some of my msbuild things to get this to work for some of my projects 😆

@alexrp
Copy link

alexrp commented Apr 12, 2023

A small thing: It would be nice if there was an ArtifactsDirectoryName property that would let the user easily override just the hardcoded .artifacts name here:

@akoeplinger
Copy link
Member

A small thing: It would be nice if there was an ArtifactsDirectoryName property that would let the user easily override just the hardcoded .artifacts name here:

We could probably still use ArtifactsPath for that if we check whether it is already a fully-qualified path or not.

@craigktreasure
Copy link

Any thoughts on the output of test results? They're still written to a TestResults folder next to the csproj. I'd personally like to see them written inside the .artifacts folder somewhere. Perhaps something like <ArtifactsPath>\test-results\<Project Name>\<Pivots>.

@Kwpolska
Copy link

I don't have any data to back this up, but I think it is pretty common knowledge for macOS power users (including developers) that command+shift+. shows the hidden files. I use it as a simple way to switch between contexts. If I am focusing on coding, I keep everything hidden, but when I am working on infra changes, I show hidden files.

Not everyone who uses .NET is a power user of their operating system. macOS also makes this harder than it needs to be by not having a way to toggle hidden files without using the keyboard (unless something changed recently).

This pattern is already used by many other tools for similar purposes, for example .config/dotnet-tools.json, .github, .editorconfig, .vs, .vscode, .git, .gitignore, .gitattributes, .yarn, .idea, etc, etc.

Many of those (.vs, .git, .yarn, .idea) have no user-serviceable parts inside. While obj matches this description, bin and especially publish do not — they are places that developers need to access regularly. The artifacts folder should not have a dot and should not be hidden on Windows.

@dsplaisted
Copy link
Member Author

Perhaps an IncludeProjectDirectoryInArtifactsPath property could be the answer? So if I have src/language/core/core.csproj and src/runtime/core/core.csproj, these would go under .artifacts/bin/src/language/core/... and .artifacts/bin/src/runtime/core/..., respectively. (This would also be more in line with how many other build tools work, incidentally.)

@alexrp You can do this with the following Directory.Build.props file:

<Project>
  <PropertyGroup>
    <UseArtifactsOutput>true</UseArtifactsOutput> 
    <ArtifactsProjectName>$([System.IO.Path]::GetRelativePath('$(MSBuildThisFileDirectory)','$(MSBuildProjectDirectory)'))</ArtifactsProjectName>
  </PropertyGroup>
</Project>

I'm not sure if we want to add a property for this, since in most cases you would probably need to be explicit about what the "root" directory is that you're calculating the relative path from.

As for using PackageID, AssemblyName, or other properties, it is true that the logic duplicated, but I'm thinking we should get rid of that duplication, and even if we didn't we wouldn't want the two definitions to be out of sync.

@Kuinox
Copy link

Kuinox commented Apr 21, 2023

What about inserting a .gitignore containing * in .artifact, automatically, so the user don't need to setup a .gitignore in the first place ?

@alexrp
Copy link

alexrp commented Apr 21, 2023

@dsplaisted

I'm not sure if we want to add a property for this, since in most cases you would probably need to be explicit about what the "root" directory is that you're calculating the relative path from.

Would you though? Isn't it just going to be the same directory that Directory.Build.props was found in? I can't really think of a scenario where I might want it to be anything else.

@mattleibow
Copy link
Member

I can't really think of a scenario where I might want it to be anything else.

I have been using multiple levels of props. A repo root and then a separate props for tests, source and samples. This is because each level of projects has different things that need setting.

We also have sub folders with groups, so have a source folder with maybe core, graphics, and maybe compatibility. And in each of those we have a src, tests, samples.

And we would still want outputs in the top level folder.

@RussKie
Copy link
Member

RussKie commented May 2, 2023

Something I stumbled upon when working on a project that uses dockerfile. Currently, a project would likely have a dockerfile at the same (or at a deeper) level as the project's .\bin folder, and the dockerfile would have commands similar to the this: COPY ./bin/Debug/net6.0/publish /app. Now, if the solution (the project belongs to) moves to use UseArtifactsOutput, the "COPY" command becomes not so "friendly"...

Obviously, this is not unresolvable, but I feel it's worth being called out. Actually, this becomes a problem as docker doesn't allow to pull from parent folders, meaning a dev won't be able to substitute ./bin/... with ../../../artifacts/....

@DamianEdwards
Copy link
Member

Actually, this becomes a problem as docker doesn't allow to pull from parent folders, meaning a dev won't be able to substitute ./bin/... with ../../../artifacts/....

This is a good point @RussKie. Generally though, it's normal for the Docker build to be run in the context of the .sln file in .NET projects, so that solution-level config like nuget.config and global.json are honored correctly. This is how the VS Container tools for Docker are setup to run by default.

@baronfel
Copy link
Member

baronfel commented May 2, 2023

The finicky nature of handling paths within Dockerfiles was also a big driver for our SDK-native container builds, which are entirely in-box for 7.0.3xx and onwards and mitigate the need to manage paths. There are a few feature gaps (e.g. lack of RUN command support) but we could encourage the use of that tech more for this layout potentially.

@davidhunter22
Copy link

davidhunter22 commented May 14, 2023

Is there a way of switching off the project name directory layer in the .artifacts/bin/ProjectName/... tree. I tried <ArtifactsProjectName></ArtifactsProjectName> but that was ignored. I can do <ArtifactsProjectName>Unused</ArtifactsProjectName> in my Directory.Build.props but that leads to a, for us, useless layer in the directory tree.
Note that I only want to omit the project name layer in the bin tree as I know have no filename conflicts there not in the obj tree where we almost certainly do.

@dsplaisted
Copy link
Member Author

Is there a way of switching off the project name directory layer in the .artifacts/bin/ProjectName/... tree. I tried <ArtifactsProjectName></ArtifactsProjectName> but that was ignored. I can do <ArtifactsProjectName>Unused</ArtifactsProjectName> in my Directory.Build.props but that leads to a, for us, useless layer in the directory tree. Note that I only want to omit the project name layer in the bin tree as I know have no filename conflicts there not in the obj tree where we almost certainly do.

You can set IncludeProjectNameInArtifactsPaths to false. However, this currently applies to the intermediate output as well as the output path. We may want to change that, as at a minimum project.assets.json will probably always collide.

@davidhunter22
Copy link

davidhunter22 commented May 15, 2023

Is there a way of switching off the project name directory layer in the .artifacts/bin/ProjectName/... tree. I tried <ArtifactsProjectName></ArtifactsProjectName> but that was ignored. I can do <ArtifactsProjectName>Unused</ArtifactsProjectName> in my Directory.Build.props but that leads to a, for us, useless layer in the directory tree. Note that I only want to omit the project name layer in the bin tree as I know have no filename conflicts there not in the obj tree where we almost certainly do.

You can set IncludeProjectNameInArtifactsPaths to false. However, this currently applies to the intermediate output as well as the output path. We may want to change that, as at a minimum project.assets.json will probably always collide.

Aaah so close. I can't imagine any situation your would ever want to remove the project name from the obj tree, unless you have a solution with only one project. I suspect removing the project name from the bin tree would be very common, in fact maybe more common than wanting it there IMHO!

As a work around I found that you can add

        <IncludeProjectNameInArtifactsPaths>false</IncludeProjectNameInArtifactsPaths>
        <BaseIntermediateOutputPath>obj</BaseIntermediateOutputPath>

I suspect nobody really cares where obj goes, although it is nice to have it out of tree, sort of

@jrdodds
Copy link

jrdodds commented Jun 13, 2023

Will the proposal document be updated regarding not using .artifacts as the folder name?

Also this line:

  • 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.

is off the mark.

On Unix (macOS) and Unix-like (Linux) OSs files and folders that begin with . are hidden. For example, the ls command with no switches will not show hidden files. It's not "some contexts". It's universal. And you have to 'opt-in' in some form to make the hidden files visible.

@msedi
Copy link

msedi commented Aug 14, 2023

I like the proposal, it more or less does what we had to do manually. One question though:

We have introduced categories: Apps, Assemblies, Plugins, Tools and each assembly is built into this subcategory, otherwise we get a very long list of inidividual outputs. So the question is if we are able to adjust these. In the proposal I have read about

Directory.AfterTargetFrameworkInference.targets

is this what is meant to do the above and should it already work?

@davhdavh
Copy link

IMHO this does not solve the biggest problem there currently is with builds and containers.
There is no way to split the library dlls from the application dlls.
Example, this is the Dockerfile generated today:

FROM base AS final
WORKDIR /app
COPY --from=publish /app/publish .
ENTRYPOINT ["dotnet", "WebApplication1.dll"]

but this means EVERY SINGLE TIME I rebuild this application, it has to make a layer that contains both my application code and library.
What we would really like is that there is a seldom changing layer with library, and a layer with application

FROM base AS final
COPY --from=publish /app/publish/library /library
ENV DOTNET_EXTRA_DLL_PATH=/library
WORKDIR /app
COPY --from=publish /app/publish/application .
ENTRYPOINT ["dotnet", "WebApplication1.dll"]

@LunarWhisper
Copy link

Please pay attention to this case:

        <IncludeProjectNameInArtifactsPaths>false</IncludeProjectNameInArtifactsPaths>
        <BaseIntermediateOutputPath>obj</BaseIntermediateOutputPath>

It seems to me that everyone who wanted to move the Output of their projects did it a long time ago, via adding Output with $(ProjectName).

What we really need - a way to build .sln of 300 projects with shared DLLs and independent EXE to the SINGLE output folder (not a Per-project). With correct resolving of NuGet dependencies, build order, error handling when two projects trying to CopyLocal different files with the same name, and avoid access denied because two projects want to copy .pdb of a shared DLL to the output during multi-threaded build.

Think about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.