335 lines
13 KiB
Rust
335 lines
13 KiB
Rust
//! `dependency <number> [list|add|remove] [numbers...]` — manage an
|
|
//! issue/PR's "blocked by" dependency links. Default action: list.
|
|
//!
|
|
//! Forgejo's dependency endpoints work on the shared issue/PR index (a
|
|
//! PR is an issue internally), so this is wired into both `issue <verb>`
|
|
//! and `pr <verb>` the same way `labels`/`assign`/`timeline` are.
|
|
//! Setting `<number>`'s dependency on `<dep>` means `<number>` is
|
|
//! blocked by `<dep>` — matching the forge web UI's "add dependency"
|
|
//! action, which is a same-repo-only relationship (there is no
|
|
//! cross-repo dependency support in this CLI, mirroring the UI).
|
|
|
|
use std::collections::{HashMap, HashSet, VecDeque};
|
|
|
|
use anyhow::{Result, bail};
|
|
use clap::{Args as ClapArgs, Subcommand};
|
|
use forgejo_api::ForgejoError;
|
|
use forgejo_api::structs::IssueMeta;
|
|
use serde_json::Value;
|
|
|
|
use crate::client::{Client, index};
|
|
use crate::verbs::{dependency_summaries, print_json};
|
|
|
|
#[derive(ClapArgs)]
|
|
pub struct Args {
|
|
/// Issue or PR number.
|
|
pub(crate) number: u64,
|
|
#[command(subcommand)]
|
|
action: Option<Action>,
|
|
}
|
|
|
|
#[derive(Subcommand)]
|
|
enum Action {
|
|
/// List dependencies (default when you give no action) — the
|
|
/// issues/PRs blocking this one.
|
|
List,
|
|
/// Add one or more issues/PRs that block this one.
|
|
///
|
|
/// The command refuses an add that would close a cycle, directly or
|
|
/// through other issues, before it sends any request, and the error
|
|
/// names the existing path.
|
|
Add {
|
|
/// Issue/PR numbers to add as dependencies.
|
|
deps: Vec<u64>,
|
|
},
|
|
/// Remove one or more dependency links.
|
|
Remove {
|
|
/// Issue/PR numbers to remove as dependencies.
|
|
deps: Vec<u64>,
|
|
},
|
|
}
|
|
|
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
|
let (owner, name) = client.owner_repo()?;
|
|
let idx = index(args.number)?;
|
|
match args.action.unwrap_or(Action::List) {
|
|
Action::List => {}
|
|
Action::Add { deps } => {
|
|
if deps.is_empty() {
|
|
bail!("hive-forge dependency add: pass at least one issue/PR number");
|
|
}
|
|
apply_and_verify(client, owner, name, idx, args.number, &deps, true)?;
|
|
}
|
|
Action::Remove { deps } => {
|
|
if deps.is_empty() {
|
|
bail!("hive-forge dependency remove: pass at least one issue/PR number");
|
|
}
|
|
apply_and_verify(client, owner, name, idx, args.number, &deps, false)?;
|
|
}
|
|
}
|
|
let deps = dependency_summaries(client, owner, name, args.number)?;
|
|
print_json(&serde_json::json!(deps))
|
|
}
|
|
|
|
/// Apply an add/remove edit for each of `deps` against `number`, then
|
|
/// verify by reading the dependency list back rather than trusting the
|
|
/// HTTP status this verb family returns.
|
|
///
|
|
/// Measured: `create` can 500 on a call that actually wrote the edge (an
|
|
/// intermittent server-side double-processing, not a client retry —
|
|
/// ruled out separately). A `send()` error there only means "maybe" —
|
|
/// the authoritative signal is whether the edge is actually there
|
|
/// afterward.
|
|
///
|
|
/// `remove`'s known quirk is handled below, before it ever reaches this
|
|
/// path: the forge always answers `201 Created` for a successful
|
|
/// deletion, which the generated client's endpoint spec doesn't expect
|
|
/// (it only treats 200 as success for that verb), so that specific
|
|
/// status is treated as success outright rather than routed through the
|
|
/// read-back below.
|
|
///
|
|
/// Calls that errored are only re-classified as real failures if the
|
|
/// read-back does NOT show the expected converged state (edge present
|
|
/// after an add, absent after a remove) — a call that errored *and*
|
|
/// didn't converge is a genuine failure and still surfaces.
|
|
fn apply_and_verify(
|
|
client: &Client,
|
|
owner: &str,
|
|
name: &str,
|
|
idx: i64,
|
|
number: u64,
|
|
deps: &[u64],
|
|
adding: bool,
|
|
) -> Result<()> {
|
|
if adding {
|
|
for dep in deps {
|
|
refuse_cycle(client, owner, name, number, *dep)?;
|
|
}
|
|
}
|
|
let mut call_errors = Vec::new();
|
|
for dep in deps {
|
|
let meta = dep_meta(owner, name, *dep)?;
|
|
let res = if adding {
|
|
client
|
|
.api()
|
|
.issue_create_issue_dependencies(owner, name, idx, meta)
|
|
.send()
|
|
} else {
|
|
client
|
|
.api()
|
|
.issue_remove_issue_dependencies(owner, name, idx, meta)
|
|
.send()
|
|
};
|
|
if let Err(e) = res {
|
|
// `remove` always answers 201 for a successful deletion (not
|
|
// the 200 the generated client expects for that verb) — a
|
|
// deterministic, always-on mismatch, not a "something else
|
|
// went wrong after the write" anomaly, so it's not worth a
|
|
// read-back or a warning: just accept it.
|
|
let is_remove_201 = !adding
|
|
&& matches!(
|
|
&e,
|
|
ForgejoError::UnexpectedStatusCode(code) if *code == reqwest::StatusCode::CREATED
|
|
);
|
|
if !is_remove_201 {
|
|
call_errors.push((*dep, e));
|
|
}
|
|
}
|
|
}
|
|
if call_errors.is_empty() {
|
|
return Ok(());
|
|
}
|
|
let after = match dependency_summaries(client, owner, name, number) {
|
|
Ok(deps) => deps,
|
|
Err(read_err) => {
|
|
// The read-back itself failed, so none of the per-dep errors
|
|
// above could be re-classified — surface all of them rather
|
|
// than dropping them behind the read-back's own error.
|
|
let originals: Vec<String> = call_errors
|
|
.iter()
|
|
.map(|(dep, e)| format!("#{dep}: {e:#}"))
|
|
.collect();
|
|
let verb = if adding { "add" } else { "remove" };
|
|
bail!(
|
|
"hive-forge dependency {verb}: {} call(s) reported an error, and the \
|
|
read-back to check whether they actually landed also failed ({read_err:#}). \
|
|
Original error(s):\n{}",
|
|
call_errors.len(),
|
|
originals.join("\n")
|
|
);
|
|
}
|
|
};
|
|
let present: HashSet<i64> = after
|
|
.iter()
|
|
.filter_map(|d| d.get("number").and_then(Value::as_i64))
|
|
.collect();
|
|
let mut real_failures = Vec::new();
|
|
for (dep, e) in call_errors {
|
|
let still_present = present.contains(&index(dep).unwrap_or(-1));
|
|
// Converged iff present after an add, absent after a remove.
|
|
if still_present == adding {
|
|
// The write landed despite the error, but the error itself is
|
|
// still real signal: it means something *after* the write
|
|
// failed server-side (a timeline entry, a notification, a
|
|
// cycle check — we don't know which). Converging proves the
|
|
// effect is present, not that the operation fully succeeded,
|
|
// so don't let a clean exit make that anomaly unobservable.
|
|
eprintln!(
|
|
"hive-forge: warning: dependency on #{dep} converged despite a reported \
|
|
error (server-side issue after the write, not a real failure): {e:#}"
|
|
);
|
|
} else {
|
|
real_failures.push(format!("#{dep}: {e:#}"));
|
|
}
|
|
}
|
|
if !real_failures.is_empty() {
|
|
let verb = if adding { "add" } else { "remove" };
|
|
bail!(
|
|
"hive-forge dependency {verb}: {} call(s) genuinely failed (read-back doesn't \
|
|
show the expected state):\n{}",
|
|
real_failures.len(),
|
|
real_failures.join("\n")
|
|
);
|
|
}
|
|
Ok(())
|
|
}
|
|
|
|
/// Refuse an add of `dep` as a blocker of `number` when `dep` already
|
|
/// depends on `number`, directly or through other issues.
|
|
///
|
|
/// Forgejo answers a direct reverse edge with a bare `500` (its handler
|
|
/// maps every model error, `ErrCircularDependency` included, to
|
|
/// `Internal Server Error`) and accepts longer cycles without complaint,
|
|
/// so the check walks `dependency list` client-side.
|
|
fn refuse_cycle(client: &Client, owner: &str, name: &str, number: u64, dep: u64) -> Result<()> {
|
|
let path = find_dependency_path(dep, number, |n| {
|
|
let deps = dependency_summaries(client, owner, name, n)?;
|
|
Ok(deps
|
|
.iter()
|
|
.filter_map(|d| d.get("number").and_then(Value::as_u64))
|
|
.collect())
|
|
})?;
|
|
let Some(path) = path else { return Ok(()) };
|
|
let how = match path.as_slice() {
|
|
[_, _] => "directly".to_owned(),
|
|
[_, via @ .., _] => format!(
|
|
"via {}",
|
|
via.iter()
|
|
.map(|n| format!("#{n}"))
|
|
.collect::<Vec<_>>()
|
|
.join(" -> ")
|
|
),
|
|
_ => String::new(),
|
|
};
|
|
bail!(
|
|
"hive-forge dependency add: #{dep} already depends on #{number} ({how}), so adding \
|
|
#{dep} as a dependency of #{number} would create a cycle; remove that edge first"
|
|
);
|
|
}
|
|
|
|
/// Breadth-first walk of the "depends on" relation from `from`, using
|
|
/// `deps_of` to list an issue's dependencies. Returns the shortest chain
|
|
/// `from, …, to` when `to` is reachable, else `None`.
|
|
fn find_dependency_path(
|
|
from: u64,
|
|
to: u64,
|
|
mut deps_of: impl FnMut(u64) -> Result<Vec<u64>>,
|
|
) -> Result<Option<Vec<u64>>> {
|
|
let mut parent: HashMap<u64, u64> = HashMap::new();
|
|
let mut seen = HashSet::from([from]);
|
|
let mut queue = VecDeque::from([from]);
|
|
while let Some(cur) = queue.pop_front() {
|
|
for next in deps_of(cur)? {
|
|
if !seen.insert(next) {
|
|
continue;
|
|
}
|
|
parent.insert(next, cur);
|
|
if next == to {
|
|
let mut path = vec![to];
|
|
let mut at = to;
|
|
while let Some(&p) = parent.get(&at) {
|
|
path.push(p);
|
|
at = p;
|
|
}
|
|
path.reverse();
|
|
return Ok(Some(path));
|
|
}
|
|
queue.push_back(next);
|
|
}
|
|
}
|
|
Ok(None)
|
|
}
|
|
|
|
/// Build the `IssueMeta` body the create/remove endpoints want.
|
|
///
|
|
/// `owner`/`repo` are filled in with the *same* repo the request URL
|
|
/// already targets — not left `None`. Forgejo's dependency handler
|
|
/// resolves the dependency's repo from these body fields rather than
|
|
/// defaulting an absent value to the URL's own owner/repo, so omitting
|
|
/// them 404s looking up an empty-string repo (`owner/repo: not found`
|
|
/// — measured, not guessed: `hive-forge dependency <n> add <dep>`
|
|
/// failed this way for every same-repo pair before this fix). This CLI
|
|
/// still only supports same-repo dependencies (matching the forge web
|
|
/// UI) — the fields are populated, not opened up to cross-repo.
|
|
fn dep_meta(owner: &str, repo: &str, dep: u64) -> Result<IssueMeta> {
|
|
Ok(IssueMeta {
|
|
index: Some(index(dep)?),
|
|
owner: Some(owner.to_owned()),
|
|
repo: Some(repo.to_owned()),
|
|
})
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
/// The comment above `dep_meta` records a measured failure: leaving
|
|
/// `owner`/`repo` as `None` made every same-repo dependency 404,
|
|
/// because forgejo resolves the dependency's repo from the *body*
|
|
/// rather than falling back to the request URL. Nothing but a test
|
|
/// stops a future tidy-up from "simplifying" them back to `None`.
|
|
#[test]
|
|
fn dep_meta_populates_owner_and_repo_rather_than_leaving_them_none() {
|
|
let m = dep_meta("hyperhive", "hyperhive", 3950).expect("in range");
|
|
assert_eq!(m.owner.as_deref(), Some("hyperhive"));
|
|
assert_eq!(m.repo.as_deref(), Some("hyperhive"));
|
|
assert_eq!(m.index, Some(3950));
|
|
}
|
|
|
|
fn graph(edges: &[(u64, u64)]) -> impl FnMut(u64) -> Result<Vec<u64>> + '_ {
|
|
move |n| {
|
|
Ok(edges
|
|
.iter()
|
|
.filter(|(a, _)| *a == n)
|
|
.map(|(_, b)| *b)
|
|
.collect())
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn path_finds_direct_transitive_and_absent() {
|
|
let edges = [(1, 2), (2, 3), (4, 5)];
|
|
assert_eq!(
|
|
find_dependency_path(1, 2, graph(&edges)).unwrap(),
|
|
Some(vec![1, 2])
|
|
);
|
|
assert_eq!(
|
|
find_dependency_path(1, 3, graph(&edges)).unwrap(),
|
|
Some(vec![1, 2, 3])
|
|
);
|
|
assert_eq!(find_dependency_path(1, 5, graph(&edges)).unwrap(), None);
|
|
}
|
|
|
|
#[test]
|
|
fn path_walk_terminates_on_an_existing_cycle() {
|
|
let edges = [(1, 2), (2, 1)];
|
|
assert_eq!(find_dependency_path(1, 9, graph(&edges)).unwrap(), None);
|
|
}
|
|
|
|
/// Out-of-range must be an error, not a wrapped negative index that
|
|
/// the forge would then look up as some other issue.
|
|
#[test]
|
|
fn dep_meta_refuses_a_number_no_forge_could_have() {
|
|
assert!(dep_meta("o", "r", u64::MAX).is_err());
|
|
}
|
|
}
|