fallback, promote, etc.

[trefis]

In merlin, starting from a parser_raw.mly file, we generate a bunch of files (and commit some of them):

• parser_raw.ml and parser_raw.mly which are generated by menhir --infer and are commited in the source tree
• parser_raw.cmly which is generated by the same menhir invokation
• parser_explain.ml, parser_printer.ml and parser_recover.ml which are generated from parser_raw.cmly, and are commited in the source tree.

When switching to jbuilder, a lot of manual rules had to be written, as can be seen here and there; i.e. I just run jbuilder build @src/ocaml/preprocess/406/preprocess when I want to refresh the committed files listed above.

There are two reasons to this hackery:

• we didn't want to set the mode to promote because we don't want this to run on every build: the grammar basically never changes
• we can't set the mode fallback because the .cmly file is not commited (otherwise I'd have been satisfied with setting the mode to fallback and temporarily changing it to promote when I need to regen the various generated files).

I think the current restriction on fallback makes sense, and I actually think having to change from fallback to promote temporarily would be somewhat annoying over time.

I believe there are a few options at this point:

1. I actually misunderstood what's happening and my workflow is already supported if I do
2. we introduce a new mode (promotable?) which is symmetric to promote: the rule would be ignored unless one passes --auto-promote on the command line.
3. (suggested by @diml) we get a smarter version of promote which doesn't regenerate things if its dependencies haven't changed; that requires saving a hash of the dependencies somewhere in the repo though.

I personally find option 3 a bit distasteful (I don't want to commit hashes), and like option 2 better. But you guys might have a different opinion.

If you think option 2 is acceptable I'd be willing to try to implement it.

[ghost]

Personally I still believe option 3 will provide the best user experience. If one is willing to commit a whole generated file, a 32 bytes hash to commit is not that bad.

[rgrinberg]

@trefis, I think this needs feedback from you to proceed.

I'm not sure I understand this point however:

we didn't want to set the mode to promote because we don't want this to run on every build: the grammar basically never changes

It would only run once and then be reused from the build dir then. Or do you mean every install build?

[trefis]

I think this needs feedback from you to proceed.

That wasn't obvious to me, sorry :p I still find option 3 distasteful (and currently still feel like I wouldn't use it even if it was available), and would still be happy with option 2. But I'd understand if you guys were against it.

Or do you mean every install build?

Partly. That was what concerned me most, but I now see that -p <pkg name> implies that promote rules will be ignored, which sounds pretty good.

So I guess with setting the mode to promote I will just have to regenerate the files on every "clean" build, which is a bit sad but perhaps not too bad (after all, there should be no reason for me to dune clean if incremental builds work as they should).

[trefis]

I've discussed this some more with @diml this morning. Considering that I'm now OK with the idea of switching to promote, and that there are many more important things to work on. I think we can close this one.

[trefis]

And of course, things cannot be simple. In the generated .ml file menhir includes something like this

# LOC "/home/<user>/.opam/<switch>/lib/menhir/stardard.mly"

which will be different for each person invoking menhir. That is: if we commit the generated .ml file, we'll keep getting a diff if the committed version was generated by someone else.

---

For the record: the reason why I like option 2 is that it gives control to the user, and the reason why I dislike option 3 is because it implies committing even more garbage in the repository.

[trefis]

And actually, that's not the only thing that would change based on who invokes menhir, so really, promote is not good for us.

We could also at some point just add a menhir dependency to the merlin package and stop all this hackery. But in the meantime I think I'll give a go at implementing option 2. :>

[rgrinberg]

We could also at some point just add a menhir dependency to the merlin package and stop all this hackery.

Before we do this, I think menhir should be ported to dune first. Otherwise, the ocaml platform that is built from a single workspace might not happen this year after all :)

In the generated .ml file menhir includes something like this

Isn't this already a problem? I see quite a bit of this in the current merlin:

# 219 "/home/trefis/.opam/4.06.1/lib/menhir/standard.mly"

If you're already vendoring the menhir stdlib, maybe it makes sense to vendor standard.mly? Would that be at all possible with the current menhir. cc @fpottier

[trefis]

Isn't this already a problem? I see quite a bit of this in the current merlin: [...]

That's what I was referring to. It is not currently a problem because of the horrible hand-written rules hackery that I sprinkled all over merlin's dune files. But if we were to switch to simply use the menhir stanza with the promote mode, then we'd be in trouble.

If you're already vendoring the menhir stdlib, maybe it makes sense to vendor standard.mly

That has been mentioned for the compiler as well. Currently we're in exactly the same situation regarding menhir as the compiler, so I think we'll on that front when the compiler does.

[rgrinberg]

Thinking about this again, perhaps this problem could be fixed with changing the mode based on the profile. For example:

(rule
 (action ..)
 (mode (if (= %{profile) menhir) promote fallback))

Then you would only promote if you run $ dune build --profile menhir.

[kit-ty-kate]

For posterity, I also got the same issue in https://github.com/kit-ty-kate/spdx_licenses and I ended up using:

 (mode promote)
 (enabled_if (= %{profile} "regenerate"))

given (mode (if ...)) isn't an accepted syntax