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

SDK doesnt publish exes correctly when used as a project reference #1675

Closed
jaredpar opened this issue Oct 20, 2017 · 36 comments
Closed

SDK doesnt publish exes correctly when used as a project reference #1675

jaredpar opened this issue Oct 20, 2017 · 36 comments
Assignees
Milestone

Comments

@jaredpar
Copy link
Member

jaredpar commented Oct 20, 2017

Often a group of exes need to be built and deployed together. This is common in applications like Roslyn where we ship several exes as a single unit: csc, vbc and VBCSCompiler.

Rather than building each project separately and then copying the outputs together we create deployment projects. Essentially dummy exes projects that just reference all of the exes that we need via project reference elements

<ProjectReference Include="..\..\src\Compilers\CSharp\csc\csc.csproj" />
<ProjectReference Include="..\..\src\Compilers\VisualBasic\vbc\vbc.csproj" />
<ProjectReference Include="..\..\src\Compilers\Server\VBCSCompiler\VBCSCompiler.csproj" />

This approach works fine in desktop MSBuild as it will correctly deploy all of the runtime assets. The same approach does not work on SDK projects though as the publish step fails to include critical files like deps.json and runtimeconfig.json. That completely breaks this approach.

If this isn't meant to be supported by SDK that would be unfortunate but understandable. I would expect an error though positively asserting that this case is not supported. Today everything builds and publishes without error but the output is completely unusable.

Repo:

Clone https://github.com/jaredpar/roslyn and checkout the branch repro/group-exe. Then run the following commands:

> dotnet restore build/ToolsetPackages/BaseToolset.csproj
> dotnet restore build/ToolsetPackages/CoreToolset.csproj
> dotnet restore build/Toolset/CoreToolset.csproj
> dotnet publish -o ${HOME}/temp/test --framework netcoreapp2.0 build/Toolset/CoreToolset.csproj

The directory ${HOME}/temp/test should contain the deps.json / runtimeconfig.json for csc, vbc and VBCSCompiler but it does not. That means the output of publish is unusable.

CC @khyperia

@livarcocc
Copy link
Contributor

So, if I understand this correct, you have a project that references three other self-contained netcoreapp projects as project references and you expect that publish this aggregator project will also publish all its references as an app?

If that's the case, you are right, we do not support that. P2P references are treated as libraries and not applications. In order to accomplish this I think you could change your aggregator project to have a list of projects that it needs to publish and for you to invoke publish in each of them separately.

@jaredpar
Copy link
Member Author

. In order to accomplish this I think you could change your aggregator project to have a list of projects that it needs to publish and for you to invoke publish in each of them separately.

That doesn't quite get the behavior we want though. Part of the advantage to using the <ProjectReference> route is that MSBuild hepls ensure we have consistent dependencies across the projects. If any dependencies are wrong the standard warnings around conflicts will come into play.

This is important because the versions need to be consistent in order to ensure the exes will run correctly when all their dependencies are dumped into a single directory.

If that's the case, you are right, we do not support that. P2P references are treated as libraries and not applications

In that case why do the app.config files get deployed at all? Thats only relevant when running the application. I would assume, possibly wrong, that app.config, runtimeconfig.json and deps.json would all be deployed or none.

@dsplaisted
Copy link
Member

I would also like some way to publish multiple apps to the same folder (and correctly handle the dependencies).

However, it's not as simple as copying the additional files when an app project is referenced. The deps.json and runtimeconfig.json will have been generated within the context of the app's dependencies. If the "publish" project ends up resolving different versions of those dependencies, then the versions deployed won't match what the app is expecting, which I think could lead to failures.

@jaredpar
Copy link
Member Author

If the "publish" project ends up resolving different versions of those dependencies, then the versions deployed won't match what the app is expecting, which I think could lead to failures.

Exactly. We want these failures. It tells us that we screwed up our dependencies 😄

@dsplaisted
Copy link
Member

Here's a potential workaround that includes the deps.json and runtimeconfig.json in the files that referenced projects will copy.

  <!-- Include MSBuild.deps.json and MSBuild.runtimeconfig.json in ContentWithTargetPath so they will be copied to the output folder of projects
       that reference this one. -->
  <Target Name="AddRuntimeDependenciesToContent" Condition=" '$(TargetFrameworkIdentifier)' == '.NETCoreApp'" BeforeTargets="GetCopyToOutputDirectoryItems">
    <ItemGroup>
      <ContentWithTargetPath Include="$(ProjectDepsFilePath)" CopyToOutputDirectory="PreserveNewest" TargetPath="$(ProjectDepsFileName)" />

      <ContentWithTargetPath Include="$(ProjectRuntimeConfigFilePath)" CopyToOutputDirectory="PreserveNewest" TargetPath="$(ProjectRuntimeConfigFileName)" />
    </ItemGroup>
  </Target>

@jaredpar
Copy link
Member Author

jaredpar commented Dec 5, 2017

@dsplaisted

How supported is that work around? This is a feature we sorely miss since switching to the new SDK / Exes (lots of duplication + missed errors). If this is fairly supported we'd consider switching to use it.

@livarcocc livarcocc added this to the Unknown milestone Jan 25, 2018
@dsplaisted
Copy link
Member

@jaredpar I wouldn't call it supported. The problem is going to be if the different Exes have different versions of shared dependencies. If that's not the case, then I think everything should work well.

@jaredpar
Copy link
Member Author

If it's not supported I'm rather hesitant to move to it. Depending on unsupported features is equally painful to both our teams when they eventually get broken

@ericstj
Copy link
Member

ericstj commented May 16, 2018

@dsplaisted what part of this are you feeling like is unsupported? Ensuring the workaround stays working? Seems like we can just add a test case to make sure that is true. If the SDK does add support for this then you can make sure you won't break the workaround. I don't think there is anything fundamentally wrong with flowing the deps/runtimeconfig along with other things. I can see how this might be important for other project types like NuProj/WixProj/Azure-packages/etc.

It sounds like @jaredpar's not asking us to vouch for the deployment strategy of combining apps into a single directory as "supported" since he's expecting it to fail sometimes if they do the wrong thing and mentioned they'll need to test for that.

@dsplaisted dsplaisted self-assigned this Feb 5, 2019
@dsplaisted dsplaisted modified the milestones: Unknown, 3.0.1xx Feb 5, 2019
sbomer added a commit to sbomer/linker that referenced this issue Mar 5, 2019
The workaround for dotnet/sdk#1675 must run
after GenerateBuildDependencyFile and
GenerateBuildRuntimeConfigurationFiles to ensure that the deps.json
and runtimeconfig.json files exist at that time.
marek-safar pushed a commit to dotnet/linker that referenced this issue Mar 8, 2019
* Run illink out-of-process by implementing ToolTask

* Publish illink with runtimeconfig.json and deps.json

* Ensure that publish workaround runs after runtime files exist

The workaround for dotnet/sdk#1675 must run
after GenerateBuildDependencyFile and
GenerateBuildRuntimeConfigurationFiles to ensure that the deps.json
and runtimeconfig.json files exist at that time.

* Avoid string concatenation when using StringBuilder
@ViktorHofer
Copy link
Member

@dsplaisted can we please adjust the milestone? We have a tracking issue against this in dotnet/runtime: https://github.com/dotnet/runtime/blob/ec9d33daf4f1922b2f36f548752ac4539f370c91/eng/testing/runtimeConfiguration.targets#L19

@dsplaisted dsplaisted modified the milestones: 3.0.1xx, 5.0.1xx Apr 22, 2020
@MartyIX
Copy link

MartyIX commented Jul 8, 2020

This helped me https://github.com/dotnet/runtime/blob/ec9d33daf4f1922b2f36f548752ac4539f370c91/eng/testing/runtimeConfiguration.targets#L19 for my .NET Core 3.1 application:

  <ItemGroup Condition="'$(GenerateRuntimeConfigurationFiles)' == 'true'">
    <None Include="$(TestRuntimeConfigurationFile)"
          Condition="Exists('$(TestRuntimeConfigurationFile)')"
          Link="$(TargetName).exe.config"
          CopyToOutputDirectory="PreserveNewest"
          Visible="false" />
    <!--
      Include deps.json and runtimeconfig.json in ContentWithTargetPath so they will
      be copied to the output folder of projects that reference this one.
      Tracking issue: https://github.com/dotnet/sdk/issues/1675
    -->
    <ContentWithTargetPath Include="$(ProjectDepsFilePath)"
                           Condition="'$(TargetsNetCoreApp)' == 'true' and '$(GenerateDependencyFile)' == 'true'"
                           CopyToOutputDirectory="PreserveNewest"
                           TargetPath="$(ProjectDepsFileName)" />
    <ContentWithTargetPath Include="$(ProjectRuntimeConfigFilePath)"
                           Condition="'$(TargetsNetCoreApp)' == 'true'"
                           CopyToOutputDirectory="PreserveNewest"
                           TargetPath="$(ProjectRuntimeConfigFileName)" />
  </ItemGroup>

sbomer/linker@fda18bc

edit: useful link for my future reference: https://github.com/dotnet/sdk/blob/master/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets

@ViktorHofer
Copy link
Member

That's me implementing (mostly copying) what @dsplaisted suggested above 😄

@MartyIX
Copy link

MartyIX commented Jul 8, 2020

@ViktorHofer I've just spent an hour trying to fix the original issue, so I posted your version because it's not obvious that one needs to use the latest master version in git to get to a working solution.

Thank you for your work!

@ViktorHofer
Copy link
Member

Sure, np.

because it's not obvious that one needs to use the latest master version in git to get to a working solution.

what do you mean by that?

@MartyIX
Copy link

MartyIX commented Jul 8, 2020

@dsplaisted can we please adjust the milestone? We have a tracking issue against this in dotnet/runtime: https://github.com/dotnet/runtime/blob/ec9d33daf4f1922b2f36f548752ac4539f370c91/eng/testing/runtimeConfiguration.targets#L19

@ViktorHofer This refers to some commit from 2019. This one is newer: https://github.com/dotnet/runtime/blob/ec9d33daf4f1922b2f36f548752ac4539f370c91/eng/testing/runtimeConfiguration.targets#L19 and that code works for me.

@ViktorHofer
Copy link
Member

Got it, thanks for clarifying.

@kirsan31
Copy link

The same approach does not work on SDK projects though as the publish step fails to include critical files like deps.json and runtimeconfig.json.

Just to note, that the problem appear not only on publish step, but on simple build too, see #11207.

@urosjovanovic
Copy link

Yes, it is also not copied on build action.

I have tried the workaround proposed above but had no luck. It throws a build error that it cannot copy *.deps.json and *.runtimeconfig.json because they do not exist.

I've added the target at the end of referenced .csproj (.net console app).

@TonyValenti
Copy link

We're running into challenges because of this too. It would be really nice if this could get resolved.

@dsplaisted dsplaisted modified the milestones: 5.0.1xx, 5.0.2xx Oct 8, 2020
@MartyIX
Copy link

MartyIX commented Oct 27, 2020

How can I use #1675 (comment) in .NET 5?

I believe that '$(TargetFrameworkIdentifier)' == '.NETCoreApp' needs to be modified but I'm not sure what the new identifier is.

@ltrzesniewski
Copy link
Contributor

@MartyIX $(TargetFrameworkIdentifier) is still .NETCoreApp in .NET 5.

@ChrisUnityArto
Copy link

ChrisUnityArto commented Nov 4, 2020

I'm glad to see this is due to be worked on.

dsplaisted added a commit to dsplaisted/sdk that referenced this issue Nov 5, 2020
DavidKarlas added a commit to dotnet/templating that referenced this issue Dec 4, 2020
Changes file by file:
-`dotnet-new3.UnitTests.csproj`: Add "Microsoft.DotNet.Cli.Utils" nuget because `Command` and `CommandResult` are all defined there...
-`AllProjectsWork.cs`: Change namespace so we reach "Program" in `dotnet-new3.csproj` without new `using`
-`dotnet-new3.csproj`: This is workaround for dotnet/sdk#1675 problem is that for our unit tests it copies .exe and all .dlls, but it doesn't copy `dotnet-new3.deps.json` and `dotnet-new3.runtimeconfig.json` which are critical to execute `dotnet-new3.exe`
DavidKarlas added a commit to DavidKarlas/templating that referenced this issue Dec 11, 2020
Changes file by file:
-`dotnet-new3.UnitTests.csproj`: Add "Microsoft.DotNet.Cli.Utils" nuget because `Command` and `CommandResult` are all defined there...
-`AllProjectsWork.cs`: Change namespace so we reach "Program" in `dotnet-new3.csproj` without new `using`
-`dotnet-new3.csproj`: This is workaround for dotnet/sdk#1675 problem is that for our unit tests it copies .exe and all .dlls, but it doesn't copy `dotnet-new3.deps.json` and `dotnet-new3.runtimeconfig.json` which are critical to execute `dotnet-new3.exe`
DavidKarlas added a commit to dotnet/templating that referenced this issue Dec 14, 2020
Changes file by file:
-`dotnet-new3.UnitTests.csproj`: Add "Microsoft.DotNet.Cli.Utils" nuget because `Command` and `CommandResult` are all defined there...
-`AllProjectsWork.cs`: Change namespace so we reach "Program" in `dotnet-new3.csproj` without new `using`
-`dotnet-new3.csproj`: This is workaround for dotnet/sdk#1675 problem is that for our unit tests it copies .exe and all .dlls, but it doesn't copy `dotnet-new3.deps.json` and `dotnet-new3.runtimeconfig.json` which are critical to execute `dotnet-new3.exe`
@ViktorHofer
Copy link
Member

Fixed by #14488

@kirsan31
Copy link

@ViktorHofer In What VS version it will be included?

@dsplaisted
Copy link
Member

It should be in .NET SDK 5.0.200, which will ship with VS 16.9

@RandomEngy
Copy link

VS 16.9.0 is out and it works! Thanks for the fix.

@MartyIX
Copy link

MartyIX commented Oct 14, 2021

Fixed by #14488

This works for me in .NET 5. However, I wonder whether this issue is back in .NET 6 RC 2 as I see the following issue: https://github.com/dotnet/efcore/issues/26350?

Would anybody know?

GangWang01 pushed a commit to GangWang01/sdk that referenced this issue Jun 7, 2022
Changes file by file:
-`dotnet-new3.UnitTests.csproj`: Add "Microsoft.DotNet.Cli.Utils" nuget because `Command` and `CommandResult` are all defined there...
-`AllProjectsWork.cs`: Change namespace so we reach "Program" in `dotnet-new3.csproj` without new `using`
-`dotnet-new3.csproj`: This is workaround for dotnet#1675 problem is that for our unit tests it copies .exe and all .dlls, but it doesn't copy `dotnet-new3.deps.json` and `dotnet-new3.runtimeconfig.json` which are critical to execute `dotnet-new3.exe`
GangWang01 pushed a commit to GangWang01/sdk that referenced this issue Jul 11, 2022
Changes file by file:
-`dotnet-new3.UnitTests.csproj`: Add "Microsoft.DotNet.Cli.Utils" nuget because `Command` and `CommandResult` are all defined there...
-`AllProjectsWork.cs`: Change namespace so we reach "Program" in `dotnet-new3.csproj` without new `using`
-`dotnet-new3.csproj`: This is workaround for dotnet#1675 problem is that for our unit tests it copies .exe and all .dlls, but it doesn't copy `dotnet-new3.deps.json` and `dotnet-new3.runtimeconfig.json` which are critical to execute `dotnet-new3.exe`
tkapin pushed a commit to tkapin/runtime that referenced this issue Jan 31, 2023
* Run illink out-of-process by implementing ToolTask

* Publish illink with runtimeconfig.json and deps.json

* Ensure that publish workaround runs after runtime files exist

The workaround for dotnet/sdk#1675 must run
after GenerateBuildDependencyFile and
GenerateBuildRuntimeConfigurationFiles to ensure that the deps.json
and runtimeconfig.json files exist at that time.

* Avoid string concatenation when using StringBuilder


Commit migrated from dotnet/linker@ddf99d8
JL03-Yue pushed a commit that referenced this issue Mar 19, 2024
…728.16 (#1675)

[main] Update dependencies from dotnet/arcade
@jeremy-visionaid
Copy link

Looks like this is still a problem for Managed C++ dependencies, but the workarounds suggested here for adding it to ContentWithTargetPath manually work for those too. I'll open a new issue if I get the chance.

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

No branches or pull requests