Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 37 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ tokio = { version = "1", default-features = false, features = ["net", "rt-multi-
tempfile = "3.19.1"
thiserror = "2.0.18"
toml = "1.0"
toml_edit = "0.22"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not using the latest version of this package? I.e. what cargo add adds.


[dev-dependencies]
ansi_term = "0.12.1"
Expand Down
189 changes: 187 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ use anyhow::{Context, Error, bail, format_err};
use api::github;
use clap::Parser;
use log::{error, info, warn};
use std::collections::{BTreeMap, HashMap};
use std::path::PathBuf;
use std::collections::{BTreeMap, HashMap, HashSet};
use std::path::{Path, PathBuf};
use std::str::FromStr;

#[derive(clap::ValueEnum, Clone, Debug)]
Expand Down Expand Up @@ -106,6 +106,9 @@ enum RootOpts {
DecryptEmail,
/// Generate a x25519 key for use with the email encryption module
GenerateKey,
/// Archive a repo or team, moving it to the archive directory
#[clap(subcommand)]
Archive(ArchiveOpts),
/// CI scripts
#[clap(subcommand)]
Ci(CiOpts),
Expand Down Expand Up @@ -133,6 +136,20 @@ enum CiOpts {
CheckUntrackedRepos,
}

#[derive(clap::Parser, Clone, Debug)]
enum ArchiveOpts {
/// Archive a repository
Repo {
/// Repository in "org/name" format (e.g. "rust-lang/homu")
name: String,
},
/// Archive a team
Team {
/// Team name (e.g. "project-generic-associated-types")
name: String,
},
}

#[derive(clap::Parser, Clone, Debug)]
struct SyncOpts {
/// Comma-separated list of available services
Expand Down Expand Up @@ -563,6 +580,14 @@ async fn run() -> Result<(), Error> {
let (secret, public) = rust_team_data::email_encryption::generate_x25519_keypair();
println!("Generated keypair: secret: {} - public: {}", secret, public);
}
RootOpts::Archive(opts) => match opts {
ArchiveOpts::Repo { ref name } => {
archive_repo(&cli.data_dir, name)?;
}
ArchiveOpts::Team { ref name } => {
archive_team(&cli.data_dir, name)?;
}
},
RootOpts::Ci(opts) => match opts {
CiOpts::GenerateCodeowners => generate_codeowners_file(data)?,
CiOpts::CheckCodeowners => check_codeowners(data)?,
Expand Down Expand Up @@ -647,3 +672,163 @@ async fn perform_sync(opts: SyncOpts, data: Data) -> anyhow::Result<()> {
)
.await
}

fn get_access_teams(doc: &mut toml_edit::DocumentMut) -> Option<&mut toml_edit::Table> {
doc.get_mut("access")?.get_mut("teams")?.as_table_mut()
}

fn archive_repo(data_dir: &Path, name: &str) -> Result<(), Error> {
let (org, repo_name) = name
.split_once('/')
.ok_or_else(|| format_err!("repository must be in 'org/name' format, got '{}'", name))?;

let src = data_dir
.join("repos")
.join(org)
.join(format!("{repo_name}.toml"));
let dest_dir = data_dir.join("repos").join("archive").join(org);
let dest = dest_dir.join(format!("{repo_name}.toml"));

if !src.is_file() {
bail!("repo file not found: {}", src.display());
}
if dest.is_file() {
bail!("repo is already archived: {}", dest.display());
}

let content = std::fs::read_to_string(&src)
.with_context(|| format!("failed to read {}", src.display()))?;
let mut doc: toml_edit::DocumentMut = content
.parse()
.with_context(|| format!("failed to parse {}", src.display()))?;

if let Some(table) = get_access_teams(&mut doc) {
table.clear();
}

std::fs::create_dir_all(&dest_dir)
.with_context(|| format!("failed to create directory {}", dest_dir.display()))?;
std::fs::write(&dest, doc.to_string())
.with_context(|| format!("failed to write {}", dest.display()))?;
std::fs::remove_file(&src).with_context(|| format!("failed to remove {}", src.display()))?;
Comment on lines +709 to +713
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this code is duplicated among the two functions. So it would be nice to extract it into a function and reuse it.

In general, please inspect your code and find opportunities similar to this one to extract common code.


info!("archived repo {} -> {}", src.display(), dest.display());
Ok(())
}

fn archive_team(data_dir: &Path, name: &str) -> Result<(), Error> {
let src = data_dir.join("teams").join(format!("{name}.toml"));
let dest_dir = data_dir.join("teams").join("archive");
let dest = dest_dir.join(format!("{name}.toml"));

if !src.is_file() {
bail!("team file not found: {}", src.display());
}
if dest.is_file() {
bail!("team is already archived: {}", dest.display());
}

let content = std::fs::read_to_string(&src)
.with_context(|| format!("failed to read {}", src.display()))?;
let mut doc: toml_edit::DocumentMut = content
.parse()
.with_context(|| format!("failed to parse {}", src.display()))?;

if let Some(people) = doc.get_mut("people")
&& let Some(people_table) = people.as_table_mut()
{
let mut all_alumni: Vec<String> = Vec::new();
let mut seen = HashSet::new();
Comment on lines +740 to +741
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we use only the hashset here?


// Collect everyone from leads, members, and existing alumni
for key in &["leads", "members", "alumni"] {
if let Some(arr) = people_table.get(key).and_then(|v| v.as_array()) {
for item in arr.iter() {
let username = if let Some(s) = item.as_str() {
s.to_string()
} else if let Some(tbl) = item.as_inline_table() {
match tbl.get("github").and_then(|v| v.as_str()) {
Some(s) => s.to_string(),
None => continue,
}
} else {
continue;
};
if !username.is_empty() && seen.insert(username.clone()) {
all_alumni.push(username);
}
}
}
}

people_table.insert("leads", toml_edit::value(toml_edit::Array::new()));
people_table.insert("members", toml_edit::value(toml_edit::Array::new()));
Comment on lines +764 to +765
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems that

 toml_edit::Array::new().into()

works equality as

toml_edit::value(toml_edit::Array::new())

but it reads a bit shorter (also better, imho)


let mut alumni_array = toml_edit::Array::new();
for person in &all_alumni {
let mut val = toml_edit::Value::from(person.as_str());
val.decor_mut().set_prefix("\n ");
alumni_array.push_formatted(val);
}
alumni_array.set_trailing("\n");
alumni_array.set_trailing_comma(true);
people_table.insert("alumni", toml_edit::value(alumni_array));
}

std::fs::create_dir_all(&dest_dir)
.with_context(|| format!("failed to create directory {}", dest_dir.display()))?;
std::fs::write(&dest, doc.to_string())
.with_context(|| format!("failed to write {}", dest.display()))?;
std::fs::remove_file(&src).with_context(|| format!("failed to remove {}", src.display()))?;

info!("archived team {} -> {}", src.display(), dest.display());

remove_team_from_repos(data_dir, name)?;

Ok(())
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this function is very long. I would extract it into smaller ones.


fn remove_team_from_repos(data_dir: &Path, team_name: &str) -> Result<(), Error> {
let repos_dir = data_dir.join("repos");
if !repos_dir.is_dir() {
return Ok(());
}

for org_entry in std::fs::read_dir(&repos_dir)
.with_context(|| format!("failed to read {}", repos_dir.display()))?
{
let org_path = org_entry?.path();
if !org_path.is_dir() || org_path.file_name() == Some(std::ffi::OsStr::new("archive")) {
continue;
}

for repo_entry in std::fs::read_dir(&org_path)
.with_context(|| format!("failed to read {}", org_path.display()))?
{
let repo_path = repo_entry?.path();
if !repo_path.is_file() || repo_path.extension() != Some(std::ffi::OsStr::new("toml")) {
continue;
}

let content = std::fs::read_to_string(&repo_path)
.with_context(|| format!("failed to read {}", repo_path.display()))?;
let mut doc: toml_edit::DocumentMut = content
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the turbofish notation speaks better to what we have in other parts of the code. It worth's a look in case you are not familiar with :)

.parse()
.with_context(|| format!("failed to parse {}", repo_path.display()))?;

let removed = if let Some(table) = get_access_teams(&mut doc) {
table.remove(team_name).is_some()
} else {
false
};

if removed {
std::fs::write(&repo_path, doc.to_string())
.with_context(|| format!("failed to write {}", repo_path.display()))?;
info!("removed team '{}' from {}", team_name, repo_path.display());
}
}
}

Ok(())
}