Skip to content

refactor: DRY flake metadata extraction and ref-statuses loading #232

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.

Sign up for GitHub

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

Open
Malix-Labs wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: DRY flake metadata extraction and ref-statuses loading #232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 40 additions & 6 deletions parse-flake-lock/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,7 @@ impl<'de> Deserialize<'de> for FlakeLock {
};

for (root_name, root_input) in root_node.inputs.iter() {
let inputs: VecDeque<String> = match root_input.clone() {
Input::String(s) => [s].into(),
Input::List(keys) => keys.into(),
};
let inputs: VecDeque<String> = root_input.to_deque();

let real_node = chase_input_node(&nodes, inputs).map_err(|e| {
de::Error::custom(format!("failed to chase input {}: {:?}", root_name, e))
Expand Down Expand Up @@ -173,7 +170,7 @@ fn chase_input_node(
let next_inputs = &node_inputs[&input];
node = match next_inputs {
Input::String(s) => &nodes[s],
Input::List(inputs) => chase_input_node(nodes, inputs.to_owned().into())?,
Input::List(_) => chase_input_node(nodes, next_inputs.to_deque())?,
};
}

Expand Down Expand Up @@ -214,7 +211,6 @@ pub enum Node {
Fallthrough(serde_json::value::Value), // Covers all other node types
}

// A string representation of the node variant (for logging).
impl Node {
fn variant(&self) -> &'static str {
match self {
Expand All @@ -226,6 +222,34 @@ impl Node {
Node::Fallthrough(_) => "Fallthrough", // Covers all other node types
}
}

/// Returns the Git reference if this is a repository node.
pub fn git_ref(&self) -> Option<&str> {
match self {
Node::Repo(repo) => repo.original.git_ref.as_deref(),
_ => None,
}
}

/// Returns the last modified timestamp if available.
pub fn last_modified(&self) -> Option<i64> {
match self {
Node::Repo(repo) => Some(repo.locked.last_modified),
Node::Tarball(tarball) => tarball.locked.last_modified,
Node::Path(path) => Some(path.locked.last_modified),
Node::Indirect(indirect) => Some(indirect.locked.last_modified),
_ => None,
}
}

/// Returns the repository owner if available.
pub fn owner(&self) -> Option<&str> {
match self {
Node::Repo(repo) => Some(&repo.original.owner),
Node::Indirect(indirect) => Some(&indirect.locked.owner),
_ => None,
}
}
}

/// An enum type representing node input references.
Expand All @@ -238,6 +262,16 @@ pub enum Input {
List(Vec<String>),
}

impl Input {
/// Returns the input as a list of keys (VecDeque).
pub fn to_deque(&self) -> VecDeque<String> {
match self {
Input::String(s) => vec![s.clone()].into(),
Input::List(keys) => keys.clone().into(),
}
}
}

/// A flake [Node] representing a raw mapping of strings to [Input]s.
#[derive(Clone, Debug, Deserialize)]
#[serde(deny_unknown_fields)]
Expand Down
32 changes: 9 additions & 23 deletions src/condition.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use cel::{Context, Program, Value};
use parse_flake_lock::{FlakeLock, Node};
use parse_flake_lock::FlakeLock;

use std::collections::{BTreeMap, HashMap};

Expand Down Expand Up @@ -34,15 +34,9 @@ pub(super) fn evaluate_condition(
let deps = nixpkgs_deps(flake_lock, nixpkgs_keys)?;

for (name, node) in deps {
let (git_ref, last_modified, owner) = match node {
Node::Repo(repo) => (
repo.original.git_ref,
Some(repo.locked.last_modified),
Some(repo.original.owner),
),
Node::Tarball(tarball) => (None, tarball.locked.last_modified, None),
_ => (None, None, None),
};
let git_ref = node.git_ref();
let last_modified = node.last_modified();
let owner = node.owner();

add_cel_variables(&mut ctx, git_ref, last_modified, owner);

Expand Down Expand Up @@ -70,22 +64,14 @@ pub(super) fn evaluate_condition(

fn add_cel_variables(
ctx: &mut Context,
git_ref: Option<String>,
git_ref: Option<&str>,
last_modified: Option<i64>,
owner: Option<String>,
owner: Option<&str>,
) {
ctx.add_variable_from_value(KEY_GIT_REF, value_or_empty_string(git_ref));
ctx.add_variable_from_value(KEY_GIT_REF, Value::from(git_ref.unwrap_or("")));
ctx.add_variable_from_value(
KEY_NUM_DAYS_OLD,
value_or_zero(last_modified.map(num_days_old)),
Value::from(last_modified.map(num_days_old).unwrap_or(0)),
);
ctx.add_variable_from_value(KEY_OWNER, value_or_empty_string(owner));
}

fn value_or_empty_string(value: Option<String>) -> Value {
Value::from(value.unwrap_or(String::from("")))
}

fn value_or_zero(value: Option<i64>) -> Value {
Value::from(value.unwrap_or(0))
ctx.add_variable_from_value(KEY_OWNER, Value::from(owner.unwrap_or("")));
}
40 changes: 11 additions & 29 deletions src/flake.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,7 @@ pub(super) fn nixpkgs_deps(

for (ref key, node) in flake_lock.root.clone() {
match &node {
Node::Repo(_) => {
if keys.contains(key) {
deps.insert(key.to_string(), node);
}
}
Node::Tarball(_) => {
Node::Repo(_) | Node::Tarball(_) => {
if keys.contains(key) {
deps.insert(key.to_string(), node);
}
Expand Down Expand Up @@ -86,20 +81,14 @@ pub(crate) fn check_flake_lock(
let deps = nixpkgs_deps(flake_lock, &config.nixpkgs_keys)?;

for (name, node) in deps {
let (git_ref, last_modified, owner) = match node {
Node::Repo(repo) => (
repo.original.git_ref,
Some(repo.locked.last_modified),
Some(repo.original.owner),
),
Node::Tarball(tarball) => (None, tarball.locked.last_modified, None),
_ => (None, None, None),
};
let git_ref = node.git_ref();
let last_modified = node.last_modified();
let owner = node.owner();

// Check if not explicitly supported
if let Some(git_ref) = git_ref {
// Check if not explicitly supported
if config.check_supported && !allowed_refs.contains(&git_ref) {
if config.check_supported && !allowed_refs.iter().any(|r| r == git_ref) {
issues.push(Issue {
input: name.clone(),
kind: IssueKind::Disallowed(Disallowed {
Expand Down Expand Up @@ -128,7 +117,7 @@ pub(crate) fn check_flake_lock(
if config.check_owner && owner.to_lowercase() != "nixos" {
issues.push(Issue {
input: name.clone(),
kind: IssueKind::NonUpstream(NonUpstream { owner }),
kind: IssueKind::NonUpstream(NonUpstream { owner: owner.to_string() }),
});
}
}
Expand All @@ -144,7 +133,6 @@ pub(super) fn num_days_old(timestamp: i64) -> i64 {

#[cfg(test)]
mod test {
use std::collections::BTreeMap;
use std::path::PathBuf;

use crate::{
Expand All @@ -160,11 +148,9 @@ mod test {
let cases: Vec<(&str, bool)> = vec![
(include_str!("../tests/cel-condition.cel"), true),
("supportedRefs.contains(gitRef) && owner != 'NixOS'", false),
("supportedRefs.contains(gitRef) && owner != 'NixOS'", false),
];

Comment on lines 164 to 165

@Malix-Labs Malix-Labs Jun 17, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this one was basically duplicated, i assume by mistake?

let ref_statuses: BTreeMap<String, String> =
serde_json::from_str(include_str!("../ref-statuses.json")).unwrap();
let ref_statuses = crate::local_ref_statuses();
let supported_refs = supported_refs(ref_statuses.clone());
let path = PathBuf::from("tests/flake.cel.0.lock");

Expand Down Expand Up @@ -196,8 +182,7 @@ mod test {

#[test]
fn clean_flake_locks() {
let ref_statuses: BTreeMap<String, String> =
serde_json::from_str(include_str!("../ref-statuses.json")).unwrap();
let ref_statuses = crate::local_ref_statuses();
let allowed_refs = supported_refs(ref_statuses);
for n in 0..=7 {
let path = PathBuf::from(format!("tests/flake.clean.{n}.lock"));
Expand All @@ -217,8 +202,7 @@ mod test {

#[test]
fn dirty_flake_locks() {
let ref_statuses: BTreeMap<String, String> =
serde_json::from_str(include_str!("../ref-statuses.json")).unwrap();
let ref_statuses = crate::local_ref_statuses();
let allowed_refs = supported_refs(ref_statuses);
let cases: Vec<(&str, Vec<Issue>)> = vec![
(
Expand Down Expand Up @@ -272,8 +256,7 @@ mod test {

#[test]
fn explicit_nixpkgs_keys() {
let ref_statuses: BTreeMap<String, String> =
serde_json::from_str(include_str!("../ref-statuses.json")).unwrap();
let ref_statuses = crate::local_ref_statuses();
let allowed_refs = supported_refs(ref_statuses);
let cases: Vec<(&str, Vec<String>, Vec<Issue>)> = vec![(
"flake.explicit-keys.0.lock",
Expand Down Expand Up @@ -301,8 +284,7 @@ mod test {

#[test]
fn missing_nixpkgs_keys() {
let ref_statuses: BTreeMap<String, String> =
serde_json::from_str(include_str!("../ref-statuses.json")).unwrap();
let ref_statuses = crate::local_ref_statuses();
let allowed_refs = supported_refs(ref_statuses);
let cases: Vec<(&str, Vec<String>, String)> = vec![
(
Expand Down
11 changes: 6 additions & 5 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ struct Cli {
condition: Option<String>,
}

#[cfg(not(feature = "ref-statuses"))]
pub(crate) fn local_ref_statuses() -> BTreeMap<String, String> {
serde_json::from_str(include_str!("../ref-statuses.json")).unwrap()
}

pub(crate) fn supported_refs(ref_statuses: BTreeMap<String, String>) -> Vec<String> {
let mut return_value: Vec<String> = ref_statuses
.iter()
Expand All @@ -120,8 +123,7 @@ async fn main() -> Result<ExitCode, FlakeCheckerError> {
.with(EnvFilter::from_default_env())
.init();

let ref_statuses: BTreeMap<String, String> =
serde_json::from_str(include_str!("../ref-statuses.json")).unwrap();
let ref_statuses = local_ref_statuses();

let Cli {
no_telemetry,
Expand Down Expand Up @@ -279,8 +281,7 @@ fn main() -> Result<ExitCode, FlakeCheckerError> {
}

if check_ref_statuses {
let mut ref_statuses: BTreeMap<String, String> =
serde_json::from_str(include_str!("../ref-statuses.json")).unwrap();
let ref_statuses = local_ref_statuses();

match ref_statuses::check_ref_statuses(ref_statuses) {
Ok(equals) => {
Expand Down
2 changes: 1 addition & 1 deletion src/ref_statuses.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub(crate) fn check_ref_statuses(
}

pub(crate) fn fetch_ref_statuses() -> Result<BTreeMap<String, String>, FlakeCheckerError> {
let mut officially_supported: BTreeMap<String, String> =
let officially_supported: BTreeMap<String, String> =
reqwest::blocking::get(ALLOWED_REFS_URL)?
.json::<Response>()?
.data
Expand Down
26 changes: 7 additions & 19 deletions src/summary.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,13 @@ use std::path::PathBuf;
use handlebars::Handlebars;
use serde_json::json;

static CEL_MARKDOWN_TEMPLATE: &str = include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/src/templates/summary.cel.md.hbs"
));

static CEL_TEXT_TEMPLATE: &str = include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/src/templates/summary.cel.txt.hbs"
));

static STANDARD_MARKDOWN_TEMPLATE: &str = include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/src/templates/summary.standard.md.hbs"
));

static STANDARD_TEXT_TEMPLATE: &str = include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/src/templates/summary.standard.txt.hbs"
));
static CEL_MARKDOWN_TEMPLATE: &str = include_str!("templates/summary.cel.md.hbs");

static CEL_TEXT_TEMPLATE: &str = include_str!("templates/summary.cel.txt.hbs");

static STANDARD_MARKDOWN_TEMPLATE: &str = include_str!("templates/summary.standard.md.hbs");

static STANDARD_TEXT_TEMPLATE: &str = include_str!("templates/summary.standard.txt.hbs");

pub(crate) struct Summary {
pub issues: Vec<Issue>,
Expand Down