Skip to content

compiletest: track LLDB inputs in up-to-date checks #160137

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

Closed
ArshLabs wants to merge 1 commit into from
Closed

compiletest: track LLDB inputs in up-to-date checks #160137

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
13 changes: 12 additions & 1 deletion src/tools/compiletest/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,16 @@ fn common_inputs_stamp(config: &Config) -> Stamp {
"src/etc/gdb_load_rust_pretty_printers.py",
"src/etc/gdb_lookup.py",
"src/etc/gdb_providers.py",
"src/etc/lldb_batchmode",
"src/etc/lldb_lookup.py",
"src/etc/lldb_providers.py",
];
for file in &pretty_printer_files {
let path = src_root.join(file);
stamp.add_path(&path);
}
let lldb_batchmode = src_root.join("src/etc/lldb_batchmode");
stamp.add_path(&lldb_batchmode);
stamp.add_dir(&lldb_batchmode);
Comment on lines +232 to +233

@Mark-Simulacrum Mark-Simulacrum Aug 8, 2026

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.

Hm, this makes me wonder if we should make add_path detect whether you're adding a directory and recursively add in that case. It's not obvious to me why you would only want to stamp the directory itself, and it seems easy to not notice that you're doing a partial stamp.

@jieyouxu jieyouxu Aug 9, 2026

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.

Yeah, that feels broadly more reasonable


stamp.add_dir(&src_root.join("src/etc/natvis"));

Expand Down Expand Up @@ -575,6 +577,15 @@ fn is_up_to_date(
for path in files_related_to_test(&cx.config, testpaths, aux_props, variant.revision()) {
inputs_stamp.add_path(&path);
}
if variant.debugger == Some(Debugger::Lldb) {
let test_dir = testpaths.file.parent().unwrap();
inputs_stamp.add_path(test_dir);
let lldb_input = test_dir.join("lldb_input");
if lldb_input.is_dir() {
inputs_stamp.add_path(&lldb_input);
inputs_stamp.add_dir(&lldb_input);

@Mark-Simulacrum Mark-Simulacrum Aug 8, 2026

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.

These files look potentially generated? E.g., looking at tests/debuginfo/basic-types/lldb_input/non_windows.json I see bless_metadata which doesn't look written by humans... maybe we should include a __comment field in those files with some annotation of what they are?

I'm not clear from the PR description that added these files (#158298) on whether they are always 'outputs' from some process or true inputs.

cc @Walnut356 @jieyouxu

@jieyouxu jieyouxu Aug 9, 2026

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.

They're intended to be generated (as in the JSON files). The bless metadata was intended to be a record of how the metadata was produced, and without reblessing, bless metadata shouldn't participate in the "diff" of the JSONs. I think __comment could make sense, or maybe we could try to have a bit more elaboration in r-d-g about what the general sections are for these.

cc @Kobzol too

@Kobzol Kobzol Aug 10, 2026

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.

Maybe just renaming lldb_input to something like lldb_output, lldb_generated, or something like that, would make it more obvious?

}
}

// If no relevant files have been modified since the stamp file was last
// written, the test is up-to-date.
Expand Down
Loading