packagedcode: add DotNetDepsJsonHandler to parse .deps.json files#4803
packagedcode: add DotNetDepsJsonHandler to parse .deps.json files#4803kumarasantosh wants to merge 2 commits intoaboutcode-org:developfrom
Conversation
Add support for parsing NuGet .deps.json lockfiles which are present alongside .dll and .pdb files in .NET projects and NuGet packages. Extracts package names, versions, types and dependencies from the libraries section of .deps.json files. Fixes aboutcode-org#4496 Signed-off-by: kumarasantosh <santosh.pulikond02@gmail.com>
kumarasantosh
force-pushed
the
feature/deps-json-handler
branch
from
09f8534 to
98766fe
Compare
AyanSinhaMahapatra
left a comment
AyanSinhaMahapatra
left a comment
There was a problem hiding this comment.
@kumarasantosh thank you for the PR.
See comments for feedback, some major changes are required, and please apply the same to your other PRs :)
| @@ -0,0 +1,23 @@ | |||
| { | |||
There was a problem hiding this comment.
Please add the source link/download URL for this file as a comment, we need to know where the test data is located/comes from.
| if not selected_targets: | ||
| selected_targets = available_targets | ||
|
|
||
| for lib_key, lib_info in libraries.items(): |
There was a problem hiding this comment.
Can you extract the parsing of dependencies in another function.
| with open(location) as loc: | ||
| try: | ||
| parsed = json.load(loc) | ||
| except Exception: |
There was a problem hiding this comment.
Not required, exception handling is handled in upstream code.
| "hashPath": "wpfanalyzers.4.1.1.nupkg.sha512" | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
We need the full scan results to verify everything works correctly.
Also this file is quite large, can you keep only a subset of dependencies in here.
Be careful as there are a few parallal lists, libraries and targets which you need to merge, so make sure you test merging data and consistency
There was a problem hiding this comment.
Finding an additional smaller test case and adding that helps too
| is_optional=False, | ||
| is_pinned=True, | ||
| is_direct=True, | ||
| ).to_dict() |
There was a problem hiding this comment.
resolved_package needs to be populated as these are runtime packages which are resolved to versions.
|
@AyanSinhaMahapatra , thanks for the detailed review - really helpful feedback. I've gone through all the comments and here's what's been updated: Removed the local try/except around json.load since you mentioned upstream already handles parse errors. Pulled the dependency parsing out into its own helper function to clean up the main method. Populated resolved_package on the runtime dependencies. On the test side, moved everything into the existing test_nuget.py rather than a separate file. Trimmed the Snoop fixture down to a smaller consistent subset, regenerated the expected scan output, and added a second smaller real-world fixture as well. Added .ABOUT files to document where each fixture came from — simple.deps.json is hand-crafted and is noted as such. Also added tests for the target fallback behaviour and multi-target merge consistency. Please take another look when you get a chance — happy to make further changes! |
Signed-off-by: kumarasantosh <santosh.pulikond02@gmail.com>
kumarasantosh
force-pushed
the
feature/deps-json-handler
branch
from
4b404fd to
0708369
Compare
2 participants
Fixes #4496
NuGet .deps.json files appear alongside .dll and .pdb files in .NET
projects but were not being parsed by ScanCode. This PR adds a new
DotNetDepsJsonHandler in nuget.py to handle these files.
The handler detects *.deps.json files, reads the libraries section
and extracts package names, versions, types and their dependencies.
Target framework is stored in extra_data.
Tested against a real world fixture from snoopwpf v5.1.0 release
and a minimal hand-crafted fixture for edge cases.
Tasks