Skip to content

Commit 53c794b

Browse files
nikomatsakisClaude
andauthored
create issues for 2025h2 project goals (#417)
* add tracking issue for trait goal * update goals with tracking issues * fix bug when the number of labels>32 * Fix infinite loop when multiple goals share same tracking issue - Add maximum iteration limit (10) to prevent infinite loops - Add validation to detect and report duplicate tracking issues - Provide clear error message when duplicate tracking issues are found Co-authored-by: Claude <[email protected]> * fix table rewrite within the file The span for the table needs to be the full span and we need to take a `Spanned<Table>`. I wonder what this will break that I'm not thinking of! * make github command more resilient --------- Co-authored-by: Claude <[email protected]>
1 parent ed1b63f commit 53c794b

29 files changed

+319
-233
lines changed

crates/rust-project-goals-cli/src/rfc.rs

Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,18 @@ pub fn generate_issues(
112112

113113
// Hacky but works: we loop because after creating the issue, we sometimes have additional sync to do,
114114
// and it's easier this way.
115+
let mut iteration_count = 0;
116+
const MAX_ITERATIONS: usize = 10;
117+
115118
loop {
119+
iteration_count += 1;
120+
if iteration_count > MAX_ITERATIONS {
121+
spanned::bail_here!(
122+
"Fixed point iteration failed to converge after {} iterations. \
123+
This may indicate duplicate tracking issues assigned to multiple goals.",
124+
MAX_ITERATIONS
125+
);
126+
}
116127
let timeframe = validate_path(path)?;
117128

118129
let mut goal_documents = goal::goals_in_dir(path)?;
@@ -168,6 +179,9 @@ pub fn generate_issues(
168179
eprintln!("Use `--commit` to execute the actions.");
169180
return Ok(());
170181
}
182+
183+
eprintln!("Waiting for github commands to propagate.");
184+
std::thread::sleep(Duration::from_millis(1000));
171185
}
172186
}
173187

@@ -221,11 +235,6 @@ enum GithubAction<'doc> {
221235
LockIssue {
222236
number: u64,
223237
},
224-
225-
LinkToTrackingIssue {
226-
goal_document: &'doc GoalDocument,
227-
issue_id: IssueId,
228-
},
229238
}
230239

231240
/// Initializes the required `T-<team>` labels on the repository.
@@ -281,6 +290,35 @@ fn initialize_issues<'doc>(
281290
.map(|goal_document| issue(timeframe, goal_document))
282291
.collect::<Result<_>>()?;
283292

293+
// Check for duplicate tracking issues
294+
let mut tracking_issue_counts = std::collections::HashMap::new();
295+
for issue in &desired_issues {
296+
if let Some(tracking_issue) = issue.tracking_issue {
297+
let count = tracking_issue_counts
298+
.entry(tracking_issue.number)
299+
.or_insert(0);
300+
*count += 1;
301+
}
302+
}
303+
304+
for (issue_number, count) in tracking_issue_counts {
305+
if count > 1 {
306+
let goals_with_issue: Vec<_> = desired_issues
307+
.iter()
308+
.filter(|issue| issue.tracking_issue.map(|ti| ti.number) == Some(issue_number))
309+
.map(|issue| issue.goal_document.path.display().to_string())
310+
.collect();
311+
312+
spanned::bail_here!(
313+
"Tracking issue #{} is assigned to {} goals: {}. \
314+
Each tracking issue can only be assigned to one goal.",
315+
issue_number,
316+
count,
317+
goals_with_issue.join(", ")
318+
);
319+
}
320+
}
321+
284322
// the list of existing issues in the target milestone
285323
let milestone_issues = list_issues_in_milestone(repository, timeframe)?;
286324

@@ -390,14 +428,6 @@ fn initialize_issues<'doc>(
390428
body,
391429
});
392430
}
393-
394-
let issue_id = IssueId::new(repository.clone(), existing_issue.number);
395-
if desired_issue.tracking_issue != Some(&issue_id) {
396-
actions.insert(GithubAction::LinkToTrackingIssue {
397-
goal_document: desired_issue.goal_document,
398-
issue_id,
399-
});
400-
}
401431
}
402432

403433
None => {
@@ -534,7 +564,7 @@ impl Display for GithubAction<'_> {
534564
write!(f, "create label `{}` with color `{}`", name, color)
535565
}
536566
GithubAction::CreateIssue { issue } => {
537-
write!(f, "create issue \"{}\"", issue.title)
567+
write!(f, "create issue \"{}\"", issue.title,)
538568
}
539569
GithubAction::ChangeMilestone { number, milestone } => {
540570
write!(f, "update issue #{} milestone to \"{}\"", number, milestone)
@@ -568,16 +598,6 @@ impl Display for GithubAction<'_> {
568598
GithubAction::LockIssue { number } => {
569599
write!(f, "lock issue #{}", number)
570600
}
571-
GithubAction::LinkToTrackingIssue {
572-
goal_document,
573-
issue_id,
574-
} => {
575-
write!(
576-
f,
577-
"link issue {issue_id:?} to the markdown document at {}",
578-
goal_document.path.display()
579-
)
580-
}
581601
}
582602
}
583603
}
@@ -598,12 +618,13 @@ impl GithubAction<'_> {
598618
body,
599619
labels,
600620
tracking_issue: _,
601-
goal_document: _,
621+
goal_document,
602622
},
603623
} => {
604-
create_issue(repository, &body, &title, &labels, &assignees, timeframe)?;
624+
let issue_id =
625+
create_issue(repository, &body, &title, &labels, &assignees, timeframe)?;
605626

606-
// Note: the issue is not locked, but we will reloop around later.
627+
goal_document.link_issue(issue_id)?;
607628

608629
Ok(())
609630
}
@@ -640,11 +661,6 @@ impl GithubAction<'_> {
640661
}
641662

642663
GithubAction::LockIssue { number } => lock_issue(repository, number),
643-
644-
GithubAction::LinkToTrackingIssue {
645-
goal_document,
646-
issue_id: number,
647-
} => goal_document.link_issue(number),
648664
}
649665
}
650666
}

crates/rust-project-goals/src/gh/issue_id.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::re::{REPOSITORY, TRACKING_ISSUE};
1+
use crate::re::{GITHUB_ISSUE_URL, REPOSITORY, TRACKING_ISSUE};
22
use std::fmt::Display;
33

44
#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Debug)]
@@ -58,6 +58,17 @@ impl IssueId {
5858
} = self;
5959
format!("https://github.com/{org}/{repo}/issues/{number}")
6060
}
61+
62+
pub fn from_url(url: &str) -> Option<Self> {
63+
let Some(c) = GITHUB_ISSUE_URL.captures(url) else {
64+
return None;
65+
};
66+
67+
let repository = Repository::new(&c[1], &c[2]);
68+
let number = c[3].parse().unwrap();
69+
70+
Some(IssueId::new(repository, number))
71+
}
6172
}
6273

6374
impl std::fmt::Debug for IssueId {

crates/rust-project-goals/src/gh/issues.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use rust_project_goals_json::{GithubIssueState, Progress};
55
use serde::{Deserialize, Serialize};
66
use spanned::{Context, Error, Result};
77

8-
use crate::{re, util::comma};
8+
use crate::{gh::issue_id::IssueId, re, util::comma};
99

1010
use super::{issue_id::Repository, labels::GhLabel, milestone::GhMilestone};
1111

@@ -174,7 +174,7 @@ pub fn create_issue(
174174
labels: &[String],
175175
assignees: &BTreeSet<String>,
176176
milestone: &str,
177-
) -> Result<()> {
177+
) -> Result<IssueId> {
178178
let output = Command::new("gh")
179179
.arg("-R")
180180
.arg(&repository.to_string())
@@ -193,14 +193,24 @@ pub fn create_issue(
193193
.output()?;
194194

195195
if !output.status.success() {
196-
Err(Error::str(format!(
196+
return Err(Error::str(format!(
197197
"failed to create issue `{}`: {}",
198198
title,
199199
String::from_utf8_lossy(&output.stderr)
200-
)))
201-
} else {
202-
Ok(())
200+
)));
203201
}
202+
203+
// Output in stdout looks like
204+
//
205+
// https://github.com/rust-lang/rust-project-goals/issues/413}
206+
207+
for line in str::from_utf8(&output.stdout)?.lines() {
208+
if let Some(issue_id) = IssueId::from_url(line.trim()) {
209+
return Ok(issue_id);
210+
}
211+
}
212+
213+
Err(Error::str(format!("creating issue did not return a URL")))
204214
}
205215

206216
pub fn change_title(repository: &Repository, number: u64, title: &str) -> Result<()> {

crates/rust-project-goals/src/gh/labels.rs

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,30 @@ pub struct GhLabel {
1313

1414
impl GhLabel {
1515
pub fn list(repository: &Repository) -> Result<Vec<GhLabel>> {
16-
let output = Command::new("gh")
17-
.arg("-R")
18-
.arg(&repository.to_string())
19-
.arg("label")
20-
.arg("list")
21-
.arg("--json")
22-
.arg("name,color")
23-
.output()?;
16+
let mut limit = 128;
17+
18+
loop {
19+
let output = Command::new("gh")
20+
.arg("-R")
21+
.arg(&repository.to_string())
22+
.arg("label")
23+
.arg("list")
24+
.arg("--json")
25+
.arg("name,color")
26+
.arg("-L")
27+
.arg(format!("{limit}"))
28+
.output()?;
2429

25-
let labels: Vec<GhLabel> = serde_json::from_slice(&output.stdout)?;
30+
let labels: Vec<GhLabel> = serde_json::from_slice(&output.stdout)?;
31+
if labels.len() >= limit {
32+
// If we got exactly as many as we asked for,
33+
// we might be missing some.
34+
limit = limit * 2;
35+
continue;
36+
}
2637

27-
Ok(labels)
38+
return Ok(labels);
39+
}
2840
}
2941

3042
pub fn create(&self, repository: &Repository) -> Result<()> {

crates/rust-project-goals/src/goal.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,9 @@ impl GoalDocument {
194194
metadata_table
195195
.content
196196
.add_key_value_row(TRACKING_ISSUE_ROW, &number);
197-
self.metadata
198-
.table
199-
.overwrite_in_path(&self.path, &metadata_table)?;
197+
198+
Table::overwrite_in_path(&self.metadata.table, &self.path, &metadata_table)?;
199+
200200
Ok(())
201201
}
202202

crates/rust-project-goals/src/markwaydown.rs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,19 +79,20 @@ pub fn parse_text(text: Spanned<&str>) -> Result<Vec<Section>> {
7979
row.push(Spanned::here(String::new()));
8080
}
8181

82+
table.span.bytes.end = line.span.bytes.end;
8283
table.content.rows.push(row);
8384
} else {
8485
open_table = Some(Spanned::new(
8586
Table {
8687
header: row,
8788
rows: vec![],
8889
},
89-
line.span.clone().shrink_to_start(),
90+
line.span.clone(),
9091
));
9192
}
9293
}
9394
CategorizeLine::TableDashRow(dashes) => {
94-
if let Some(table) = &open_table {
95+
if let Some(table) = &mut open_table {
9596
if table.header.len() != dashes.len() {
9697
spanned::bail!(
9798
dashes.last().unwrap(),
@@ -108,6 +109,8 @@ pub fn parse_text(text: Spanned<&str>) -> Result<Vec<Section>> {
108109
)
109110
.wrap_str(first[0].as_ref().map(|_| "already saw table row here")));
110111
}
112+
113+
table.span.bytes.end = line.span.bytes.end;
111114
} else {
112115
spanned::bail!(dashes[0], "did not expect table header here",);
113116
}
@@ -198,18 +201,25 @@ impl Table {
198201
}
199202

200203
/// Modify `path` to replace the lines containing this table with `new_table`.
201-
pub fn overwrite_in_path(&self, path: &Path, new_table: &Table) -> Result<()> {
204+
pub fn overwrite_in_path(this: &Spanned<Self>, path: &Path, new_table: &Table) -> Result<()> {
202205
let full_text = std::fs::read_to_string(path)?;
206+
let table_span = &this.span.bytes;
207+
208+
let table_text = &full_text[table_span.start..table_span.end];
209+
assert!(
210+
table_text.starts_with("|") && table_text.ends_with("|"),
211+
"table_text doesn't appear to be a table: {table_text:?}"
212+
);
203213

204-
let mut new_text = full_text[..self.header[0].span.bytes.start].to_string();
214+
let mut new_text = full_text[..table_span.start].to_string();
205215

206216
let table_text = {
207217
let mut new_rows = vec![new_table.header.clone()];
208218
new_rows.extend(new_table.rows.iter().cloned());
209219
util::format_table(&new_rows)
210220
};
211221
new_text.push_str(&table_text);
212-
new_text.push_str(&full_text[self.rows.last().unwrap().last().unwrap().span.bytes.end..]);
222+
new_text.push_str(&full_text[table_span.end..]);
213223

214224
std::fs::write(path, new_text)?;
215225

crates/rust-project-goals/src/re.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ lazy_static! {
6767
pub static ref TRACKING_ISSUE: Regex = Regex::new(r"\[([^#/]*)/([^#/]*)#([0-9]+)\]").unwrap();
6868
}
6969

70+
lazy_static! {
71+
pub static ref GITHUB_ISSUE_URL: Regex =
72+
Regex::new(r"https://github.com/([^#/]*)/([^#/]*)/issues/([0-9]+)").unwrap();
73+
}
74+
7075
lazy_static! {
7176
pub static ref CHECKBOX: Regex = Regex::new(r"\s*[-*] \[[ x]\] ").unwrap();
7277
}

src/2025h2/FLS-up-to-date-capabilities.md

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
# Develop the capabilities to keep the FLS up to date
22

3-
| Metadata | |
4-
|:-----------------|----------------------------------------------------------------------------------|
5-
| Point of contact | @PLeVasseur |
6-
| Status | Proposed |
7-
| Tracking issue | |
8-
| Zulip channel | #t-spec |
9-
| [bootstrap] champion | @kobzol |
10-
| [lang] champion | @nikomatsakis |
11-
| [spec] champion | @PLeVasseur |
3+
| Metadata | |
4+
| :-- | :-- |
5+
| Point of contact | @PLeVasseur |
6+
| Status | Proposed |
7+
| Tracking issue | [rust-lang/rust-project-goals#391] |
8+
| Zulip channel | #t-spec |
9+
| [bootstrap] champion | @kobzol |
10+
| [lang] champion | @nikomatsakis |
11+
| [spec] champion | @PLeVasseur |
12+
1213

1314
## Summary
1415

0 commit comments

Comments
 (0)