-
Notifications
You must be signed in to change notification settings - Fork 735
Dotnet list package json output implementation #4855
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
Merged
Merged
Changes from all commits
Commits
Show all changes
54 commits
Select commit
Hold shift + click to select a range
100a5b4
Add output format and output version
erdembayar d011047
Add JsonRenderer
erdembayar 855288c
Add v1 of JsonRenderer
erdembayar 83593de
Add v1 of JsonRenderer
erdembayar 9fd3da0
Replace console.write with generic IRenderer.Write
erdembayar 1a24aed
Start implementing JsonRenderer
erdembayar 8dee65f
Introduce non-zero return value from dotnet list package
erdembayar ccfeacd
Replace all Console instance with Renderer
erdembayar eb2894d
Start implementing JsonRenderer
erdembayar 6d4e553
Introduce column naming enum
erdembayar 3da53b5
Separate Vulnurability info
erdembayar 5fae241
Fix vulnerability data column after refactoring
erdembayar 4b3612e
Pass data for JsonRendering
erdembayar a80006b
Fix test
erdembayar ad603e3
Basic json generation
erdembayar d8270e4
Fix basic json output
erdembayar 9408c57
Clean up
erdembayar 8b4d969
Remove check if JsonRenderer
erdembayar fb49a3b
Abstract write project data
erdembayar b46ba56
Clean up
erdembayar 8dcd633
Make JsonRenderer more abstract for possible feature extension for ot…
erdembayar c840abf
Error on bad version.
erdembayar ebbb756
Separate data model from business logic for dotnet list package command
erdembayar c77cb23
Json output deprecation data
erdembayar ee7defd
Add problem printing for json output
erdembayar 2ee4340
Reimplement print functionality for dotnet list package console output
erdembayar 766481a
Print AutoReferenceFound flag
erdembayar 8790b8c
Fix arguments for json output
erdembayar ab1ec6a
Simplify namespace
erdembayar 41950ba
Clean up
erdembayar 6b8c764
Return failure code 1 if there is problem when running dotnet list pa…
erdembayar c60eb03
Add XPlat list package unit tests
erdembayar 1e111fe
Clean up
erdembayar eb4ec17
Handle http warning edge case
erdembayar 03db2dc
Fix formatting
erdembayar 0b557a7
Address PR feedback
erdembayar 6bca799
Rename methods
erdembayar c88d256
Address PR comments
erdembayar 7691366
Address latest PR feedbacks
erdembayar aec4dbe
Fix handling of invalid output version.
erdembayar fa02e20
Address PR comment
erdembayar e081b66
Merge ListPackageJsonRenderer.cs and ListPackageJsonRendererV1.cs files
erdembayar 2d98be1
Removed used vars
erdembayar 805cf06
Address PR comment about not useful inline comment.
erdembayar 2a89fde
Address latest comment about problem level.
erdembayar fb4c2bd
Remove Information enum from ProblemType enums.
erdembayar abb5159
MSBuildAPIUtility.GetResolvedVersions shouldn't print anything to con…
erdembayar 9374c1c
Remove LoggerWarning type
erdembayar 345c354
In case of error need to stop processing for Console.Output
erdembayar 86650d0
Make ListReportPackage immutable
erdembayar ff6b93b
Fix failing test
erdembayar ce33770
Make logger 1
erdembayar 96524d5
Remove duplicate jsonserialization
erdembayar d0fbf0c
Remove unnecessary file
erdembayar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no way to get all the arguments out of the parser instead of us buildiing the arguments ourselves?
My concern is that if/when we add a new option, it'd be super easy to miss that the argument text needs updating.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In command argument/option parse we manually determine what to care about, so currently I don't know any other easy way to doing this.
We can get unprocessed arguments from here.
NuGet.Client/src/NuGet.Core/NuGet.CommandLine.XPlat/Program.cs
Line 101 in 41c08c5
It looks something like below, it still needs processing to make it look like
list --include-transitive:list --format json C:\Users\...\MyProjectD\MyProjectD.csproj --include-transitiveThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. You actually want to trim down the argument.
I guess it makes sense in that case.
I'm wondering if there's a way to ensure that we don't forget to add this in a future through tests.
Maybe use some reflection?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to check it simple way using reflection.
Could please you check new
JsonRenderer_ListPackageArgse_Verify_AllFields_Coveredtest in test\NuGet.Core.FuncTests\NuGet.XPlat.FuncTest\ListPackageTests.cs ? Do you think is it enough?https://github.com/NuGet/NuGet.Client/pull/4855/files#diff-9aff685d365ad24a46596f586a60c5ff0d23a53c1daf4fe275b21ad64d1fd898R191
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me.
If you're motivated, you can even add something where you create an args list and make sure all of the relevant ones are included in the string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not motivated to add more test for this moment but added reminder in follow up issue #12167.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviving this thread because I think I have relevant new information to add:
System.Environment.CommandLine
A complication is due to the way that the dotnet cli works. From the operation system's point of view, the command run is
dotnet c:\path\to\NuGet.CommandLine.XPlat.dll package list --other --args, so the value will need to be cleaned up a bit to remove everything before the first package list, and also take into account when the command line passes a project/solution path.Also, looking at the spec PR, I don't see where any customers have asked for the CLI arguments to be written to the json output. Since customers control how
dotnet list packageis called, from my point of view, they already know the arguments, or at least can look it up in their pipeline scripts. Is this an actually useful feature, or is it just adding technical debt for little benefit?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I saw command line args, but that one needs more manual processing than this one.
Intention for having parameters section is we keep this json output somewhere as archive for safe keeping and having input information is useful later because many CI pipelines runs get deleted after some time.
That time it's very hard to tell if json output is from
dotnet list package --vulnerableordotnet list package --deprecatedcommand. They assumed it was fordotnet list package --vulnerable(check below example) and there was nothing in json output, but actually if it wasdotnet list package --deprecatedthen it misleads the someone looking at report.