Cross-compiling sedlex (ppx) for windows #2252
Comments
|
This is actually the expected behaviour. Inside a given package, all the artifacts are meant for the same target. When you think about it, it makes sense for ppx.exe as well. It would be odd to have the object files of the library be Windows object files and the ppx.exe, which in the end is just the library linked with the driver be a Linux executable. Having all the object files of the library be Linux object files would complicate the system a lot. In particular, what would we do for libraries that are needed both for the build and at runtime? What the current scheme means is that in order to cross-compile a project using sedlex, you need both sedlex and sexlex-windows. |
|
@diml Hmm okay. In this case, what is the recommended And I get: Any advice on how to properly do that would be greatly appreciated! |
|
I'd argue that the most likely scenario is that the generated artifacts are meant to be run by the system compiling them and, thus, that the executable should be compiled for the hosting architecture. Pretty sure cc/ @rgrinberg for opinion 🙂 |
|
The model we implement in dune is the following: we have two parallel worlds: the one for the one for the build and the one for the target. When we need to run an executable during the build, we get it in the build world. It seems to me that the model you are thinking of is one where for each library/executable, you have to make a decision of whether the element is needed only for the build or whether it is only needed at runtime. Which is quite limiting since you cannot use the same library for both. |
|
This is not the way the |
|
Note that |
|
Where do you install the Windows binaries in this world? |
|
It's complicated 🙂 I believe most people use the cross-compiling environment as leverage to produce binaries that aren't living inside the cross-compiling path. Typically, most of the packages in opam-cross-windows are libraries. One way to keep this conversation going could be to figure out the right call to compile using |
I see, I was wondering about that.
I have no idea ¯_(ツ)_/¯ To be clear, I'm not against solving this particular problem, however I'm against solving it in an ad-hoc way. Here is an attempt at a general design we could follow in dune: Let's consider a package P and a file X installed by this package. We cross-compile on B for T.
|
|
Yes, that makes sense. But in this case, don't you think that this |
|
Not sure. Do you think it matters though? |
|
To get back at my original report and issue, if Would just work when executed with |
|
This makes sense to me because when cross-compiling, |
|
I agree with you, but I still feel odd about making an exception just for this executable. We should either implement a new better proposal completely or stick with the current one. |
|
Okay. Unless you can provide an alternative command line to cross-compile using I will go ahead and push a |
|
It's not broken, dune is just being consistent about the model it implements: the sedlex-windows package is the same as the sedlex package you'd get if you were building on Windows. If you cross-compile another project using sedlex via dune, it will work perfectly fine. However, I thought a bit more about this and given that you are the one maintaining the opam-cross repositories and these are the only use case for the dune cross compilation feature, we should just trust your judgement here. So making an exception seems fine. The change itself is simple: you can add the following line to the let sctx = SC.host sctx inIf you can confirm that it works for you, let's merge it. Opening an issue here is the best way to get eyes on the topic. However, we are quite busy with other things right now, in particular preparing for the next major release. So we can't focus on every problem. |
|
@toots sorry I haven't gotten back to you about this earlier. As Jeremie said, we are more focused on getting 2.0 out of the door right now. This does not mean we'll abandon dune 1.x, but that smaller issues like this would have to wait a bit. I agree with Jeremie on both counts:
@toots do you want the prepare the patch? @diml I think that eventually this problem will eventually disappear once we drop support for classical ppx. In the end, all we're losing is some edge case functionality for other users compiling with classical ppx. |
Indeed, that's a good point |
|
Thanks guys. I pushed the changes in #2286, gonna test them next. |
|
I can confirm that it fixes all of our compilation issues. Thanks for baring with me on this, let me know if I can help further. |
Signed-off-by: Romain Beauxis <[email protected]>
CHANGES: - Don't select all local implementations in `dune utop`. Instead, let the default implementation selection do its job. (ocaml/dune#2327, fixes ocaml/dune#2323, @TheLortex, review by @rgrinberg) - Check that selected implementations (either by variants or default implementations) are indeed implementations. (ocaml/dune#2328, @TheLortex, review by @rgrinberg) - Don't reserve the `Ppx` toplevel module name for ppx rewriters (ocaml/dune#2242, @diml) - Redesign of the library variant feature according to the ocaml/dune#2134 proposal. The set of variants is now computed when the virtual library is installed. Introducing a new `external_variant` stanza. (ocaml/dune#2169, fixes ocaml/dune#2134, @TheLortex, review by @diml) - Add proper line directives when copying `.cc` and `.cxx` sources (ocaml/dune#2275, @rgrinberg) - Fix error message for missing C++ sources. The `.cc` extension was always ignored before. (ocaml/dune#2275, @rgrinberg) - Add `$ dune init project` subcommand to create project boilerplate according to a common template. (ocaml/dune#2185, fixes ocaml/dune#159, @shonfeder) - Allow to run inline tests in javascript with nodejs (ocaml/dune#2266, @hhugo) - Build `ppx.exe` as compiling host binary. (ocaml/dune#2286, fixes ocaml/dune#2252, @toots, review by @rgrinberg and @diml) - Add a `cinaps` extension and stanza for better integration with the [cinaps tool](https://github.com/janestreet/cinaps) tool (ocaml/dune#2269, @diml) - Allow to embed build info in executables such as version and list and version of statically linked libraries (ocaml/dune#2224, @diml) - Set version in `META` and `dune-package` files to the one read from the vcs when no other version is available (ocaml/dune#2224, @diml) - Add a variable `%{target}` to be used in situations where the context requires at most one word, so `%{targets}` can be confusing; stdout redirections and "-o" arguments of various tools are the main use case; also, introduce a separate field `target` that must be used instead of `targets` in those situations. (ocaml/dune#2341, @aalekseyev) - Fix dependency graph of wrapped_compat modules. Previously, the dependency on the user written entry module was omitted. (ocaml/dune#2305, @rgrinberg) - Allow to promote executables built with an `executable` stanza (ocaml/dune#2379, @diml) - When instantiating an implementation with a variant, make sure it matches virtual library's list of known implementations. (ocaml/dune#2361, fixes ocaml/dune#2322, @TheLortex, review by @rgrinberg) - Add a variable `%{ignoring_promoted_rules}` that is `true` when `--ingore-promoted-rules` is passed on the command line and false otherwise (ocaml/dune#2382, @diml) - Fix a bug in `future_syntax` where the characters `@` and `&` were not distinguished in the names of binding operators (`let@` was the same as `let&`) (ocaml/dune#2376, @aalekseyev, @diml) - Workspaces with non unique project names are now supported. (ocaml/dune#2377, fix ocaml/dune#2325, @rgrinberg) - Improve opam generation to include the `dune` dependncies with the minimum constraint set based on the dune language version specified in the `dune-project` file. (2383, @avsm) - The order of fields in the generated opam file now follows order preferred in opam-lib. (@avsm, ocaml/dune#2380) - Fix coloring of error messages from the compiler (@diml, ocaml/dune#2384) - Add warning `66` to default set of warnings starting for dune projects with language verison >= `1.11` (@rgrinberg, @diml, fixes ocaml/dune#2299) - Add (dialect ...) stanza (@nojb, ocaml/dune#2404) - Add a `--context` argument to `dune install/uninstall` (@diml, ocaml/dune#2412) - Do not warn about merlin files pre 1.9. This warning can only be disabled in 1.9 (ocaml/dune#2421, fixes ocaml/dune#2399, @emillon)
Fixes: ocaml#2252 Signed-off-by: Romain Beauxis <[email protected]> Signed-off-by: Rudi Grinberg <[email protected]>
CHANGES: - Don't select all local implementations in `dune utop`. Instead, let the default implementation selection do its job. (ocaml/dune#2327, fixes ocaml/dune#2323, @TheLortex, review by @rgrinberg) - Check that selected implementations (either by variants or default implementations) are indeed implementations. (ocaml/dune#2328, @TheLortex, review by @rgrinberg) - Don't reserve the `Ppx` toplevel module name for ppx rewriters (ocaml/dune#2242, @diml) - Redesign of the library variant feature according to the ocaml/dune#2134 proposal. The set of variants is now computed when the virtual library is installed. Introducing a new `external_variant` stanza. (ocaml/dune#2169, fixes ocaml/dune#2134, @TheLortex, review by @diml) - Add proper line directives when copying `.cc` and `.cxx` sources (ocaml/dune#2275, @rgrinberg) - Fix error message for missing C++ sources. The `.cc` extension was always ignored before. (ocaml/dune#2275, @rgrinberg) - Add `$ dune init project` subcommand to create project boilerplate according to a common template. (ocaml/dune#2185, fixes ocaml/dune#159, @shonfeder) - Allow to run inline tests in javascript with nodejs (ocaml/dune#2266, @hhugo) - Build `ppx.exe` as compiling host binary. (ocaml/dune#2286, fixes ocaml/dune#2252, @toots, review by @rgrinberg and @diml) - Add a `cinaps` extension and stanza for better integration with the [cinaps tool](https://github.com/janestreet/cinaps) tool (ocaml/dune#2269, @diml) - Allow to embed build info in executables such as version and list and version of statically linked libraries (ocaml/dune#2224, @diml) - Set version in `META` and `dune-package` files to the one read from the vcs when no other version is available (ocaml/dune#2224, @diml) - Add a variable `%{target}` to be used in situations where the context requires at most one word, so `%{targets}` can be confusing; stdout redirections and "-o" arguments of various tools are the main use case; also, introduce a separate field `target` that must be used instead of `targets` in those situations. (ocaml/dune#2341, @aalekseyev) - Fix dependency graph of wrapped_compat modules. Previously, the dependency on the user written entry module was omitted. (ocaml/dune#2305, @rgrinberg) - Allow to promote executables built with an `executable` stanza (ocaml/dune#2379, @diml) - When instantiating an implementation with a variant, make sure it matches virtual library's list of known implementations. (ocaml/dune#2361, fixes ocaml/dune#2322, @TheLortex, review by @rgrinberg) - Add a variable `%{ignoring_promoted_rules}` that is `true` when `--ingore-promoted-rules` is passed on the command line and false otherwise (ocaml/dune#2382, @diml) - Fix a bug in `future_syntax` where the characters `@` and `&` were not distinguished in the names of binding operators (`let@` was the same as `let&`) (ocaml/dune#2376, @aalekseyev, @diml) - Workspaces with non unique project names are now supported. (ocaml/dune#2377, fix ocaml/dune#2325, @rgrinberg) - Improve opam generation to include the `dune` dependncies with the minimum constraint set based on the dune language version specified in the `dune-project` file. (2383, @avsm) - The order of fields in the generated opam file now follows order preferred in opam-lib. (@avsm, ocaml/dune#2380) - Fix coloring of error messages from the compiler (@diml, ocaml/dune#2384) - Add warning `66` to default set of warnings starting for dune projects with language verison >= `1.11` (@rgrinberg, @diml, fixes ocaml/dune#2299) - Add (dialect ...) stanza (@nojb, ocaml/dune#2404) - Add a `--context` argument to `dune install/uninstall` (@diml, ocaml/dune#2412) - Do not warn about merlin files pre 1.9. This warning can only be disabled in 1.9 (ocaml/dune#2421, fixes ocaml/dune#2399, @emillon) - Add a new `inline_tests` field in the env stanza to control inline_tests framework with a variable (ocaml/dune#2313, @mlasson, original idea by @diml, review by @rgrinberg). - New binary kind `js` for executables in order to explicitly enable Javascript targets, and a switch `(explicit_js_mode)` to require this mode in order to declare JS targets corresponding to executables. (ocaml/dune#1941, @nojb)
Hi,
I'm trying to cross-compile sedlex for windows, following the usual pattern outlined here.
I get a binary
ppx.exethat compiled forwindowsinstead of the compiling host. It is my understanding that this should not be the case. Is there anything that I am doing wrong?Here's the log of the build. Dune files are the ones from the sedlex repo linked above.
The text was updated successfully, but these errors were encountered: