Feedback from scanning https://github.com/oss-review-toolkit/ort/tree/main/analyzer/src/funTest/assets/projects/synthetic/dotnet/subProjectTest

[pombredanne]

I received this report:

While scanning https://raw.githubusercontent.com/oss-review-toolkit/ort/236923c66aa73dd11fcaa77a8d89e77788ef71b6/analyzer/src/funTest/assets/projects/synthetic/dotnet/subProjectTest/test.csproj some questions came up.

The nuget-inspector was launched on Ubuntu 22.04 this way, using also a checkout of ORT:

./nuget-inspector –verbose \
   --project-file ort/analyzer/src/funTest/assets/projects/synthetic/dotnet/subProjectTest/test.csproj \
   --nuget-config ort/analyzer/src/funTest/assets/projects/synthetic/dotnet/nuget.config \
   --json nuget-inspector-result.json.txt

The output is this file nuget-inspector-result.json.txt

Packages detected by the nuget-inspector

(Note: the difference between packages and dependencies is everything but clear, hence both are listed)

$ cat nuget-inspector-result.json | jq ".packages[].packages[].package.purl"
"pkg:nuget/System.Globalization@4.3.0"
"pkg:nuget/System.Threading@4.0.11"
"pkg:nuget/Antlr@3.4.1.9004"
"pkg:nuget/Newtonsoft.Json@5.0.4"
"pkg:nuget/WebGrease@1.5.2"
"pkg:nuget/foobar@1.2.3"
"pkg:nuget/System.Threading.Tasks.Extensions@4.5.4"

$ cat nuget-inspector-result.json | jq ".packages[].dependencies[].purl"
"pkg:nuget/System.Globalization@4.3.0"
"pkg:nuget/System.Threading@4.0.11"
"pkg:nuget/WebGrease@1.5.2"
"pkg:nuget/foobar@1.2.3"
"pkg:nuget/System.Threading.Tasks.Extensions@4.5.4"

Packages in the expected ort result

$ cat dotnet-expected-output.yml | yq ".packages[].id"
NuGet::Antlr:3.4.1.9004
NuGet::Microsoft.NETCore.Platforms:1.0.1
NuGet::Microsoft.NETCore.Platforms:1.1.0
NuGet::Microsoft.NETCore.Targets:1.0.1
NuGet::Microsoft.NETCore.Targets:1.1.0
NuGet::Newtonsoft.Json:5.0.4
NuGet::System.Collections:4.3.0
NuGet::System.Globalization:4.3.0
NuGet::System.Runtime:4.1.0
NuGet::System.Runtime:4.3.0
NuGet::System.Runtime.CompilerServices.Unsafe:4.5.3
NuGet::System.Threading:4.0.11
NuGet::System.Threading.Tasks:4.0.11
NuGet::System.Threading.Tasks:4.3.0
NuGet::System.Threading.Tasks.Extensions:4.5.4
NuGet::WebGrease:1.5.2

Issues and questions

1. The nuget-inspector currently doesn’t seem to support projects with a packages.config file, like this one: https://github.com/oss-review-toolkit/ort/tree/main/analyzer/src/funTest/assets/projects/synthetic/nuget

2. There is no metadata listed at all in the nuget-inspector-result.json. This metadata would be required to replace the current NuGet implementation in ORT.

3. TargetFrameworks seem to be ignored by the nuget-inspector. In the output you can see that System.Threading.Tasks.Extensions and System.Globalization are both listed. We need some way to tell the nuget-inspector which TargetFramework should be taken into account.

4. The foobar@1.2.3 package is listed as a valid dependency, while it is explicitly in this test to check if the package manager fails if there is a non-resolvable package defined.

5. Some transitive dependencies are missing in the output of the nuget-inspector. For instance Microsoft.NETCore.Platforms and System.Threading.Tasks.

[pombredanne]

1. The nuget-inspector currently doesn’t seem to support projects with a packages.config file, like this one: https://github.com/oss-review-toolkit/ort/tree/main/analyzer/src/funTest/assets/projects/synthetic/nuget

nuget-inspector does support packages.config files alright, but the input is not a packages.config file directly, it is instead a *.*proj file aka. "projet file". If a package.config is present side-by-side it will used.

A packages.config (now mostly replaced by PackageReference in a "projet file" does not exists exist alone but is always a companion to a project file to the best of my knowledge. So if you have a .Net project that uses the "legacy" packages.config file, it will be used.
We have several tests for this with these files.
See https://github.com/search?q="packages"+repo%3AnexB%2Fnuget-inspector+filename%3Apackages.config+language%3AXML+language%3AXML&type=Code&ref=advsearch&l=XML&l=XML for some examples.

2. There is no metadata listed at all in the nuget-inspector-result.json. This metadata would be required to replace the current NuGet implementation in ORT.

That's correct. We do not return metadata for now, we only resolve the
dependency tree. We should add metadata. See #2 for tracking this feature

3. TargetFrameworks seem to be ignored by the nuget-inspector. In the output you can see that System.Threading.Tasks.Extensions and System.Globalization are both listed. We need some way to tell the nuget-inspector which TargetFramework should be taken into account.

For now the target framework used should be the first of the target(s)
declared in the properties of the project, but we should expose this
as a command line instead.
See #4 for tracking this feature

4. The foobar@1.2.3 package is listed as a valid dependency, while it is explicitly in this test to check if the package manager fails if there is a non-resolvable package defined.

We are not returning errors for now for these cases indeed. We should. What
should be the preferred behaviour? Just error out and fail? This is tracked now in #5

5. Some transitive dependencies are missing in the output of the nuget-inspector. For instance Microsoft.NETCore.Platforms and System.Threading.Tasks.

The test project file has these declared targets:
<TargetFrameworks>net45;netcoreapp3.1</TargetFrameworks>

Based on the current logic of using the first declared target framework, we would pick net45 as a target; therefore Microsoft.NETCore.Platforms and System.Threading.Tasks would not be resolved with net45 as a target. But they would be resolved in the transitive
closure for a netcoreapp3.1 target. You can test this by editing the
csproj with <TargetFrameworks>netcoreapp3.1</TargetFrameworks>
See the results below:

• net45-target.json.txt
• netcoreapp3.1-target.json.txt

You can see the diff there https://gist.github.com/pombredanne/7e6be90c24c44c6438dfc264bd34ac67/revisions

This is closely relate top the target feature. See #4 for tracking this feature

[pombredanne]

I consider this all done now. I am closing.