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

MSBuild does not honor solution Project Dependencies #8099

Closed
TheXenocide opened this issue Oct 27, 2022 · 22 comments
Closed

MSBuild does not honor solution Project Dependencies #8099

TheXenocide opened this issue Oct 27, 2022 · 22 comments
Labels
bug needs-triage Have yet to determine what bucket this goes in.

Comments

@TheXenocide
Copy link

TheXenocide commented Oct 27, 2022

Issue Description

There does not appear to be a functional way to represent a soft (build order only) project dependency using the MSBuild command line. Visual Studio has a feature to indicate that one project depends on another without referencing it, which works, but when building from the command line these relationships are not honored.

Steps to Reproduce

In one scenario we need this functionality, we have a basic pattern with at least 3 projects (this is slightly simplified from our real world implementation, but expresses the important moving parts):

  • A Contracts project that contains serializable POCO DTO types and service interfaces.
  • A Web API project with controllers that implement service interfaces and return the DTOs.
  • A Client project that contains generated typed HTTP clients (which also implement the service interfaces so that shared code can swap implementations between server-side and client-side)

The Contracts project is a fairly straightforward class library that targets netstandard2.0.

The Web API project is an ASP.NET Core project (currently targeting netcore3.1, but with some prototypes migrating to net6 that have the same issues) that references the Contracts project. There is a post-build target that generates an Open API Specification file describing the controllers (NSwag json output).

The Client project is a class library that targets netstandard2.0 and references the Contracts project. There is a pre-build step that uses NSwag to generate typed HTTP clients (and some supporting extension methods for registering them in a service collection, etc.). This project has a loose build order dependency on the Web API project (because it consumes the swagger json file produced post-build in the Web API, but it can not--and does not want to--reference the netcoreapp3.1 project from the netstandard2.0 project).

Expected Behavior

In Visual Studio we use the "Project Dependencies" dialog to indicate that Client depends on Web API and it works well within the IDE. Ideally that feature would just work when building from the command line as well.

Actual Behavior

When a clean source tree is checked out or the bin/obj directories are cleaned out (or, really, when the swagger json file does not exist on disk) the build fails because the Client project builds before the Web API project is finished.

Analysis

We've tried a variety of ways to work around this with a ProjectReference from the Client project to the Web API project, but can't seem to figure out the right metadata/configuration to express the loose dependency. Any guidance would be greatly appreciated. Here are some of the things we've tried in a variety of combinations:

<ProjectReference Include="..\AspNetCore.TestHarness\AspNetCore.TestHarness.csproj">
  <Private>false</Private>
  <ReferenceOutputAssembly>false</ReferenceOutputAssembly>
  <ExcludeAssets>all</ExcludeAssets>
  <PrivateAssets>all</PrivateAssets>
  <DisableTransitiveProjectReferences>true</DisableTransitiveProjectReferences>
  <DisableTransitiveFrameworkReferences>true</DisableTransitiveFrameworkReferences>
  <SkipGetTargetFrameworkProperties>true</SkipGetTargetFrameworkProperties>
</ProjectReference>

Some references found along the way:

Versions & Configurations

This has been an ongoing struggle with VS 2017, 2019, and 2022 in a variety of different scenarios. One of the latest machines I reproduced this on is running:

MSBuild version 17.3.1+2badb37d1 for .NET Framework
17.3.1.41501

@TheXenocide TheXenocide added bug needs-triage Have yet to determine what bucket this goes in. labels Oct 27, 2022
@rainersigwald
Copy link
Member

Actual Behavior

When a clean source tree is checked out or the bin/obj directories are cleaned out (or, really, when the swagger json file does not exist on disk) the build fails because the Client project builds before the Web API project is finished.

Can you please be very specific about how you're doing the builds?

Details about what you're seeing with the XML you shared would also be helpful.

@TheXenocide
Copy link
Author

Our NSwag generation functionality is packaged together with a variety of other features in a custom SDK I can't share here or I would already have a repro project up for you, but I will try to extract out the functionality necessary to repro the issue if I can find some time (very busy trying to prepare for a release these days).

There's not much to say from the output of the above XML except that, somehow the Client project builds way earlier than the Web API project which seems like it's disregarding the reference entirely (maybe we've disabled too many of the relevant functionlities with the metadata?). The only error we get with the whole set of those specified is that the json file our NSwag client generation is trying to use doesn't exist (NSwag throws a FileNotFoundException) and then much further down that particular solution's build log the Web API project builds and successfully generates a swagger document.

In past experiments (which I don't have a lot of time to rerun at the immediate moment, but will try to gather some sensible permutations if it will help, though it seems like the repro solution would probably be a better priority) if we omit various metadata from that set then we wind up with TFM validation errors (you can't target netcoreapp3.1 from netstandard2.0) or with that disabled and other metadata omitted we generate NuGet packages with dependencies we don't actually want (like server-side ASP.NET Core libraries/transitive references being referenced from the client-side package, etc.)

One separate workaround (not using ProjectReference for the loose dependency at all) that we've used in CI/CD environments is to disable parallel builds and make sure the project entries in solution file are in an order that places the Web API project before the Client project because it seems MSBuild somewhat honors the order they appear in when the projects have equivalent order in the sorted dependency hierarchy, but this doesn't fit all situations if the dependency hierarchy with other real project references places them at different orders (I believe we "fixed" any of those we had a while back when we split things into smaller solutions for other purposes, but anecdotally I believe I remember the challenge/behavior). That said, it would be nice if we could properly leverage parallel builds; also it doesn't seem to work quite as reliably for me when running scripts on dev machines, though I'm not sure what's different.

@Forgind
Copy link
Member

Forgind commented Oct 27, 2022

I'm not sure if this is helpful, but I think there's an option on ProjectReference items (ReferenceOutputAssembly?) that, when set to false, basically just says "build the other thing first."

Doesn't directly fix your situation, but if you could use that as a workaround.

@TheXenocide
Copy link
Author

@Forgind yeah, we started with that (and it's in the sample attempt at a workaround above) but there were still side effects from transitive dependencies so we've been trying to add more metadata to varying (unsuccessful) results.

@rainersigwald
Copy link
Member

Can you please be very specific about how you're doing the builds?

I didn't see an answer to this. Are you building the solution, or individual projects? Exactly how?

@rainersigwald
Copy link
Member

The only error we get with the whole set of those specified is that the json file our NSwag client generation is trying to use doesn't exist

How is this file specified in the Client project?

@TheXenocide
Copy link
Author

TheXenocide commented Oct 28, 2022

Can you please be very specific about how you're doing the builds?

MSBuild.exe [path to solution] /t:Restore;Build

How is this file specified in the Client project?

A Target in our custom MSBuild SDK somewhat like this guidance, reused throughout a number of solutions. We define some custom Item groups for parameters and paths and are looking at reworking some of it as we move to .NET 6 (especially if we can get a decent solution to the dependency order concern). If we build the solution and it fails then build just the client project it succeeds because the swagger document now exists, just not at the time the client project tried to build in the solution build order. There are some other improvements we can make with better input/output tracking for build optimization, but those aren't related to the build order issue. In its current incarnation, the client project contains something like the following:

<ItemGroup>
  <ApiClientAdditionalNamespaces Include="System.Configuration" />
  <!-- ... etc. ... -->
</ItemGroup>

<ItemGroup>
  <SwaggerClient Include="ApiClient">
    <Swagger>[... path to swagger document ...].json</Swagger>
    <AdditionalNamespaces>@(ApiClientAdditionalNamespaces)</AdditionalNamespaces>
    <GenerateSyncMethods>false</GenerateSyncMethods>
  </SwaggerClient>
  
  <Compile Include="obj\$(Configuration)\$(TargetFramework)\SwaggerClient\ApiClient.Generated.cs" Link="ApiClient.Generated.cs">
    <DependentUpon>ApiClient.cs</DependentUpon>
  </Compile>
</ItemGroup>

Honestly, if there's a nice way to export the path to the swagger document from the ProjectReference that would be a lovely benefit to this "workaround" approach and we might be able to better abstract a few things, but the most painful part is the build order at the immediate moment.

@KalleOlaviNiemitalo
Copy link

MSBuild.exe [path to solution] /t:Restore;Build

Better use /restore /t:Build instead, so that the build phase will reload any MSBuild files that were updated during restore. I don't see how that could cause the problem with project dependencies, though.

@TheXenocide
Copy link
Author

Better use /restore /t:Build instead

Ah, thanks for the heads up! I think I've actually seen the side effects of that before, just didn't know /restore was an option.

@TheXenocide
Copy link
Author

For reference, when I say "Solution Project Dependencies" I mean our SLN file has a fragment that looks something like this:

Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "AspNetCore.TestHarness", "AspNetCore.TestHarness\AspNetCore.TestHarness.csproj", "{21F13BC5-C640-422B-9AD5-757F6FE20957}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "AspNetCore.TestHarness.Client", "AspNetCore.TestHarness.Client\AspNetCore.TestHarness.Client.csproj", "{C3F56781-79BB-4C55-B806-196119BD73C5}"
	ProjectSection(ProjectDependencies) = postProject
		{21F13BC5-C640-422B-9AD5-757F6FE20957} = {21F13BC5-C640-422B-9AD5-757F6FE20957}
	EndProjectSection
EndProject

The part not being honored is the ProjectSection(ProjectDependencies) = postProject section, which works as expected in Visual Studio but is not honored when I build from the command line. Important to note is that I forgot to include the /m on the command line example above, but it's a very simple invocation of MSBuild and these are the only parameters.

Realistically, if we can get that loose ProjectReference to work in some way it would be even better for us than the solution dependencies in the long run because we could make a new ItemGroupDefinition like NSwagGenerationReference that would generate the necessary project reference and, ideally, get the path to the generated swagger document from the referenced project itself (which it knows and calculates based on its own properties/items) so that we don't have as many manual properties and item includes. Ideally we would put all of the work in our SDK so that the projects would have very simple reusable logic (we use this pattern in many separate solutions) but it's unclear what makes the ProjectReference impact build order which is the first step to better abstracting this pattern.

That said, this still feels like a bug since the behavior differs and, even though the solution file is not an XML file, the functionality is still built into it and seems like it should be honored during command line builds.

@rainersigwald
Copy link
Member

the functionality is still built into it and seems like it should be honored during command line builds.

That is the case generally; when a project has solution build dependencies we treat it specially to enforce them. To determine what's going wrong in your specific case I think we'd need a repro or a binary log, which might have more information than you're willing to share.

@TheXenocide
Copy link
Author

I think we'd need a repro or a binary log

Yeah, I thought that might be the case. I just got back from vacation and this is now impacting some of our CI builds, even with the solution reordering trick we tried before. I'll try to extract out a repro, though I have a funny feeling once it's in a more simple solution the issue might not repro and I'm not 100% sure what conditions contribute to it. If so is there a way we can more confidentially share binlogs so we don't share any proprietary/sensitive build information on the open internet?

@rainersigwald
Copy link
Member

Yes! Open a feedback ticket, which offers uploads confidential to Microsoft.

After it's created, that will open an internal bug. If you post the link here we can short-circuit the normal "find the right team for this" process.

@TheXenocide
Copy link
Author

I was trying to create a minimal repro together in spare time but got sidetracked by a production issue as soon as I got back from holiday so apologies on the delay for additional information. In the meantime our build server has started failing in a way that is blocking development/testing so I'm going to get some binlogs together ASAP because this has become a high priority blocker for us in the time since we last spoke, and then I'll get back to trying to get the repro together (which I'm mildly concerned may be difficult to isolate in a standalone solution).

@TheXenocide
Copy link
Author

I had a few hiccups but was eventually able to get repro binlogs up in a feedback ticket.

We did find a workaround that was sufficient for our CI/CD environment (for now) which was a more aggressive attempt at our previous workaround: we manually sorted every project in the text of the sln file to match the build order, where previously we simply had to move the Client project below the Server project.

Another interesting detail I found was that the issue only happens locally when the -m (parallel build) switch is specified; single-threaded builds appear to work correctly (at least in the couple cases I tested). Unfortunately, the workaround still doesn't fix the issue in local builds using the -m switch, but it has at least unblocked us. I tried to include a good variety of binlogs both from our CI/CD environment as well as from a local development machine so that you can compare working and non-working scenarios.

@TheXenocide
Copy link
Author

So I'm in the process of renovating our NSwag design for our custom project SDKs during our migration to .NET 6 and I was hoping for an update on this. In the absence of a shorter term solution for the solution-defined project dependencies, and because I'm hoping to make a more elegant solution that can be expressed in the project file XML, is there any guidance on how I might best create a soft ProjectReference that would enforce the desired dependency without need for solution-defined project dependencies and without adding any actual compile-time or nuspec generation dependencies between the client library and the Web API library? We already have our own wrapper functionality for reusable NSwag behavior (generation properties/namespaces, etc.), but I'm tentatively thinking about adding support for something like the following (from the client library csproj file):

<NSwagProjectReference Include="..\WebAPI\WebApi.csproj">
  <ConfigurationMetadata>Some other values</ConfigurationMetadata>
</NSwagProjectReference>

which might be transformed by the project SDK into an actual ProjectReference that could be enforced by MSBuild; something like:

<ProjectReference Include="@(NSwagProjectReference)">
  <Private>false</Private>
  <ReferenceOutputAssembly>false</ReferenceOutputAssembly>
  <ExcludeAssets>all</ExcludeAssets>
  <PrivateAssets>all</PrivateAssets>
  <DisableTransitiveProjectReferences>true</DisableTransitiveProjectReferences>
  <DisableTransitiveFrameworkReferences>true</DisableTransitiveFrameworkReferences>
  <SkipGetTargetFrameworkProperties>true</SkipGetTargetFrameworkProperties>
</ProjectReference>

I'm also hoping to use it to get known output names from the reference instead of hardcoding names and paths to the json file, by using the MSBuild task with TargetOutputs (in a similar way to how other dependency information is retrieved across project boundaries), but I'm mostly concerned about what metadata to use or not use in the ProjectReference (or some more appropriate way to define the dependency) in order to ensure the MSBuild command line honors it. The set of metadata specified above seemed to be disregarded, and subsets seemed to add references we didn't want to certain aspects of the client library's outputs.

Keeping in mind the client library is targeting netstandard2.0 and is only dependent on a generated json file from the Web API (soon to be targeting net6) project.

@rainersigwald
Copy link
Member

That looks basically ok to me--have you tried it and are you seeing problems?

@TheXenocide
Copy link
Author

have you tried it and are you seeing problems?

Yeah, with that complete set of metadata we tried that locally and saw no change in behavior for local parallel builds (the Client build ran before the Web API build, even with the project reference being specified). I didn't collect binlogs back when I experimented with that (a while ago, before I ever opened this ticket) but if anyone is looking at the binlogs from the feedback ticket, that was applied in our "Common" solution (the first post in this issue mentions that we added it to the client project; the names are slightly modified from the actual project names in the solution, but I think you can see which projects that would refer to). I didn't try every permutation, but the experience I had with the full set specified here was that it stopped honoring the ProjectReference with the full set, though with a subset of that metadata (I think maybe Private, ReferenceOutputAssembly, and SkipTargetFrameworkProperties?) it did honor the dependency, but it added a bunch of inaccurate dependency information to the client assembly/nuget package (like the client nuget package wanted consumers to reference ASP.NET Core server-side libraries that they definitely shouldn't need to reference, especially given our client library is targeting .NET Standard because there are still .NET Classic 4.8 consumers of it).

I had a hard time creating a repro without extracting half of our custom SDK functionality around all this in the time I had allotted to solving our original issue and since we found a workaround for it, I no longer had capacity to focus on it much, but since I am now working on a new task we already had planned for our .NET 6 upgrade looking at rewriting some of this behavior, I'll try to do my initial development with this metadata set against some minimal test projects not using any of our other stuff to see if it works or repros before I migrate the logic back into our custom SDK (yay for a chance at prototyping that might offer a 2 birds one stone opportunity). I'll update with my findings.

@rainersigwald
Copy link
Member

Looking at your CommonComponents binlog, I can see

  <Target Name="NSwagClientGenCore"
          AfterTargets="BeforeBuild"

That is very early in the build process--specifically it's before handling ProjectReferences. Can you change that to AfterTargets="ResolveProjectReferences"?

MSBuild discovers references dynamically, and doesn't treat @(ProjectReference) specially (except in -graph mode). The ResolveProjectReferences target has an <MSBuild task call that builds the other projects--but the referencing project can start building before the referenced one.

@TheXenocide
Copy link
Author

Ooh, okay, I'll totally give that a whirl on the next go around. Do you think that would be impacted in solutions relying on solution-defined project dependencies (as in not using this ProjectReference-based model I'm working toward)? Trying to determine if I should try it out on a local build in the pre-.NET 6 codebase to see if that solves the issues in the existing stuff (It seems a little more obvious to me, even from naming alone, how it might be implicated in the ProjectReference-oriented code).

@rainersigwald
Copy link
Member

The same mechanism applies to the solution ordering, when MSBuild applies it. When VS applies it it works differently.

So I think it's worth trying the change in your existing codebase.

@TheXenocide
Copy link
Author

@rainersigwald after testing several scenarios against the existing codebase, that does appear to have solved the local console parallel build issues. Thanks so much! I guess I'm unclear on how Visual Studio behaves differently while using MSBuild under the hood, but given that works with the old solution-based project dependencies, I'm optimistic that it will also work with the new ProjectReference-based approach I'm working on. Thanks very kindly for the help!

ItEndsWithTens added a commit to ItEndsWithTens/SilentHillMapExaminer that referenced this issue Apr 16, 2023
Also run my custom build targets at the proper time; as per dotnet/msbuild#8099 they were configured to run a little too early in the build.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs-triage Have yet to determine what bucket this goes in.
Projects
None yet
Development

No branches or pull requests

4 participants