Validate triagebot.toml in tidy/CI

Would've prevented #106102, which broke in #105661.

We should at least validate the toml file is valid syntax, but ideally we'd validate more context-specific stuff.

Mentoring instructions: Add a new check to tidy around here that parses triagebot.toml to make sure it's valid toml:

Lines 41 to 58 in 17395b4

    
           pub mod alphabetical; 
        
           pub mod bins; 
        
           pub mod debug_artifacts; 
        
           pub mod deps; 
        
           pub mod edition; 
        
           pub mod error_codes_check; 
        
           pub mod errors; 
        
           pub mod extdeps; 
        
           pub mod features; 
        
           pub mod mir_opt_tests; 
        
           pub mod pal; 
        
           pub mod primitive_docs; 
        
           pub mod style; 
        
           pub mod target_specific_tests; 
        
           pub mod ui_tests; 
        
           pub mod unit_tests; 
        
           pub mod unstable_book; 
        
           pub mod walk;

See the existing tidy files for an example of how to write a tidy check.

Hi, I'm a new contributor here and thought of picking this as my first issue. From what I understand, a new crate (something like triagebot.rs) needs to be made for this issue right? Where we can parse the toml and see if it is valid.

@aadityadhruv

@aadityadhruv a new module, not a new crate. The module should be part of tidy so check runs on x test tidy.

Right, that makes sense. Thank you!

Hi, sorry for the late response, I was out traveling. I'm currently confused about a certain thing. Currently what I am trying to do is manually check the file for valid syntax, by going over it line by line. Is this the correct method to do this, or should I be using an already existing crate like toml? I would assume that extra crate dependencies aren't probably wanted.

Also, I was wondering something about Path in the check function. For this module, would I need to check/filter any paths like some other modules do? I was assuming that I will simply open path + 'tiragebot.toml' and read that file.

Any help is appreciated!

should I be using an already existing crate like toml?

Yes, please use toml, don't use regex or anything like that.

I personally would prefer to see this implemented in triagebot itself instead of tidy. That would make it work for all repositories. It should be fairly trivial to add a handler that checks for a PR that modifies triagebot.toml and to validate it, and post a comment if it is not correct. It can even use the Config type to verify that the actual structure matches. Bonus points would to also use serde-ignored to enforce that unknown fields aren't included.

@aadityadhruv

@aadityadhruv are you still working on this?

Hi, yeah I'm sorry, I'm still working on it. I've been pretty busy the last couple of days, I'll get it done this week itself. Hope that's ok!

Hi, yeah I'm sorry, I'm still working on it. I've been pretty busy the last couple of days, I'll get it done this week itself. Hope that's ok!

Yeah! it's alright

I personally would prefer to see this implemented in triagebot itself instead of tidy. That would make it work for all repositories. It should be fairly trivial to add a handler that checks for a PR that modifies triagebot.toml and to validate it, and post a comment if it is not correct. It can even use the Config type to verify that the actual structure matches. Bonus points would to also use serde-ignored to enforce that unknown fields aren't included.

Sorry for being slow with the issue, but along with being busy, I've been kinda stuck, and was unsure if I should ask for help. I am currently unsure if I should be doing work in tidy or the triagebot repository. Should I first try to get the tidy check working and then maybe open an issue on the triagebot repo?

Here is the code I had written (it does not work since I do not know what the configuration of the toml is):

use crate::walk::{filter_dirs, walk};
use serde::Deserialize;
use std::path::Path;

#[derive(Deserialize)]
struct Config {}

pub fn check(path: &Path, bad: &mut bool) {
    walk(path, &mut |path| filter_dirs(path), &mut |entry, contents| {
        let file = entry.path();
        let filename = file.file_name().unwrap();
        if filename != "triagebot.toml" {
            return;
        }
        let conf: Result<Config, toml::de::Error> = toml::from_str(contents);
        match conf {
            Ok(_) => {}
            Err(_err) => {
                tidy_error!(bad, "{} is an invalid toml file", file.display())
            }
        }
    });
}

I am unsure what would be the right next step here.

Here is a PR for the same: #106559

I personally would prefer to see this implemented in triagebot itself instead of tidy. That would make it work for all repositories. It should be fairly trivial to add a handler that checks for a PR that modifies triagebot.toml and to validate it, and post a comment if it is not correct. It can even use the Config type to verify that the actual structure matches. Bonus points would to also use serde-ignored to enforce that unknown fields aren't included.

👍 #106559 (comment)

Just thought I'd drop by and say that this would have been nice to avoid #106848 😅

Ah, that wouldn't have been caught actually - the toml syntax was valid, the file just didn't exist in the repository. Checking if the glob has at least one match seems possible.

I see the PR up but not sure if anyone is still working on the requested changes?

Yeah I'm currently working on it in the triagebot repo, I haven't opened a PR/Issue yet for it.

@rustbot

@rustbot claim

feat: valdiate proposed change in triagetoml for PRs ✨

Can someone please review this change: rust-lang/triagebot#1730

feat: valdiate proposed change in triagetoml for PRs ✨

Navigation Menu

Repository navigation

Validate `triagebot.toml` in tidy/CI #106104

2 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	pub mod alphabetical;
	pub mod bins;
	pub mod debug_artifacts;
	pub mod deps;
	pub mod edition;
	pub mod error_codes_check;
	pub mod errors;
	pub mod extdeps;
	pub mod features;
	pub mod mir_opt_tests;
	pub mod pal;
	pub mod primitive_docs;
	pub mod style;
	pub mod target_specific_tests;
	pub mod ui_tests;
	pub mod unit_tests;
	pub mod unstable_book;
	pub mod walk;

Validate triagebot.toml in tidy/CI #106104

Description

Activity

jyn514 commented on Dec 24, 2022

aadityadhruv commented on Dec 24, 2022

jyn514 commented on Dec 24, 2022

aadityadhruv commented on Dec 24, 2022

aadityadhruv commented on Dec 28, 2022

jyn514 commented on Dec 28, 2022

ehuss commented on Dec 29, 2022

dotdot0 commented on Jan 2, 2023

aadityadhruv commented on Jan 2, 2023

dotdot0 commented on Jan 2, 2023

aadityadhruv commented on Jan 5, 2023

aadityadhruv commented on Jan 5, 2023

2 remaining items

aadityadhruv commented on Jan 7, 2023

jyn514 commented on Jan 10, 2023

BoxyUwU commented on Jan 14, 2023

jyn514 commented on Jan 14, 2023

0xJepsen commented on Jan 15, 2023

aadityadhruv commented on Jan 15, 2023

meysam81 commented on Oct 7, 2023

meysam81 commented on Oct 8, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Validate `triagebot.toml` in tidy/CI #106104