Skip to content
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.

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

Prevent duplication of include_bytes! statements #85

Closed

Conversation

GuillaumeGomez
Copy link
Contributor

@GuillaumeGomez GuillaumeGomez commented Jul 21, 2024

Fixes #72.

@@ -88,6 +89,23 @@ impl<'a> Generator<'a> {
Ok(buf.buf)
}

fn generate_include_bytes<S: AsRef<str>>(&mut self, buf: &mut Buffer, path: S) {
static CACHE: OnceLock<OnceMap<String, &'static ()>> = OnceLock::new();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking about it, I don't think using a cache this why is sane:

  • Given the file layout.html is used in compiling units a.rs and b.rs, then the include might wind up only in one of the generated objects, say, a.rs.
  • Afterwards, we remove a.rs and any mentions to it.
  • Then we alter layout.html: b.rs does not mention the dependency, so the change won't cause a recompilation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we remove a.rs, the crate will be recompiled and layout.html will be used in b.rs. Or did I miss something?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You cannot assume that the whole crate is recompiled when incremental compilation is enabled, can you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hum... It's a good question. But since you remove a.rs, it also means you remove mod a somewhere else, so in this case, I wonder if the whole crate isn't simply rebuilt...

Copy link
Collaborator

@Kijewski Kijewski Jul 22, 2024

Choose a reason for hiding this comment

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

But what if the add more indirection? (Assume the usual folder structure):

mod x1 {
    mod x2 {
        mod a;
    }
    mod x3 {
        mob b;
    }
}

Then removing mod a will cause a recompilation of mod x2, but not x1. If the need to rebuild would bubble up, then there could be no incremental compilation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fact that we're debating over this is already a pretty good sign that we should not do it. Closing then! :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe once upon a time rust-lang/rust#99515 gets implemented. :) If I understand the proposal correctly, then this could be a game changer, and we could even use spans inside the template file to show error messages, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Only use include_byte! on a file once across templates
2 participants