eliom icon indicating copy to clipboard operation
eliom copied to clipboard

Dunify

Open chrismamo1 opened this issue 6 years ago • 13 comments

Eliom now compiles in ~17s on my machine, whereas the master branch takes up to 100s.

This also factors the eliom PPX into a separate package, although I suspect it would be possible to avoid doing that.

It also completely removes the syntax package, which I believe is necessary as dune actively discourages camlp4 use. This will break ocsigen-start, since that still relies on Macaque's camlp4 extension, so it cannot use PPX.

chrismamo1 avatar Apr 12 '19 09:04 chrismamo1

@Drup thank you very much for the input.

I've pulled all the compiler warning fixes out, and will submit a new PR once this one has been dealt with.

For simplicity reasons, I think it would be best to pull the PPX and Camlp4 rewriters into separate opam packages, with eliom itself depending on the PPX package.

As for your specific concerns:

  • I do not see calls to the type ppx, how does that work ?

    I'm also unsure about this. I'm able to build ocsigen-toolkit and ocsigen-start with the library as-is, which I honestly didn't expect to work precisely because I've done nothing to handle the generation of .type_mli files within eliom, and I'm a bit unsure of what I could concretely do with those.

  • you renamed .eliom[i] files in eliom/.ml[i]. This doesn't seem necessary.

    I've undone this.

  • You currently suppose that we have two building scheme, one for client and one for server. This is not the case with my new implementation. It would be nicer to only suppose you have one build scheme that output 2 things (and which happens to call 2 ppxs currently). This would provide a better way forward.

    I'm not sure what you're talking about when you mention your new implementation. Are you referring to #459 ?

chrismamo1 avatar Apr 16 '19 13:04 chrismamo1

For simplicity reasons, I think it would be best to pull the PPX and Camlp4 rewriters into separate opam packages, with eliom itself depending on the PPX package.

Agreed, but keep everything in this repository for now. Dune works particularly well for this, and opam support it just fine. And please don't push things to the opam repository before it's discussed and we have reached a stable solution. In particular it's completely unacceptable to have some eliom packages pointing to github repositories that are not under the ocsigen umbrella.

I'm also unsure about this. I'm able to build ocsigen-toolkit and ocsigen-start with the library as-is, which I honestly didn't expect to work precisely because I've done nothing to handle the generation of .type_mli files within eliom, and I'm a bit unsure of what I could concretely do with those.

If the typing files are not available, the typing is basically ignored. This is wrong as it means injections and fragments are unchecked and makes eliom's typing completely useless. Arguably, it should lead to an error, but unfortunately it currently doesn't. The fact that eliom code compiles is not sufficient to assert that eliom works. You also need wrong code to fail. :)

I'm not sure what you're talking about when you mention your new implementation. Are you referring to #459 ?

Yes.

Drup avatar Apr 16 '19 13:04 Drup

@chrismamo1 are you still working on this?

rgrinberg avatar Aug 13 '20 18:08 rgrinberg

@chrismamo1 are you still working on this?

Not actively, no.

chrismamo1 avatar Aug 14 '20 15:08 chrismamo1

I see. I was wondering if you were interested in upstreaming eliom support in dune itself. It would be nice if people didn't have to manually write eliom/eliomi rules :)

rgrinberg avatar Aug 15 '20 07:08 rgrinberg

I think dune support would be a tremendous help. Judging from my own experience, using manual rules is infeasible. Just getting it to compile is straight forward, but getting the dependencies right for server-client type-checking is tricky. That also means adding dune support may be less straight forward than one would think. I hope that doesn't sound too scary. Someone correct my if I'm wrong, but the rough approach to compiling an Eliom applications is:

  • Build dependencies separately for server and client side, these need to be included in the ruleset somehow.
  • Build the server side as a library using eliom.ppx.server, both native and bytecode are supported by the OCSIGEN server.
  • Build once again in the server side environment using eliom.ppx.type and -infer the result to produce mli files for parts shared with the client.
  • Build the client side bytecode using the eliom.ppx.client, which for each compilation unit will use the corresponding inferred mli file from the server side for the additional type-checking.
  • The bytecode can then be compiled to JavaScript with the built-in js_of_ocaml rule.

We should take advantage of the dependencies in the first step to parallelize across the succeeding steps. Note that a client side compilation unit will depend on the corresponding server side inferred type in addition to the regular dependencies, and the inferred type may have it's own dependencies on the server side.

One thing we need to consider is that client-side must have a different build directory from the server side since the module names of the compilation units overlap.

paurkedal avatar Aug 16 '20 10:08 paurkedal

@paurkedal Indeed that sounds involved. It seems like you know quite a bit of what's required. If you're interested, I can help you understand the require parts of dune needed to implement this feature.

One thing we need to consider is that client-side must have a different build directory from the server side since the module names of the compilation units overlap.

That would be a good first step to look into in dune.

rgrinberg avatar Aug 17 '20 07:08 rgrinberg

@rgrinberg I would like to take you up on that offer in a couple of months when I have more time to spare, but just to get the thoughts going:

We could try to generalize dune to support eliom-style flows or we could make a specialised rule. I think the latter is most realistic given how I understand the dune design as an end-user. Keeping all sources in a single directory for server and client is encouraged, using .client.ml, .server.ml, and .shared.ml for non-eliom files, so the application is best declared in a single dune file. Since it makes no sense to only build one side of an eliom application and since the parts have non-trivial inter-dependencies, I think the best option is to also use a single stanza. Not to get into details about syntax, a flattered stanza might that look like

(eliom_application
 (name ...) (public_name ...)
 (eliom_modules ...) (shared_modules ...) (client_modules ...) (server_modules ...)
 (shared_libraries ...) (client_libraries ...) (server_libraries ...)
 (shared_flags ...) (client_flags ...) (server_flags ...)
 (shared_preprocess ...) (client_preprocess ...) (server_proprocess ...)
 ...)

but we could also make server library and client executable nested with shared properties factored up.

Next, it would also be good to support building Eliom libraries, which would consist of two libraries, a server and client, where the latter is bytecode only. Otherwise the same logic regarding inferred mli files, PPXes, and dependencies applies.

paurkedal avatar Aug 20 '20 19:08 paurkedal

What about eliom libraries? Isn't it possible to define a library with client & server side components?

rgrinberg avatar Aug 20 '20 20:08 rgrinberg

Yes, I think an (eliom_library ...) stanza would be quite similar. The public names would by convention be foo.server and foo.client. (Also for the application I assumed above that the library and executable names would be inferred from the declared names.)

paurkedal avatar Aug 20 '20 20:08 paurkedal

I see. I wonder if it's simpler to just an eliom field to existing stanzas. I'd prefer to introduce new stanza only if some existing fields of library or executable don't apply to eliom.

rgrinberg avatar Aug 20 '20 20:08 rgrinberg

But note that there is a tight relation between the client and server side, both since declarations of modules and library dependencies will overlap (i.e. the (shared_* ...) and (eliom_* ...) will be the common case) and due to tight dependencies from servers side to client side.

paurkedal avatar Aug 20 '20 20:08 paurkedal

I imagine support for an (eliom (server_libraries ...) (client_libraries ...) (server_flags ...) .....) stanza for both executables and library stanzas would be better than brand new top-level stanzas.

clembu avatar Aug 21 '20 14:08 clembu