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

P2Ps should be allowed when ReferenceOutputAssembly=false even given TFM incompatibilities #939

Closed
AArnott opened this issue Mar 3, 2017 · 14 comments
Milestone

Comments

@AArnott
Copy link
Contributor

AArnott commented Mar 3, 2017

With VS2015 projects, I can have a P2P from a portable library to a net46 library by setting metadata on the project reference:

<ProjectReference Include="..\SomeNet46Lib\lib.csproj">
  <ReferenceOutputAssembly>false</ReferenceOutputAssembly>
</ProjectReference>

But with the .NET SDK projects, even with this metadata the build fails:

C:\Program Files (x86)\Microsoft Visual Studio\2017\d15rel\MSBuild\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Sdk.Common.targets(73,5): error : Project 'C:\git\pinvoke\src\CodeGeneration\CodeGeneration.csproj'
targets '.NETFramework,Version=v4.6'. It cannot be referenced by a project that targets '.NETPortable,Version=v0.0,Profile=Profile92'. [C:\git\pinvoke\src\CodeGeneration\CodeGeneration.csproj]

This blocks scenarios where a P2P exists merely for the sake of ensuring build ordering but without the assembly reference. In my particular scenario, the referenced project provides a binary that the build of the portable library picks up for code generation purposes.

@nguerrera
Copy link
Contributor

nguerrera commented Mar 4, 2017

Workaround: Try also adding SkipGetTargetFrameworkProperties=true metadata to the reference.

@AArnott
Copy link
Contributor Author

AArnott commented Mar 7, 2017

Thanks @nguerrera. But that doesn't work either. That causes a referencing project A to build the referenced project B per A's TargetFramework value instead of B's TargetFramework.

@nguerrera
Copy link
Contributor

Ah, I believe this would only happen if A is multi targeted. Is it?

Try adding UndefineProperties="TargetFramework" metadata as well.

@AArnott
Copy link
Contributor Author

AArnott commented Mar 7, 2017

Yes, A is multi-targeted.
And that additional metadata did the trick. Thanks.

Should we leave the issue active for making this scenario simpler, and/or work the way it used to?

@nguerrera
Copy link
Contributor

nguerrera commented Mar 7, 2017

Should we leave the issue active for making this scenario simpler, and/or work the way it used to?

Yes, this should work without the extra metadata.

dougbu added a commit to aspnet/BuildTools that referenced this issue Mar 21, 2017
- add Microsoft.AspNetCore.BuildTools.ApiCheck.Task project
  - include task assembly in the Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck package
  - add *.props and *.targets files to hook task into `Pack` target; runs just before .nupkg is created
- reference Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck project from the Sdk project
  - requires lots of hacks because dotnet/sdk#939 workarounds fail in this scenario

Improve project-to-project reference handling
- avoid `FileNotFoundException` in `AssemblyLoadContext.GetAssemblyName()` due to `\placeholder\` paths
- avoid `InvalidOperationException` in `PackageGraph.RuntimeIsCompatible()` due to missing .sha512 file

Add `api-check --compact-output` option
- use this option in the task
- separately, catch and log `FileNotFoundException`
- output types and stack traces of caught `Exception`s

nits:
- update solution to use current project type GUID
- apply VS suggestions e.g. `out var`
- make some Linq a bit more readable
- whitespace and `using` cleanup
- move all property groups above item groups in Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck.csproj

notes:
- don't use `UseCommandProcessor==true`; `ToolTask` writes out .cmd file with a BOM that cmd.exe hates
dougbu added a commit to aspnet/BuildTools that referenced this issue Mar 22, 2017
- aspnet/KoreBuild#143

Add Microsoft.AspNetCore.BuildTools.ApiCheck.Task project
  - include task assembly in the Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck package
  - add *.props and *.targets files to hook task into `Pack` target; runs just before .nupkg is created
- reference Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck project from the Sdk project
  - requires lots of hacks; dotnet/sdk#939 workarounds fail in this scenario

Fall back to `RuntimeTargets` when `RuntimeAssemblies` yields no assemblies

Improve project-to-project reference and native assembly handling
- avoid `FileNotFoundException` in `AssemblyLoadContext.GetAssemblyName()` due to `\placeholder\` paths
- catch and ignore `BadImageFormatException` in `AssemblyLoadContext.GetAssemblyName()`
- avoid `InvalidOperationException` in `PackageGraph.RuntimeIsCompatible()` due to missing .sha512 file

Change ApiCheck.exe's options and output
- add `api-check --compact-output` option
- generally make tool's output more useful to the task
  - catch and log `FileNotFoundException`; output types and stack traces of caught `Exception`s
- remove `--framework` option; unused in .NET Framework already
- change ApiCheck.exe option from `--ApiListing` to `--api-listing` for consistency w/ other options

nits:
- remove `PackageGraph.RuntimeIsCompatible()` `candidateRuntime` parameter
  - not needed and confusingly-named; passed value included in `compatibleRuntimes`
- update solution to use current project type GUID
- ignore global.json and launchSettings.json
- make some Linq a bit more readable
- whitespace and `using` cleanup
- move all property groups above item groups in Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck.csproj
- make a couple of constructors `private`; used only in `public static` methods
- ignore case a bit more

notes:
- don't use `UseCommandProcessor==true`; `ToolTask` writes out .cmd file with a BOM that cmd.exe hates
dougbu added a commit to aspnet/BuildTools that referenced this issue Mar 23, 2017
- aspnet/KoreBuild#143

Add Microsoft.AspNetCore.BuildTools.ApiCheck.Task project
  - include task assembly in the Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck package
  - add *.props and *.targets files to hook task into `Pack` target; runs just before .nupkg is created
- reference Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck project from the Sdk project
  - requires lots of hacks; dotnet/sdk#939 workarounds fail in this scenario

Fall back to `RuntimeTargets` when `RuntimeAssemblies` yields no assemblies

Improve project-to-project reference and native assembly handling
- avoid `FileNotFoundException` in `AssemblyLoadContext.GetAssemblyName()` due to `\placeholder\` paths
- catch and ignore `BadImageFormatException` in `AssemblyLoadContext.GetAssemblyName()`
- avoid `InvalidOperationException` in `PackageGraph.RuntimeIsCompatible()` due to missing .sha512 file

Change ApiCheck.exe's options and output
- add `api-check --compact-output` option
- generally make tool's output more useful to the task
  - catch and log `FileNotFoundException`; output types and stack traces of caught `Exception`s
- remove `--framework` option; unused in .NET Framework already
- change ApiCheck.exe option from `--ApiListing` to `--api-listing` for consistency w/ other options

nits:
- remove `PackageGraph.RuntimeIsCompatible()` `candidateRuntime` parameter
  - not needed and confusingly-named; passed value included in `compatibleRuntimes`
- update solution to use current project type GUID
- ignore global.json and launchSettings.json
- make some Linq a bit more readable
- whitespace and `using` cleanup
- move all property groups above item groups in Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck.csproj
- make a couple of constructors `private`; used only in `public static` methods
- ignore case a bit more

notes:
- don't use `UseCommandProcessor==true`; `ToolTask` writes out .cmd file with a BOM that cmd.exe hates
dougbu added a commit to aspnet/BuildTools that referenced this issue Mar 24, 2017
- aspnet/KoreBuild#143

Add Microsoft.AspNetCore.BuildTools.ApiCheck.Task project
  - include task assembly in the Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck package
  - add *.props and *.targets files to hook task into `Pack` target; runs just before .nupkg is created
- reference Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck project from the Sdk project
  - requires lots of hacks; dotnet/sdk#939 workarounds fail in this scenario

Fall back to `RuntimeTargets` when `RuntimeAssemblies` yields no assemblies

Improve project-to-project reference and native assembly handling
- avoid `FileNotFoundException` in `AssemblyLoadContext.GetAssemblyName()` due to `\placeholder\` paths
- catch and ignore `BadImageFormatException` in `AssemblyLoadContext.GetAssemblyName()`
- avoid `InvalidOperationException` in `PackageGraph.RuntimeIsCompatible()` due to missing .sha512 file

Change ApiCheck.exe's options and output
- add `api-check --compact-output` option
- generally make tool's output more useful to the task
  - catch and log `FileNotFoundException`; output types and stack traces of caught `Exception`s
- remove `--framework` option; unused in .NET Framework already
- change ApiCheck.exe option from `--ApiListing` to `--api-listing` for consistency w/ other options

nits:
- remove `PackageGraph.RuntimeIsCompatible()` `candidateRuntime` parameter
  - not needed and confusingly-named; passed value included in `compatibleRuntimes`
- update solution to use current project type GUID
- ignore global.json and launchSettings.json
- make some Linq a bit more readable
- whitespace and `using` cleanup
- move all property groups above item groups in Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck.csproj
- make a couple of constructors `private`; used only in `public static` methods
- ignore case a bit more

notes:
- don't use `UseCommandProcessor==true`; `ToolTask` writes out .cmd file with a BOM that cmd.exe hates
dougbu added a commit to aspnet/BuildTools that referenced this issue Mar 25, 2017
- aspnet/KoreBuild#143

Add Microsoft.AspNetCore.BuildTools.ApiCheck.Task project
  - include task assembly in the Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck package
  - add *.props and *.targets files to hook task into `Pack` target; runs just before .nupkg is created
- reference Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck project from the Sdk project
  - requires lots of hacks; dotnet/sdk#939 workarounds fail in this scenario
- bump ApiCheck package version

Improve project-to-project reference and native assembly handling
- avoid `FileNotFoundException` in `AssemblyLoadContext.GetAssemblyName()` due to `\placeholder\` paths
- catch and ignore `BadImageFormatException` in `AssemblyLoadContext.GetAssemblyName()`
- avoid `InvalidOperationException` in `PackageGraph.RuntimeIsCompatible()` due to missing .sha512 file

Fall back to `RuntimeTargets` when `RuntimeAssemblies` yields no assemblies

Change ApiCheck.exe's options and output
- add `api-check --compact-output` option
- generally make tool's output more useful to the task
  - catch and log `FileNotFoundException`; output types and stack traces of caught `Exception`s
- change ApiCheck.exe option from `--ApiListing` to `--api-listing` for consistency w/ other options

Use Microsoft.Extensions.CommandLineUtils.Sources package
- VersionTool project cannot do this because it has `public` properties of these `internal` types

Update appveyor.yml now that VS 2017 RC image is no more

nits:
- remove `PackageGraph.RuntimeIsCompatible()` `candidateRuntime` parameter
  - not needed and confusingly-named; passed value included in `compatibleRuntimes`
- update solution to use current project type GUID
- ignore global.json and launchSettings.json
- make some Linq a bit more readable
- whitespace and `using` cleanup
- move all property groups above item groups in Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck.csproj
- make a couple of constructors `private`; used only in `public static` methods
- ignore case a bit more
- remove duplicate `<PropertyGroup>` from `NugetReferenceResolver` project
- get this repo building on Linux
- don't need API checks in this repo
- centralize a few more properties in dependencies.props

notes:
- don't use `UseCommandProcessor==true`; `ToolTask` writes out .cmd file with a BOM that cmd.exe hates
@AArnott
Copy link
Contributor Author

AArnott commented Apr 11, 2017

This is badly broken. The workaround causes nuget restore to fail in VS (command line is fine) and also is related to a build failure that only occurs on some non-Windows machines including Travis CI Ubuntu.

I tried replacing this with a "project dependency" encoded in the solution file, and that fixed most of the symptoms, until I tried msbuild.exe my.sln when I learned that msbuild translates that solution dependency into a project reference during the build (@AndyGerlicher when did this feature get added?), with ReferenceOutputAssembly=false set (just as I wanted to do with my original ProjectReference item) and that of course repeats the original problem and the build fails because a net40 project can't depend on a netstandard1.5 project.

This inability to influence build ordering is really causing some pain here. Please fix soon!

@Sumo-MBryant
Copy link

Has anyone found a workaround that is close to successful?

For a minimal netstandard1.X project reference to a netcoreapp1.X project:

SkipGetTargetFrameworkProperties fails in GenerateDepsFile (#1020)

Interestingly enough when I restore and build from MSBuild directly the project.assets.json file is missing the project reference and builds successfully. When building from Visual Studio, the project.assets.json contains the reference with a broken framework "framework": "Unsupported,Version=v0.0" and fails to build.

@AArnott
Copy link
Contributor Author

AArnott commented May 8, 2017

No. I finally gave up and checked the binary into git so I didn't need a project reference. I tried for days but never found a way that got dotnet build, msbuild, and VS to all work correctly at once. 😦

@rainersigwald
Copy link
Member

This can be worked around by adding an outside-the-norm order dependency in MSBuild, by way of a custom call to the MSBuild task.

<Target Name="WorkaroundSdk939" BeforeTargets="ResolveProjectReferences">
  <MSBuild Project="..\..\the\other.csproj" />
</Target>

Note that depending on your specific needs, you might need to be careful to preserve configuration and other normally-handled-for-you properties, or call a specific target.

@AArnott

msbuild translates that solution dependency into a project reference during the build (@AndyGerlicher when did this feature get added?)

This appears to have been added to MSBuild in the dev11 timeframe. @cdmihai went into detail on the process in dotnet/msbuild#2274 (comment). The current team doesn't know why it's necessary.

@AArnott
Copy link
Contributor Author

AArnott commented Jul 14, 2017

Note that depending on your specific needs, you might need to be careful to preserve configuration and other normally-handled-for-you properties, or call a specific target.

Ya, that's what kills your proposed workaround in virtually all my scenarios. That would build the default configuration of the project, which could mean building all the target frameworks in debug mode, which is almost never what I would expect or need. Also, it would cause over-build, compiling twice etc. which can at least slow down the build, but also lead to symbols and DLLs not always matching up. It's a non-starter for me.
I've tried being very particular about passing in the right global properties to this call, but I guess there's a reason the ResolveProjectReferences target and its predecessors are so complicated. It's very hard to mimic.

@rainersigwald
Copy link
Member

@mhutch came up with an interesting workaround in dotnet/msbuild#2399 (comment):

<!-- workaround for https://github.com/Microsoft/msbuild/issues/2399 -->
<Target Name="WorkaroundMSBuildIssue2399" BeforeTargets="GetTargetFrameworkProperties">
  <PropertyGroup>
    <ReferringTargetFramework>$(TargetFramework)</ReferringTargetFramework>
  </PropertyGroup>
</Target>

In the referenced project.

That essentially disables the target-framework compatibility check for the referenced project, which could be somewhat dangerous (depending on the nature of other references to the project) but avoids this problem.

@rainersigwald
Copy link
Member

Workaround

Set

<AddSyntheticProjectReferencesForSolutionDependencies>false</AddSyntheticProjectReferencesForSolutionDependencies>

in the project that has the ProjectReference to the incompatible project. This prevents the elevation of solution build dependencies to ProjectReferences in AssignProjectConfiguration.

(Was poking around near this target for another reason and saw this.)

@nguerrera
Copy link
Contributor

Moving to msbuild because after the double-evaluation fix, this compatibility check happens there in the context of the caller.

@nguerrera
Copy link
Contributor

This issue was moved to dotnet/msbuild#2661

rainersigwald added a commit to rainersigwald/msbuild that referenced this issue Jan 16, 2018
It is confusing to explicitly specify that you _don't_ want the output
of another project referenced in this project and then be told that the
output is incompatible.

This commit listens to the preexisting ProjectReference metadatum
ReferenceOutputAssembly and avoids the compatibility/best-match checks
on ProjectReferences that avoid the dependency.

Fixes dotnet#2661 (and dotnet/sdk#939).
rainersigwald added a commit to rainersigwald/msbuild that referenced this issue Jan 19, 2018
It is confusing to explicitly specify that you _don't_ want the output
of another project referenced in this project and then be told that the
output is incompatible.

This commit listens to the preexisting ProjectReference metadatum
ReferenceOutputAssembly and avoids the compatibility/best-match checks
on ProjectReferences that avoid the dependency.

Fixes dotnet#2661 (and dotnet/sdk#939).
rainersigwald added a commit to dotnet/msbuild that referenced this issue Jan 19, 2018
It is confusing to explicitly specify that you _don't_ want the output
of another project referenced in this project and then be told that the
output is incompatible.

This commit listens to the preexisting ProjectReference metadatum
ReferenceOutputAssembly and avoids the compatibility/best-match checks
on ProjectReferences that avoid the dependency.

Fixes #2661 (and dotnet/sdk#939).
radical pushed a commit to radical/msbuild that referenced this issue Jan 30, 2018
It is confusing to explicitly specify that you _don't_ want the output
of another project referenced in this project and then be told that the
output is incompatible.

This commit listens to the preexisting ProjectReference metadatum
ReferenceOutputAssembly and avoids the compatibility/best-match checks
on ProjectReferences that avoid the dependency.

Fixes dotnet#2661 (and dotnet/sdk#939).
DotDeveloper95 added a commit to DotDeveloper95/BuildTools that referenced this issue Sep 4, 2023
- aspnet/KoreBuild#143

Add Microsoft.AspNetCore.BuildTools.ApiCheck.Task project
  - include task assembly in the Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck package
  - add *.props and *.targets files to hook task into `Pack` target; runs just before .nupkg is created
- reference Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck project from the Sdk project
  - requires lots of hacks; dotnet/sdk#939 workarounds fail in this scenario
- bump ApiCheck package version

Improve project-to-project reference and native assembly handling
- avoid `FileNotFoundException` in `AssemblyLoadContext.GetAssemblyName()` due to `\placeholder\` paths
- catch and ignore `BadImageFormatException` in `AssemblyLoadContext.GetAssemblyName()`
- avoid `InvalidOperationException` in `PackageGraph.RuntimeIsCompatible()` due to missing .sha512 file

Fall back to `RuntimeTargets` when `RuntimeAssemblies` yields no assemblies

Change ApiCheck.exe's options and output
- add `api-check --compact-output` option
- generally make tool's output more useful to the task
  - catch and log `FileNotFoundException`; output types and stack traces of caught `Exception`s
- change ApiCheck.exe option from `--ApiListing` to `--api-listing` for consistency w/ other options

Use Microsoft.Extensions.CommandLineUtils.Sources package
- VersionTool project cannot do this because it has `public` properties of these `internal` types

Update appveyor.yml now that VS 2017 RC image is no more

nits:
- remove `PackageGraph.RuntimeIsCompatible()` `candidateRuntime` parameter
  - not needed and confusingly-named; passed value included in `compatibleRuntimes`
- update solution to use current project type GUID
- ignore global.json and launchSettings.json
- make some Linq a bit more readable
- whitespace and `using` cleanup
- move all property groups above item groups in Microsoft.AspNetMicrosoft.AspNetCore.BuildTools.ApiCheck.csproj
- make a couple of constructors `private`; used only in `public static` methods
- ignore case a bit more
- remove duplicate `<PropertyGroup>` from `NugetReferenceResolver` project
- get this repo building on Linux
- don't need API checks in this repo
- centralize a few more properties in dependencies.props

notes:
- don't use `UseCommandProcessor==true`; `ToolTask` writes out .cmd file with a BOM that cmd.exe hates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants