Skip to content

Commit

Permalink
chore: minor fixups
Browse files Browse the repository at this point in the history
Signed-off-by: Andrew Lilley Brinker <[email protected]>
  • Loading branch information
alilleybrinker committed Oct 10, 2024
1 parent 8a54200 commit b956453
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 17 deletions.
49 changes: 32 additions & 17 deletions site/content/rfds/0005-target-resolution-refactor.md
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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. `[email protected]`), or provide a git reference to checkout
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()`,
Expand Down Expand Up @@ -272,12 +280,14 @@ struct TargetSeed {
pub refspec: Option<String>,
pub ignore_version_errors: bool,
}

struct Target {
pub seed: TargetSeed,
pub local: LocalGitRepo,
pub remote: Option<RemoteGitRepo>,
pub package: Option<Package>
}

struct TargetResolver {
seed: TargetSeed,
local: Option<LocalGitRepo>,
Expand All @@ -286,6 +296,7 @@ struct TargetResolver {
maven: Option<MavenPackage>,
sbom: Option<Sbom>,
}

impl TargetResolver {
pub fn get_checkout_target(&mut self) -> Result<String> {
let res = if let Some(pkg) = &self.package {
Expand All @@ -306,6 +317,7 @@ impl TargetResolver {
};
Ok(res)
}

pub fn resolve(seed: TargetSeed) -> Result<Target> {
let mut resolver = TargetResolver {
seed: seed.clone(),
Expand All @@ -315,7 +327,9 @@ impl TargetResolver {
maven: None,
sbom: None,
};

use TargetSeedKind::*;

// Resolution logic depends on seed
let local = match seed.kind {
Sbom(sbom) => {
Expand All @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions site/content/rfds/0007-simplified-release-procedures.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
---
title: "Simplified Release Procedures"
weight: 7
slug: 0007
extra:
rfd: 7
---

## The Current Situation
Expand Down

0 comments on commit b956453

Please sign in to comment.