Skip to content

Clear header directory before overwriting it #1211

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Clear header directory before overwriting it #1211

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
16 changes: 16 additions & 0 deletions crates/cxx-qt-lib/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,21 @@ fn write_headers_in(subfolder: &str) {
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

side point, wonder if sccache uses the hash of the file or the mtime ?

Eg does modifying a file in the header dir (by deleting and writing the same contents again) cause a cache miss ? Or is it comparing the contents anyway.

Looking at the times in CI it seems to help with macOS and Windows Qt 5 specifically 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now doing a clean run after rebasing things vs a run on main both with the same cache we seem to have.

Runner Before After
macOS 13 Qt 5 7m 42s 7m 33s
macOS 14 Qt 6 6m 15s 7m 28s
Ubuntu Qt 5 5m 50s 5m 43s
Ubuntu Qt 6 6m 59s 8m 4s
Windows Qt 5 10m 16s 14m 9s
Windows Qt 6 MSVC2019 12m 19s 12m 57s
Windows Qt 6 MSVC2022 13m 1s 24m 57s

Seems that this makes Qt 6 1min slower for Linux and macOS, then Windows is curious too. Would probably need more reruns to be sure of the timings.

But it does make me wonder if we should be more intelligent than just delete everything 🤔

Copy link
Collaborator

@LeonMatthesKDAB LeonMatthesKDAB Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The thing with our CI is that in most cases, this change shouldn't affect it.
Especially on Windows and macOS, we're doing only clean builds, so there is nothing to delete anyway, so this doesn't change anything.

The interesting case is the cargo_rerun test case under Linux, as that would show a difference in caching behavior.


fn clear_include_directory() {
if !header_dir().exists() {
return;
}
for entry in std::fs::read_dir(header_dir().as_path()).expect("Failed to read header directory")
{
let path = entry.expect("Failed to read path").path();
if path.is_dir() {
std::fs::remove_dir_all(path).expect("Failed to delete directory");
} else {
std::fs::remove_file(path).expect("Failed to delete file");
}
}
}

fn write_definitions_header() {
// We cannot ensure that downstream dependencies set the same compile-time definitions.
// So we generate a header file that adds those definitions, which will be passed along
Expand Down Expand Up @@ -102,6 +117,7 @@ fn main() {
let qtbuild = qt_build_utils::QtBuild::new(vec!["Core".to_owned()])
.expect("Could not find Qt installation");

clear_include_directory();
write_headers();

let emscripten_targeted = match std::env::var("CARGO_CFG_TARGET_OS") {
Expand Down
Loading