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

Update support libraries to those that are only lib. #1582

Merged
merged 1 commit into from
Sep 14, 2017

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Sep 12, 2017

@dsplaisted you may want to take this and create your own PR. Not sure if its complete.

@dsplaisted
Copy link
Member

@ericstj Looks like this breaks the test that is supposed to check that the facades aren't deployed once netstandard.dll is inbox. That test has some hacky workarounds, so I'm not sure if the issue is with the hacky workarounds or whether there's actually a break with the new package.

@ericstj
Copy link
Member Author

ericstj commented Sep 13, 2017

Got it, I need to update the test.

Copy link
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

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

Can you link to the corresponding issues and PRs with more of the context for this change?

And could you also fill out the approval template for this, which you can see here?

@ericstj
Copy link
Member Author

ericstj commented Sep 14, 2017

Customer scenario

Customer had desktop project referencing netstandard1.5 or later assembly. In their solution they have a web project

Bugs this fixes

#1509

Related Issues: https://github.com/dotnet/corefx/issues/23229, #1522, https://github.com/dotnet/corefx/issues/23505, dotnet/corefx#23711

Workarounds, if any

To all desktop project files add the following:

  <Target Name="ReplaceNetFxNetStandardRefWithLib" AfterTargets="ImplicitlyExpandNETStandardFacades">
    <ItemGroup>
      <Reference Remove="@(_NETStandardLibraryNETFrameworkReference)" Condition="'%(FileName)' != 'netfx.force.conflicts'"/>
      <Reference Include="@(_NETStandardLibraryNETFrameworkLib)">
        <Private>true</Private>
      </Reference>
    </ItemGroup>
  </Target>
  <Target Name="RemoveNetFxForceConflicts" AfterTargets="ResolveAssemblyReferences">
    <ItemGroup>
      <ReferencePath Remove="@(ReferencePath)" Condition="'%(FileName)' == 'netfx.force.conflicts'" />
    </ItemGroup>
  </Target>

Risk

Low

Performance impact

Improves performance by removing a number of items and wildcard expansion (further changes aim to improve more).

Root cause analysis

Web project system ignores CopyLocal metadata on references and will copy them to the output directory outside of the build if the file is absent. This breaks if any assembly is intentionally CopyLocal=false (AKA: Private=false), and a ReferenceAssembly without a corresponding implementation in the GAC. It will be copied to the output directory and later loaded for execution by ASP.NET.

XAML compiler attempts to load all references for execution and fails for any that are ReferenceAssemblies without a corresponding implementation in the GAC.

How was the bug found?

Customer reports

@MattGertz for approval

@ericstj ericstj merged commit 0c0ffce into dotnet:release/2.0-vs Sep 14, 2017
@ericstj
Copy link
Member Author

ericstj commented Sep 14, 2017

FYI this will likely need to be updated once CoreFx has a final 2.0.2 servicing build /cc @weshaggard

@emmanuelguerin
Copy link

It is possible that this PR breaks a build in the following context?

  • a .NET Standard 2.0 targeted library.
  • a .NET Framework 4.6.1 targeted library (not using the Microsoft.NET.Sdk) that references the other library.

With this patch, the netstandard.dll doesn't get added to the compilation step, and the build fails.
See this repo: https://github.com/emmanuelguerin/issue-sgen for an example.

@mouadhtrabelsi
Copy link

@emmanuelguerin do your patch work for .NET Framework 4.7.1 ?
i added
<Target Name="ImplicitlyExpandNETStandardFacades" ..>
..

to my csproj , but did not fix the problem,

I get this error
error : An attempt was made to load an assembly with an incorrect format: C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework.NETFramework\v4.7.1\Facades\System.IO.Compression.ZipFile.dll.

But i m not referincing this DLL in my project !

any help please ?

JL03-Yue pushed a commit that referenced this pull request Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants