From b95645344544841579c3beaee6ad93b36176adc1 Mon Sep 17 00:00:00 2001 From: Andrew Lilley Brinker Date: Thu, 10 Oct 2024 08:12:00 -0700 Subject: [PATCH] chore: minor fixups Signed-off-by: Andrew Lilley Brinker --- .../rfds/0005-target-resolution-refactor.md | 49 ++++++++++++------- .../0007-simplified-release-procedures.md | 4 ++ 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/site/content/rfds/0005-target-resolution-refactor.md b/site/content/rfds/0005-target-resolution-refactor.md index f1840d50..b9ac7fbb 100644 --- a/site/content/rfds/0005-target-resolution-refactor.md +++ b/site/content/rfds/0005-target-resolution-refactor.md @@ -1,10 +1,18 @@ +--- +title: Target Resolution Refactor +weight: 5 +slug: 0005 +extra: + rfd: 5 +--- + # Target Resolution Refactor -In Hipcheck, the target resolution finds a repository matching the -specifications a user provides on the commandline in the form of a target -specifier string, optional type hint, and additional arguments. It then copies +In Hipcheck, the target resolution process finds a repository matching the +specifications a user provides on the command line in the form of a target +specifier string, optional type hint, and additional arguments. It then copies or clones that repository to the Hipcheck cache and checks it out to a suitable -commit. As the Hipcheck team works to implement [RFD #4][rfd_4], Hipcheck as a +commit. As the Hipcheck team works to implement [RFD #4][rfd_4], Hipcheck as a codebase will transition from housing analysis techniques to becoming an engine for orchestrating analyses implemented by plugins. As a result of this transition, the target resolution phase will become a more essential and @@ -19,9 +27,9 @@ RFD aims to do the following: - Clearly establish the design goals of the refactor - Offer a new design that meets these goals -### An Illustrating Example +## An Illustrating Example -An example of a pain-point in the existing codebase involves how package +An example of a pain point in the existing codebase involves how package versions are treated by the later remote repo resolution phase. When a user targets a package for analysis, they may provide a version number in the target string itself (e.g. `node-ipc@10.1.0`), or provide a git reference to checkout @@ -33,7 +41,7 @@ We've discovered that (unsurprisingly), a package's version numbering scheme doesn't always map directly to the tags in its repository (e.g. `hipcheck` version `3.5.0` has the tag `hipcheck-v3.5.0`, not simply `3.5.0`). Therefore, specifically when we go through `Package` resolution, we would like to be able -to do "fuzzy matching" of the version to tags in the repository. We need the +to do "fuzzy matching" of the version to tags in the repository. We need the local repository clone to be able to do that matching, but once we get to cloning/updating a remote repository on the local machine, we have lost the context that we started resolving from a package and that we would like to @@ -47,11 +55,11 @@ remembers the context of previous resolution steps. Overarching goals of the refactor are as follows: -### Increased debug-ability +### Increased debugability -A developer's ability to debug the target resolution subsystem will be proportional -to how clearly the code itself reflects the business logic we want the -resolution to follow. This includes clearly delineating the "phases" of +A developer's ability to debug the target resolution subsystem will be +proportional to how clearly the code itself reflects the business logic we want +the resolution to follow. This includes clearly delineating the "phases" of resolution and what class of logic belongs where. For example, in [the example above](#an-illustrating-example), is version fuzzy-matching logic something that the "package resolution" function should handle when the inner "remote repo @@ -64,10 +72,9 @@ previously a package? Knowing what previous resolution steps were taken, being able to access what they produced, being able to mutate internal state of the yet-unfinished `Target` struct are important to allow custom logic for particular resolution -"paths". As the [illustrating example](#an-illustrating-example) shows, the +"paths". As the [illustrating example](#an-illustrating-example) shows, the current resolution subsystem does not adequately convey context to subsequent -resolution -step. +resolution step. ### Distinguish between valid and unfinished resolutions @@ -137,7 +144,8 @@ for analysis. To define how resolution goes from a loose bag of `Option` values to a `Target`, we applied the [Strategy][strategy_pattern] design pattern. We define a trait -`ResolveRepo` for each `TargetSeedKind` variant to determine the steps it will take. +`ResolveRepo` for each `TargetSeedKind` variant to determine the steps it will +take. ```rust trait ResolveRepo { @@ -215,8 +223,8 @@ impl TargetResolver { ### Context-aware logic -With this design, we make the choice to organize logic by subphase rather than by -`TargetSeedKind` type. Referring back to the [illustrating +With this design, we make the choice to organize logic by subphase rather than +by `TargetSeedKind` type. Referring back to the [illustrating example](#an-illustrating-example), although the fuzzy version matching logic is specific to resolution paths with a `Package` ancestry, instead of in `Package::resolve()` we will implement that logic in `LocalGitRepo::resolve()`, @@ -272,12 +280,14 @@ struct TargetSeed { pub refspec: Option, pub ignore_version_errors: bool, } + struct Target { pub seed: TargetSeed, pub local: LocalGitRepo, pub remote: Option, pub package: Option } + struct TargetResolver { seed: TargetSeed, local: Option, @@ -286,6 +296,7 @@ struct TargetResolver { maven: Option, sbom: Option, } + impl TargetResolver { pub fn get_checkout_target(&mut self) -> Result { let res = if let Some(pkg) = &self.package { @@ -306,6 +317,7 @@ impl TargetResolver { }; Ok(res) } + pub fn resolve(seed: TargetSeed) -> Result { let mut resolver = TargetResolver { seed: seed.clone(), @@ -315,7 +327,9 @@ impl TargetResolver { maven: None, sbom: None, }; + use TargetSeedKind::*; + // Resolution logic depends on seed let local = match seed.kind { Sbom(sbom) => { @@ -339,6 +353,7 @@ impl TargetResolver { local.resolve(&mut resolver) }, }?; + // Finally piece together the Target with the non-optional local repo Ok(Target { seed: resolver.seed, diff --git a/site/content/rfds/0007-simplified-release-procedures.md b/site/content/rfds/0007-simplified-release-procedures.md index cbeb3be1..5859ea6f 100644 --- a/site/content/rfds/0007-simplified-release-procedures.md +++ b/site/content/rfds/0007-simplified-release-procedures.md @@ -1,5 +1,9 @@ --- title: "Simplified Release Procedures" +weight: 7 +slug: 0007 +extra: + rfd: 7 --- ## The Current Situation