feat: add --print-graph-with-errors flag to return complete dep-graphs as JSONL
#6477
Conversation
thomasschafer
force-pushed
the
feat/permit-graphs-to-be-returned-unpruned-as-jsonl
branch
from
ab5e9c7 to
6cd3742
Compare
| }); | ||
| } | ||
|
|
||
| export function shouldPrintDepGraph(opts: Options): boolean { |
There was a problem hiding this comment.
This was moved down and modified slightly - see shouldPrintDepGraphLegacy
| // Some plugins e.g. Maven explicitly check for print-graph | ||
| if (shouldPrintDepGraphWithErrors(options)) { | ||
| options['print-graph'] = true; | ||
| } |
There was a problem hiding this comment.
This makes me think this option might be better expressed as an additional argument that controls whether errors are shown.
Does the maven plugin also look for --print-effective-graph as well as --print-graph?
There was a problem hiding this comment.
Does the maven plugin also look for
--print-effective-graphas well as--print-graph?
No it doesn't - see here
This makes me think this option might be better expressed as an additional argument that controls whether errors are shown.
Yes I think this would be nice - presumably we'd want to do the same with --print-effective-graph-with-errors too, i.e. we'd have a new flag like --include-print-graph-errors which could be used with either --print-graph or --print-effective-graph (and we'd get rid of the --print-effective-graph-with-errors flag)? What do you think?
There was a problem hiding this comment.
Now that errors are required to be output for two different graphs it makes sense to switch to something like --include-print-graph-errors.
One downside though, is that I wouldn’t expect adding --include-print-graph-errors to --print-graph to also change the output format from legacy to JSON.
|
Closing for now as we are considering alternative approaches that unify the printing flag (e.g. a new |
Pull Request Submission Checklist
are release-note ready, emphasizing
what was changed, not how.
What does this PR do?
This PR adds a new
--print-graph-with-errorsflag, which is almost identical to--print-effective-graph-with-errorsexcept that the graph is the "complete" dep-graph rather than the "effective" graph. This is required for us to bring uv support to thesbomCLI command.I have also done a small amount of refactoring to help with the number of print-graph flags that are now available, but I still think we could consider further refactoring in future: for instance, perhaps we could have a single
--print-graphflag with options to toggle on the graph being "effective", including errors, and being in legacy vs JSON format. However this would take a lot of work and would be somewhat risky as it would require coordinated changes across multiple plugins, so I was reluctant to make that change right now - happy to hear any thoughts.How should this be manually tested?
Build the CLI on this branch, and run
<path/to/cli> test --print-graph-with-errorsagainst some projects, possibly with--all-projects. You should see the graph printed as expected, but importantly in comparison to--print-effective-graph-with-errors, the graph will have all transitive dependencies.What's the product update that needs to be communicated to CLI users?
None