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

Build fails when returning type from PackageReference #2344

Open
timcassell opened this issue Jun 27, 2023 · 7 comments · Fixed by #2347 · May be fixed by #2508
Open

Build fails when returning type from PackageReference #2344

timcassell opened this issue Jun 27, 2023 · 7 comments · Fixed by #2347 · May be fixed by #2508
Assignees

Comments

@timcassell
Copy link
Collaborator

timcassell commented Jun 27, 2023

I added a nuget package.

<PackageReference Include="ProtoPromise" Version="2.5.4" />

Created a benchmark to return a type from that package.

[Benchmark] public CancelationToken CanceledToken() => CancelationToken.Canceled();

And the benchmark project build failed.

C:\Users\Tim\Documents\git\ProtoPromiseBenchmarks\bin\Release\net7.0\a07bed7c-cb6d-4d10-b849-74756c9d53ac\a07bed7c-cb6d-4d10-b849-74756c9d53ac.notcs(342,55): error CS0246: The type or namespace name 'Proto' could not be found (are you missing a using directive or an assembly reference?) [C:\Users\Tim\Documents\git\ProtoPromiseBenchmarks\bin\Release\net7.0\a07bed7c-cb6d-4d10-b849-74756c9d53ac\BenchmarkDotNet.Autogenerated.csproj]

Is there any way to get it to work?

[Edit] Note: it works when I use the types in my benchmarks, but when I return a type, it breaks.

@adamsitnik
Copy link
Member

adamsitnik commented Jun 27, 2023

Is the generated code using a fully qualified type name (#1009) when referring to CancelationToken?

@timcassell
Copy link
Collaborator Author

Is the generated code using a fully qualified type name (#1009) when referring to CancelationToken?

Yes, I checked the .notcs file, it's Proto.Promises.CancelationToken.

@AndreyAkinshin
Copy link
Member

This bug is specific to ProtoPromise. Let's create a new project with the following package references:

    <ItemGroup>
        <PackageReference Include="Newtonsoft.Json" Version="13.0.3"/>
        <PackageReference Include="ProtoPromise" Version="2.5.4"/>
    </ItemGroup>

Here is the relevant fragment of obj/project.assets.json:

      "Newtonsoft.Json/13.0.3": {
        "type": "package",
        "compile": {
          "lib/net6.0/Newtonsoft.Json.dll": {
            "related": ".xml"
          }
        },
        "runtime": {
          "lib/net6.0/Newtonsoft.Json.dll": {
            "related": ".xml"
          }
        }
      },
      "ProtoPromise/2.5.4": {
        "type": "package",
        "build": {
          "build/net6.0/ProtoPromise.targets": {}
        }
      }

As we can see, Newtonsoft.Json adds a reference to Newtonsoft.Json.dll in the compile section. This approach is the default one for most NuGet packages. A great thing about this approach is transitivity: if we create a second project which has a project reference to the first project, the second project will have a transitive reference to Newtonsoft.Json.

However, ProtoPromise expose references to dll not via the default mechanism, but via a handmade targets file. This file is not exposed to other projects by default. Thus, the second project will not get ProtoPromise.targets and therefore will not get a transitive reference to ProtoPromise.dll. If we want to locally workaround this problem, we have to change the package reference declaration for the first project to

<PackageReference Include="ProtoPromise" Version="2.5.4" PrivateAssets="none"  />

With PrivateAssets="none" in the first project csproj, the second project will receive the targets file and therefore a transitive reference to ProtoPromise.dll.

Conclusion: by default, BenchmarkDotNet correctly processes transitive dependencies for most NuGet packages. However, in some corner cases (as described above), the generated project doesn't inherit some of the references that are hidden for transitive resolving because of the PrivateAssets. It seems that the approach presented in 2347 is a reasonable workaround for such corner cases.

@AndreyAkinshin AndreyAkinshin added this to the v0.13.6 milestone Jul 3, 2023
@timcassell
Copy link
Collaborator Author

timcassell commented Jul 17, 2023

Reopening since copying PackageReference was reverted in #2365. #1403 should have a proper fix.

@timcassell timcassell reopened this Jul 17, 2023
@adamsitnik
Copy link
Member

Conclusion: by default, BenchmarkDotNet correctly processes transitive dependencies for most NuGet packages. However, in some corner cases (as described above), the generated project doesn't inherit some of the references that are hidden for transitive resolving because of the PrivateAssets.

If we are not able to find a more reliable solution we should mark it as by design and close. Thank you for a detailed investigation!

@timcassell
Copy link
Collaborator Author

If we are not able to find a more reliable solution we should mark it as by design and close.

I believe when #1403 is solved it will solve this as well. So it can be linked and closed at the same time as the other linked issues.

@adamsitnik
Copy link
Member

I believe when #1403 is solved it will solve this as well. So it can be linked and closed at the same time as the other linked issues.

Sounds reasonable to me 👍

@timcassell timcassell removed this from the v0.13.6 milestone Jul 17, 2023
@timcassell timcassell linked a pull request Jan 23, 2024 that will close this issue
@timcassell timcassell self-assigned this Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants