Skip to content

fix(nuget): add nuget dependency extract #3121

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

fix(nuget): add nuget dependency extract #3121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a05670b
fix(nuget): add nuget dependency extract
bloodhoundGit Oct 6, 2022
a228078
feat(nuget): add dependency check for inside group tags
bloodhoundGit Nov 18, 2022
f370581
fix(nuget): match updated develop code
bloodhoundGit Nov 18, 2022
fc2d36a
fix(nuget): update nuget test
bloodhoundGit Nov 22, 2022
60a14ec
fix(nuget): add dependency section in bootstrap test
bloodhoundGit Nov 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 90 additions & 5 deletions src/packagedcode/nuget.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,92 @@
from packagedcode import models
from packagedcode.utils import build_description


"""
Handle NuGet packages and their manifests.
"""
# TODO: add dependencies

# TODO: Add section to handle dependencies of dependencies
Copy link
Member

@pombredanne pombredanne Nov 22, 2022

Choose a reason for hiding this comment

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

For this you may want to create a separate issue... TODO comments (that I tend to abuse) eventually get lost.
Note also that for a full, dynamic dependency resolution of nugets. there is this new project at https://github.com/nexB/nuget-inspector

Copy link

@jaitaiwan jaitaiwan Nov 22, 2022

Choose a reason for hiding this comment

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

I say the same thing about TODO's internally 😂 That nuget-inspector proejct looks promising.



def get_dependencies(nuspec):
"""
Return a list of Dependent package objects found in a NuGet `nuspec` object.
"""
dependencies = []
try:
if "dependencies" in nuspec:
if "group" in nuspec["dependencies"]:
for group in nuspec["dependencies"]["group"]:
if "dependency" in group:
for dependency in group["dependency"]:
#dependencies.append(dependency)
dpurl = models.PackageURL(
type='nuget',
namespace=None,
name=dependency.get("@id"),
version=dependency.get("@version"),
Copy link
Member

@pombredanne pombredanne Nov 22, 2022

Choose a reason for hiding this comment

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

You can only put a version here if this is not a range so IMHO you would have to test for this.

qualifiers=None
)
dep_pack = models.DependentPackage(
purl=str(dpurl),
extracted_requirement=dependency.get("@version"),
scope="dependency",
Copy link
Member

@pombredanne pombredanne Nov 22, 2022

Choose a reason for hiding this comment

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

Would not each dependency group become a scope here?

is_runtime=False,
is_optional=False,
is_resolved=True,
Copy link
Member

@pombredanne pombredanne Nov 22, 2022

Choose a reason for hiding this comment

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

A dependency is resolved if an only if we have a concrete version and not a range. Here the version is optional AFAIK and could be a range?

)

dependencies.append(dep_pack)

if "dependency" in nuspec.get("dependencies"):
Copy link
Member

@pombredanne pombredanne Nov 22, 2022

Choose a reason for hiding this comment

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

I would prefer something like this, and then reuse the nuget_dependency var below:

Suggested change
if "dependency" in nuspec.get("dependencies"):
nuget_dependencies = nuspec.get("dependencies") or {}
nuget_dependency = nuget_dependencies.get("dependency")
if nuget_dependency:

Copy link

@jaitaiwan jaitaiwan Nov 22, 2022

Choose a reason for hiding this comment

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

Out of curiosity, any reason for or {} over get("...", {})?

Copy link
Member

@pombredanne pombredanne Nov 22, 2022

Choose a reason for hiding this comment

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

@jaitaiwan re:

Out of curiosity, any reason for or {} over get("...", {})?

This is an idiom I like to use to make sure that I am getting a proper empty dict in all cases even if the get returns None or some emptyish value.
Getting an empty dict when you expect a dict enable chaining the next call without requiring an extra check for empties.

>>> repr({1: None, 2: "", 3: {"bar": "baz"}}.get(1, {}))
'None'
>>> repr({1: None, 2: "", 3: {"bar": "baz"}}.get(1, {}) or {})
'{}'
>>> repr({1: None, 2: "", 3: {"bar": "baz"}}.get(1) or {})
'{}'

Copy link

@jaitaiwan jaitaiwan Nov 22, 2022

Choose a reason for hiding this comment

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

Fair call. thanks!

if "@id" and "@version" in nuspec.get("dependencies").get("dependency"):
dpurl = models.PackageURL(
type='nuget',
namespace=None,
name=nuspec.get("dependencies").get(
"dependency").get("@id"),
version=nuspec.get("dependencies").get(
"dependency").get("@version"),
qualifiers=None
)
dep_pack = models.DependentPackage(
purl=str(dpurl),
extracted_requirement=nuspec.get("dependencies").get(
"dependency").get("@version"),
Copy link
Member

@pombredanne pombredanne Nov 22, 2022

Choose a reason for hiding this comment

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

Factor version here and above in a variable.

scope="dependency",
is_runtime=False,
is_optional=False,
is_resolved=True,
Copy link
Member

@pombredanne pombredanne Nov 22, 2022

Choose a reason for hiding this comment

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

Same comment as above... resolve is True IFF we have a concrete pinned, non-empty version

)

dependencies.append(dep_pack)

else:
for dependency in nuspec.get("dependencies").get("dependency"):
dpurl = models.PackageURL(
type='nuget',
namespace=None,
name=dependency.get("@id"),
version=dependency.get("@version"),
qualifiers=None
)
dep_pack = models.DependentPackage(
purl=str(dpurl),
extracted_requirement=dependency.get(
"@version"),
scope="dependency",
is_runtime=False,
is_optional=False,
is_resolved=True,
)
dependencies.append(dep_pack)

return dependencies

except Exception as e:
print(e)
Copy link
Member

@pombredanne pombredanne Nov 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Please do not print exceptions. In general your should avoid to wrap you code in a try/except altogether as this can never be debugged this way. Let the exception bubble up and we will catch it where needed, and worse case at the top level.

return dependencies


def get_urls(name, version, **kwargs):
Expand Down Expand Up @@ -46,7 +128,7 @@ class NugetNuspecHandler(models.DatafileHandler):

@classmethod
def parse(cls, location):
with open(location , 'rb') as loc:
with open(location, 'rb') as loc:
parsed = xmltodict.parse(loc)

if not parsed:
Expand All @@ -62,14 +144,15 @@ def parse(cls, location):

# Summary: A short description of the package for UI display. If omitted, a
# truncated version of description is used.
description = build_description(nuspec.get('summary') , nuspec.get('description'))
description = build_description(
Copy link
Member

@pombredanne pombredanne Nov 22, 2022

Choose a reason for hiding this comment

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

Why wrapping this on tow lines? I am not sure this helps with readability.

nuspec.get('summary'), nuspec.get('description'))

# title: A human-friendly title of the package, typically used in UI
# displays as on nuget.org and the Package Manager in Visual Studio. If not
# specified, the package ID is used.
title = nuspec.get('title')
if title and title != name:
description = build_description(nuspec.get('title') , description)
description = build_description(nuspec.get('title'), description)

parties = []
authors = nuspec.get('authors')
Expand Down Expand Up @@ -102,7 +185,7 @@ def parse(cls, location):
# Deprecated and not a license expression, just a URL
elif 'licenseUrl' in nuspec:
extracted_license_statement = nuspec.get('licenseUrl')

yield models.PackageData(
datasource_id=cls.datasource_id,
type=cls.default_package_type,
Expand All @@ -111,8 +194,10 @@ def parse(cls, location):
description=description or None,
homepage_url=nuspec.get('projectUrl') or None,
parties=parties,
dependencies=get_dependencies(nuspec),
extracted_license_statement=extracted_license_statement,
copyright=nuspec.get('copyright') or None,
vcs_url=vcs_url,
**urls,
)

103 changes: 102 additions & 1 deletion tests/packagedcode/data/nuget/Castle.Core.nuspec.json.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,108 @@
"source_packages": [],
"file_references": [],
"extra_data": {},
"dependencies": [],
"dependencies": [
{
"purl": "pkg:nuget/[email protected]",
"extracted_requirement": "1.6.1",
"scope": "dependency",
"is_runtime": false,
"is_optional": false,
"is_resolved": true,
"resolved_package": {},
"extra_data": {}
},
{
"purl": "pkg:nuget/[email protected]",
"extracted_requirement": "4.3.0",
"scope": "dependency",
"is_runtime": false,
"is_optional": false,
"is_resolved": true,
"resolved_package": {},
"extra_data": {}
},
{
"purl": "pkg:nuget/[email protected]",
"extracted_requirement": "4.3.0",
"scope": "dependency",
"is_runtime": false,
"is_optional": false,
"is_resolved": true,
"resolved_package": {},
"extra_data": {}
},
{
"purl": "pkg:nuget/[email protected]",
"extracted_requirement": "4.3.0",
"scope": "dependency",
"is_runtime": false,
"is_optional": false,
"is_resolved": true,
"resolved_package": {},
"extra_data": {}
},
{
"purl": "pkg:nuget/[email protected]",
"extracted_requirement": "4.3.0",
"scope": "dependency",
"is_runtime": false,
"is_optional": false,
"is_resolved": true,
"resolved_package": {},
"extra_data": {}
},
{
"purl": "pkg:nuget/[email protected]",
"extracted_requirement": "4.3.0",
"scope": "dependency",
"is_runtime": false,
"is_optional": false,
"is_resolved": true,
"resolved_package": {},
"extra_data": {}
},
{
"purl": "pkg:nuget/[email protected]",
"extracted_requirement": "4.3.0",
"scope": "dependency",
"is_runtime": false,
"is_optional": false,
"is_resolved": true,
"resolved_package": {},
"extra_data": {}
},
{
"purl": "pkg:nuget/[email protected]",
"extracted_requirement": "4.3.0",
"scope": "dependency",
"is_runtime": false,
"is_optional": false,
"is_resolved": true,
"resolved_package": {},
"extra_data": {}
},
{
"purl": "pkg:nuget/[email protected]",
"extracted_requirement": "4.3.0",
"scope": "dependency",
"is_runtime": false,
"is_optional": false,
"is_resolved": true,
"resolved_package": {},
"extra_data": {}
},
{
"purl": "pkg:nuget/[email protected]",
"extracted_requirement": "4.3.0",
"scope": "dependency",
"is_runtime": false,
"is_optional": false,
"is_resolved": true,
"resolved_package": {},
"extra_data": {}
}
],
"repository_homepage_url": "https://www.nuget.org/packages/Castle.Core/4.2.1",
"repository_download_url": "https://www.nuget.org/api/v2/package/Castle.Core/4.2.1",
"api_data_url": "https://api.nuget.org/v3/registration3/castle.core/4.2.1.json",
Expand Down
Loading