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

Include transitive closure of dependencies in package #261

Closed
AArnott opened this issue May 6, 2021 · 8 comments · Fixed by #262 or #265
Closed

Include transitive closure of dependencies in package #261

AArnott opened this issue May 6, 2021 · 8 comments · Fixed by #262 or #265
Labels
bug Something isn't working

Comments

@AArnott
Copy link
Member

AArnott commented May 6, 2021

...sticking a debugger onto the Omnisharp process, it appears the generator is throwing an exception. We also seem to be eating the exception rather than reporting it, so we'll need to fix that! But the exception is:

Could not load file or assembly 'System.Numerics.Vectors, Version=4.1.3.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. The system cannot find the file specified.

Which seems to point to the first problem: the generator package is packaging dependencies like System.Text.Json, but isn't packaging dependencies of System.Text.Json (namely System.Numerics.Vectors):

<Target Name="PackBuildOutputs" DependsOnTargets="ResolveProjectReferences;SatelliteDllsProjectOutputGroup;SatelliteDllsProjectOutputGroupDependencies">
<ItemGroup>
<!-- Analysis of C# projects -->
<TfmSpecificPackageFile Include="$(TargetPath)" PackagePath="analyzers\cs\" />
<TfmSpecificPackageFile Include="@(ReferencePath)" PackagePath="analyzers\cs\" Condition=" '%(FileName)%(Extension)' == 'YamlDotNet.dll' " />
<TfmSpecificPackageFile Include="@(ReferencePath)" PackagePath="analyzers\cs\" Condition=" '%(FileName)%(Extension)' == 'Microsoft.Bcl.AsyncInterfaces.dll' " />
<TfmSpecificPackageFile Include="@(ReferencePath)" PackagePath="analyzers\cs\" Condition=" '%(FileName)%(Extension)' == 'System.Text.Json.dll' " />
<TfmSpecificPackageFile Include="@(ReferencePath)" PackagePath="analyzers\cs\" Condition=" '%(FileName)%(Extension)' == 'System.Text.Encodings.Web.dll' " />
<TfmSpecificPackageFile Include="@(SatelliteDllsProjectOutputGroupDependency)" PackagePath="analyzers\cs\%(SatelliteDllsProjectOutputGroupDependency.DestinationSubDirectory)" Condition=" '%(SatelliteDllsProjectOutputGroupDependency.DestinationSubDirectory)' != '' " />
<TfmSpecificPackageFile Include="@(SatelliteDllsProjectOutputGroupOutput->'%(FinalOutputPath)')" PackagePath="analyzers\cs\%(SatelliteDllsProjectOutputGroupOutput.Culture)\" />
<TfmSpecificPackageFile Include="%(_ResolvedProjectReferencePaths.Identity)" PackagePath="analyzers\cs\" />
</ItemGroup>
</Target>

So that probably should be updated to copy in the dependency.

Originally posted by @jasonmalinowski in #218 (reply in thread)

@AArnott AArnott added the bug Something isn't working label May 6, 2021
@AraHaan
Copy link
Contributor

AraHaan commented May 7, 2021

I think what someone needs to do is walk the dependency trees to all of the dependencies of CsWin32 and then copy them over.

Also I think the IsExternalInit.cs file can be nuked for the nuget package of the same name that provides that code for the generator.

Ironically I have not figured out how you guys got the init; on your properties to work with System.Test.Json with language version of latest yet I try it on my projects and the unit tests fail.

@AraHaan
Copy link
Contributor

AraHaan commented May 7, 2021

Also After looking in Rider don't we also got to include:

  • System.Threading.Tasks.Extensions (dependency of the included Microsoft.Bcl.AsyncInterfaces which is dependency of System.Text.Json)
  • System.Memory (dependency of System.Text.Json and other dependencies as well)
  • System.Buffers (dependency of System.Memory and others)
  • System.Runtime.CompilerServices.Unsafe (dependency of System.Text.Json and others)

I will see if I can work out a patch.

@AraHaan
Copy link
Contributor

AraHaan commented May 14, 2021

uh did the bot just remove the commit I made? It does not look to be anywhere in the main branch currently.

@AArnott
Copy link
Member Author

AArnott commented May 15, 2021

Yes, it appears since your PR source branch was main that your bot force-pushed your main branch to match ours, just before I completed your PR, so there was nothing there. I suggest you create your PRs from a branch with another name.

@AraHaan
Copy link
Contributor

AraHaan commented May 19, 2021

Since this is technically a fix to a major issue should a new nuget.org version be pushed?

@AArnott
Copy link
Member Author

AArnott commented May 20, 2021

Yes, I'll be pushing a new one out soon. I want folks to try out tonight's CI build (the one that includes #270) before I push to nuget.org in case there is important feedback to account for.

@AraHaan
Copy link
Contributor

AraHaan commented May 20, 2021

ah ok

@AArnott
Copy link
Member Author

AArnott commented Jan 6, 2023

This 'fix' was ultimately reverted in #843 to solve #836. I found that Omnisharp and Rider still work after this change.

AArnott added a commit that referenced this issue Mar 20, 2024
Extend default timeout for APIScan job
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants