Use include_bytes macro to avoid the need for a build script #199

de-vri-es · 2019-01-10T21:12:39Z

This PR adds an otherwise unused include_bytes!(...); statement in the generated template code for each template in the context map and for each included template. This allows the compiler to detect file level dependencies without the need for a custom build script.

I believe I caught all the relevant file dependencies: extends, imports, includes and of course the template itself (if it is backed by a file, which requires some checks).

This gives more granular file-level dependency information than the build script, although I don't believe that actually gives any practical benefits right now.

Feedback welcome.

djc · 2019-01-10T21:19:38Z

Very nice, thanks for implementing this! I've thought about it myself a few times. Have you checked that this has the desired effect? (That is, touching a template file will trigger recompilation even without a build script.)

One issue here is that I think it should probably include bytes for all templates used, including inherited templates, imported templates and included templates. That might be a bit more work to get right, and will actually need something that I've so far resisted doing, which is walking the entire template AST during the Context creation phase.

Are you up for taking a look at that? It basically means adapting Context::new() to do a full traversal of the root template and any inherited templates to discover what templates are used, and then including bytes for all of them.

djc · 2019-01-10T21:21:08Z

Update: oh wait, you have handled all that stuff! More power to you!

So, that means only the includes probably don't work correctly right now.

de-vri-es · 2019-01-10T21:23:07Z

I believe I handled includes properly too ;)

I did test locally, and the rebuilds worked as intended. I did not test multi-level includes or extends but from the code I believe it should work correctly recursively already.

/edit: To clarify, I did test extends, but not multi-level extends.

/edit2: Currently, all travis tests for nightly are failing because of this: rust-lang/rust#57488

de-vri-es · 2019-01-10T21:31:02Z

Hmm, it looks like the path comparison to avoid include_bytes!() for the fake path of templates from rust source break on Windows. Or otherwise something else is going wrong on Windows =]

de-vri-es · 2019-01-10T21:35:55Z

I also tested if a rebuild is properly triggered for nested includes, imports and extends now. On Linux, it works as expected. That still leaves the windows issues to fix.

djc · 2019-01-10T21:38:41Z

Did you test includes in a for or if block?

de-vri-es · 2019-01-10T21:46:31Z

I did now. Regardless of the true/false value of the condition, changes to the included file trigger a rebuild (which makes sense, since the include_bytes!() is done compile time for the generated code, not runtime).

I think the windows issue is caused by backslashes being parsed into escape sequences somewhere. Not sure why that happens though.

de-vri-es · 2019-01-10T21:51:38Z

Oh, of course. We're outputting source and there's backslashes in the string literal. Is there a proper way to escape string literals in the output of a procedural macro?

djc · 2019-01-10T21:53:53Z

Not sure, maybe look at the quote crate API?

djc · 2019-01-10T21:56:42Z

Also as follow-up (ideally in a separate commit) get rid of all the build script stuff? That includes in the README, the build script in the test crate and the helper code in the crates that supports the build script API.

This is meant to allow the compiler to understand the dependency between the generated code and the template source. It removes the need for a build script.

de-vri-es · 2019-01-10T22:08:52Z

Right, I was meaning to ask if it should be deprecated, but if it can be removed, awesome :) Will do once the tests all pass.

In the mean time, to give you an idea of what the resulting generated code looks like, this was generated from a template with nested imports, nested extends and a conditional include with a nested include (now using quote!{} to fix the generated output if it has escape sequences):

impl < 'a > ::askama::Template for Hello< 'a > {
    fn render_into(&self, writer: &mut ::std::fmt::Write) -> ::askama::Result<()> {
        include_bytes ! ( "/home/maarten/dev/rust/lummel/templates/base2.html" ) ;
        include_bytes ! ( "/home/maarten/dev/rust/lummel/templates/base.html" ) ;
        include_bytes ! ( "/home/maarten/dev/rust/lummel/templates/import1.html" ) ;
        include_bytes ! ( "/home/maarten/dev/rust/lummel/templates/import2.html" ) ;
        include_bytes ! ( "/home/maarten/dev/rust/lummel/templates/hello.html" ) ;
        write!(
            writer,
            "<!DOCTYPE html>\n<html>\n<head>\n\t<title>{expr0}</title>\n\t\n\t<link rel=\"stylesheet\" type=\"text/css\" href=\"/css/main.css\">\n\t\n</head>\n<body>\n<p>Hello world!</p>\n",
            expr0 = &::askama::MarkupDisplay::from(&self.title),
        )?;
        if self.include_foo {
            writer.write_str("\n")?;
            include_bytes ! ( "/home/maarten/dev/rust/lummel/templates/foo.html" ) ;
            writer.write_str("FOO\n")?;
            include_bytes ! ( "/home/maarten/dev/rust/lummel/templates/foo2.html" ) ;
            writer.write_str("FOO2")?;
            writer.write_str("\n")?;
        }
        writer.write_str("\n</body>\n</html>")?;
        Ok(())
    }
    fn extension() -> Option<&'static str> {
        Some("html")
    }
}
impl < 'a > ::std::fmt::Display for Hello< 'a > {
    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
        ::askama::Template::render_into(self, f).map_err(|_| ::std::fmt::Error {})
    }
}

de-vri-es · 2019-01-10T22:27:12Z

The tests are now passing, except for nightly still because of rust-lang/rust#57488.

I added a commit to remove the rerun_if_templates_changed() function and updated the README. I added a note that the build script is no longer needed, but maybe that note shouldn't be there. Let me know if you want me to reword or remove it.

It would probably also be prudent to update the version number in the README just before the next release, since 0.8.0 still needs a build script.

de-vri-es · 2019-01-11T09:52:19Z

Force pushed to re-trigger CI, it now passed =)

djc · 2019-01-11T11:21:12Z

Yeah, can you remove the note from the README again? Also I changed my mind, so maybe bring back rerun_if_templates_changed with an empty implementation and a #[deprecated] attribute?

After that I'm done, promise.

de-vri-es · 2019-01-11T14:47:53Z

Righty, removed the note from the readme and re-added the old function, but deprecated and with updated doc.

djc · 2019-01-11T15:31:20Z

Thanks, great work!

de-vri-es · 2019-01-12T11:59:47Z

Thanks for the review :)

de-vri-es force-pushed the include-bytes branch from d57dbc5 to cd04ede Compare January 10, 2019 21:14

de-vri-es force-pushed the include-bytes branch from cd04ede to 27f1399 Compare January 10, 2019 22:03

Inject template file into generated source as bytes.

227b39f

This is meant to allow the compiler to understand the dependency between the generated code and the template source. It removes the need for a build script.

de-vri-es force-pushed the include-bytes branch from 27f1399 to 227b39f Compare January 10, 2019 22:04

de-vri-es force-pushed the include-bytes branch 2 times, most recently from bb5e071 to 74fc90c Compare January 11, 2019 09:13

Remove rerun_if_templates_changed and update README.

7208aaf

de-vri-es force-pushed the include-bytes branch from 74fc90c to 7208aaf Compare January 11, 2019 14:46

djc approved these changes Jan 11, 2019

View reviewed changes

djc merged commit b3c1fc1 into djc:master Jan 11, 2019

jxs mentioned this pull request Sep 2, 2021

Ability to run both SQL and RS migrations rust-db/refinery#170

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use include_bytes macro to avoid the need for a build script #199

Use include_bytes macro to avoid the need for a build script #199

de-vri-es commented Jan 10, 2019

djc commented Jan 10, 2019

djc commented Jan 10, 2019

de-vri-es commented Jan 10, 2019 •

edited

de-vri-es commented Jan 10, 2019

de-vri-es commented Jan 10, 2019

djc commented Jan 10, 2019

de-vri-es commented Jan 10, 2019 •

edited

de-vri-es commented Jan 10, 2019

djc commented Jan 10, 2019

djc commented Jan 10, 2019

de-vri-es commented Jan 10, 2019

de-vri-es commented Jan 10, 2019

de-vri-es commented Jan 11, 2019

djc commented Jan 11, 2019

de-vri-es commented Jan 11, 2019

djc commented Jan 11, 2019

de-vri-es commented Jan 12, 2019

Use include_bytes macro to avoid the need for a build script #199

Use include_bytes macro to avoid the need for a build script #199

Conversation

de-vri-es commented Jan 10, 2019

djc commented Jan 10, 2019

djc commented Jan 10, 2019

de-vri-es commented Jan 10, 2019 • edited

de-vri-es commented Jan 10, 2019

de-vri-es commented Jan 10, 2019

djc commented Jan 10, 2019

de-vri-es commented Jan 10, 2019 • edited

de-vri-es commented Jan 10, 2019

djc commented Jan 10, 2019

djc commented Jan 10, 2019

de-vri-es commented Jan 10, 2019

de-vri-es commented Jan 10, 2019

de-vri-es commented Jan 11, 2019

djc commented Jan 11, 2019

de-vri-es commented Jan 11, 2019

djc commented Jan 11, 2019

de-vri-es commented Jan 12, 2019

de-vri-es commented Jan 10, 2019 •

edited

de-vri-es commented Jan 10, 2019 •

edited