Skip to content

chore(tr): add dependency graph to TR rpc interface#348

Merged
aryx merged 2 commits intomainfrom
bk/tr/add-dep-graph-to-tr-rpc-interface
Feb 21, 2025
Merged

chore(tr): add dependency graph to TR rpc interface#348
aryx merged 2 commits intomainfrom
bk/tr/add-dep-graph-to-tr-rpc-interface

Conversation

@bkettle
Copy link
Contributor

@bkettle bkettle commented Feb 11, 2025

TR will require sending the dependency graph in addition to the transitive findings in order to do the filtering step. This adds a single dependency graph to the RPC interface.

  • I ran make setup && make to update the generated code after editing a .atd file (TODO: have a CI check)
  • I made sure we're still backward compatible with old versions of the CLI.
    For example, the Semgrep backend need to still be able to consume data
    generated by Semgrep 1.50.0.
    See https://atd.readthedocs.io/en/latest/atdgen-tutorial.html#smooth-protocol-upgrades
    Note that the types related to the semgrep-core JSON output or the
    semgrep-core RPC do not need to be backward compatible!

Copy link
Contributor Author

bkettle commented Feb 11, 2025

@github-actions
Copy link

github-actions bot commented Feb 11, 2025

Backwards compatibility summary:

Checking backward compatibility of semgrep_output_v1.atd against past version v1.100.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.101.0
Skipping v1.102.0 because commit 1c82453e89e0b569630e48ddde015e201df0e5f9 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.103.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.104.0
Skipping v1.106.0 because commit 5e0c767ec323f3f2356d3bf8dbdf7c7836497d8a has already been checked
Skipping v1.107.0 because commit 5e0c767ec323f3f2356d3bf8dbdf7c7836497d8a has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.108.0
--- before.txt
+++ after.txt
@@ -0,0 +1,21 @@
+[105360ef] Incompatibility in both directions:
+Case 'TransitiveUndetermined' used to not have an argument.
+The following types are affected:
+  ci_scan_results
+  cli_match
+  cli_match_extra
+  cli_output
+  cli_output_extra
+  core_match
+  core_match_extra
+  core_output
+  finding
+  function_call
+  function_return
+  matching_explanation
+  matching_explanation_extra
+  partial_scan_result
+  sca_match
+  sca_match_kind
+  transitive_finding
+
ERROR: semgrep_output_v1.atd is not backward compatible with v1.108.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.109.0
--- before.txt
+++ after.txt
@@ -0,0 +1,21 @@
+[105360ef] Incompatibility in both directions:
+Case 'TransitiveUndetermined' used to not have an argument.
+The following types are affected:
+  ci_scan_results
+  cli_match
+  cli_match_extra
+  cli_output
+  cli_output_extra
+  core_match
+  core_match_extra
+  core_output
+  finding
+  function_call
+  function_return
+  matching_explanation
+  matching_explanation_extra
+  partial_scan_result
+  sca_match
+  sca_match_kind
+  transitive_finding
+
ERROR: semgrep_output_v1.atd is not backward compatible with v1.109.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.75.0
Skipping v1.76.0 because commit 9102031608aa4154e1c37f557550ec4eabc8780c has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.77.0
Skipping v1.78.0 because commit dcb5d77b420ddee61f58aadd3c2c7aef38778154 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.79.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.80.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.81.0
Skipping v1.82.0 because commit 9e0f3bec26b07b4fb6753a32cb75277f45f2572c has already been checked
Skipping v1.83.0 because commit 9e0f3bec26b07b4fb6753a32cb75277f45f2572c has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.84.0
Skipping v1.84.1 because commit 3daef49297ada205359cc1d2996354c94b628b0d has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.85.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.86.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.87.0
Skipping v1.88.0 because commit 512c0bd97db59c48a5705b2741662a338776e438 has already been checked
Skipping v1.89.0 because commit 512c0bd97db59c48a5705b2741662a338776e438 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.90.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.91.0
Skipping v1.92.0 because commit 2351c5e528cb7430422208dc66707894c066b508 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.93.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.94.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.95.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.96.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.97.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.98.0
Skipping v1.99.0 because commit 60809032a2e39742f42910d46b3e5dd305b8b8cf has already been checked

@bkettle bkettle marked this pull request as ready for review February 11, 2025 01:18
| CallTransitiveReachabilityFilter of transitive_finding list
(* for now, the transitive reachability filter takes only a single dependency graph as input.
* it is up to the caller to call it several times, one for each subproject *)
| CallTransitiveReachabilityFilter of ((found_dependency * downloaded_dependency list) * transitive_finding list)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you want (found _dependency * downloaded_dependency) list no ?
Also why you talk about graph in the comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct, fixed.

The (found _dependency * downloaded_dependency) list encodes the dependency graph, so the comment is referring to that

*)
}

type transitive_reachability_filter_params = {
Copy link
Contributor

Choose a reason for hiding this comment

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

make sense

@bkettle bkettle force-pushed the bk/add-internal-dependency-type branch from b8aaedd to c70701f Compare February 18, 2025 19:07
@bkettle bkettle force-pushed the bk/tr/add-dep-graph-to-tr-rpc-interface branch from 2c662e2 to 2fabafb Compare February 18, 2025 19:07
@bkettle bkettle force-pushed the bk/add-internal-dependency-type branch from c70701f to 9056434 Compare February 20, 2025 01:39
@bkettle bkettle force-pushed the bk/tr/add-dep-graph-to-tr-rpc-interface branch from 2fabafb to 031c5a8 Compare February 20, 2025 01:39
* (similar to (LockfileOnlyMatch Transitive))
*)
| TransitiveUndetermined
| TransitiveUndetermined of transitive_undetermined
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aryx I think we will need to override the backwards compatibility check for this, does that seem reasonable? Should be okay since these types aren't consumed anywhere, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes it's fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you override and merge? I don't think I have permission

@bkettle bkettle changed the base branch from bk/add-internal-dependency-type to graphite-base/348 February 20, 2025 01:48
@bkettle bkettle force-pushed the bk/tr/add-dep-graph-to-tr-rpc-interface branch from 031c5a8 to 2d8231e Compare February 20, 2025 01:48
@bkettle bkettle changed the base branch from graphite-base/348 to main February 20, 2025 01:48
@bkettle bkettle force-pushed the bk/tr/add-dep-graph-to-tr-rpc-interface branch from 2d8231e to 8289a19 Compare February 20, 2025 01:48
@aryx aryx merged commit 47b9407 into main Feb 21, 2025
3 of 4 checks passed
@aryx aryx deleted the bk/tr/add-dep-graph-to-tr-rpc-interface branch February 21, 2025 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants