revisit how snapshot works

[nikomatsakis]

I was thinking about the LALRPOP snapshot. Now that we have revisions published on crates.io, it is annoying that we have to carry about lalrpop-snap. It would be nice if we could instead rely on an older, published version of lalrpop. The problem with that is that if we are not careful we quickly create a chain reaching very far back. It's also annoying that end users have to build lalrpop-snap.

I was considering an alternative:

• First, convert to using a hash of lalrpop file to decide when rebuild is needed (use hash of lalrpop file to decide if rebuild is needed, not date #347).
  • This may not be important, but it makes me less nervous about some other details of this plan.
• Next, we commit the generated code for the parser into the repo.
  • This, combined with using hash, means that end-users building LALRPOP will never have to rebuild the parser.
• Finally, we modify build.rs so that when the generated code needs to be rebuilt, we run the lalrpop executable instead.
  • We should check the version number as well, and require that it is equal to the last published release.
  • This means that LALRPOP devs will have to keep the LALRPOP executable (of the correct version) installed.
• Even better, we could run cargo install locally, using --root to place the executable into the OUT_DIR or something like that.
  • Actually, we should support this generally (make it easy for build.rs to use lalrpop executable #349), as it can workaround Feature request: compile build.rs scripts in release mode even when doing a debug build rust-lang/cargo#2424.

Thoughts?

[Marwes]

Finally, we modify build.rs so that when the generated code needs to be rebuilt, we run the lalrpop executable instead.

An alternative to this could be to make lalrpop-snap an optional dependency of lalrpop. That way it is possible to develop on LALRPOP without needing the executable installed. The downside is that one must pass --features lalrpop-snap to update the commited parser in the lalrpop crate.

[matklad]

For fall, I use a slightly more "worse as better" approach.

1. commit generated code to the repository, so that no bootstrapping is needed for usual build.
2. Don't have any build.rs. Instead, manually run (via shell script/justfile, or cargo run --bin bootstrap) parser generator on itself, when this is needed.
3. As a part of test-suite, run the parser generator on itself to verify that the generated parser is fresh.

[nikomatsakis]

@matklad hmm, I could live with that.

[nikomatsakis]

Though I think it sounds maybe harder than what I suggested =)

[nikomatsakis]

@matklad but also, it seems like detecting a hash mismatch is pretty easy, so the build.rs could just look for that and error if found?

[matklad]

@nikomatsakis I am not entirely sure...

I think there's an important difference between other users of LALRPOP and LALRPOP bootstrapping process.

For other users, we need to rebuild if the hash of *.lalrpop file is changed.

For LALRPOP itself, there's an additional condition. Specifically, I think we'd love to make sure that bootstrapping has reached a fixed point: that is, if we generate code for lrgrammar.lalrpop by current LALRPOP, we should get exactly the same parser file, which is committed to the repository.

That means, we should potentially regenrate parser if either the grammar file is changed, or the code of lalrpop itself is changed.

Another thing to note is that the end users of LALRPOP don't actually need to execute LALRPOP's own build.rs script at all, because the generated code is committed to the repository. I wonder if we can add build.rs to ignored files in Cargo.toml, so that it is not uploaded to crates.io at all and is only used for local development. If this works, I think the following solution might work:

1. commit the generated code to the repository, maintaining the invariant that it is exactly the same code that you would get by running lalrpop on itself.
2. enforce the invariant via a test (I am not sure we can enforce it reliably implicitly, by writing a magical build.rs)
3. write a build.rs file for LALRPOP developers, which regenrates the parser, using lalrpop from PATH or from target/debug/lalrpop, if the hash of lrgrammar.lalrpop is changed (and don't forget to println!("rerun-if-changed=src/parser/lrgramar.lalrpop") to avoid rebuilds by Cargo). This won't catch "I modified code gen" cases, but there's a test for that!
4. exclude build.rs from Cargo.toml.