Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release profile documentation is unclear about exact effects #3359

Open
yawaramin opened this issue Apr 11, 2020 · 11 comments
Open

Release profile documentation is unclear about exact effects #3359

yawaramin opened this issue Apr 11, 2020 · 11 comments
Labels
docs Documentation improvements

Comments

@yawaramin
Copy link
Contributor

As per https://dune.readthedocs.io/en/stable/faq.html#how-to-make-warnings-non-fatal , using the release profile has certain specific effects:

you can pass --profile release to dune. It will set up different compilation options that usually make sense for release builds, including making warnings non-fatal. This is done by default when installing packages from opam.

However, it's not clear exactly what the compilation options are. For example, do they include -O2? Or -O3? Etc. I think these settings are important to know.

@ghost
Copy link

ghost commented Apr 15, 2020

Indeed. The doc should be clear about this. Additionally, this recent discussion makes me think that maybe we should indeed set -O2 by default in the release profile: https://discuss.ocaml.org/t/functorial-vs-comparator-container-interfaces/5384/9

@jberdine
Copy link
Contributor

Sorry if this is a bit of a tangent, but is it clear that this sort of thing should be handled at the dune level, or would it make sense to have a broader discussion of whether e.g. we should be setting OCAMLPARAM=_,O3=1 when asking opam or esy or whatever to build and install packages?

@yawaramin
Copy link
Contributor Author

@jberdine a typical workflow nowadays is to use dune to build and then publish packages to opam. So when you opam install a package, you're usually installing the build artifact i.e. the binaries. At least this is my understanding 🙂

@jberdine
Copy link
Contributor

@yawaramin Yes, but opam has to compile those binaries, and using dune to build your package in no way implies accepting dune's default compiler flags. In all of my code I override the dune defaults entirely. I'm don't know if it is common to just blindly use the defaults, especially when they are known to differ from the upstream compiler. On the other hand, telling dune to build with -O3 necessitates setting things up to ensure that an flambda compiler is used, which leads to checking in dune-workspace files or similar, and is definitely not the approved path. So there is a bit of a catch-22 situation as far as I can see.

@yawaramin
Copy link
Contributor Author

yawaramin commented Apr 16, 2020

@jberdine

opam has to compile those binaries,

Maybe I'm misunderstanding but as far as I can tell, opam doesn't compile binaries. It runs whatever you define is the build command, which builds the package. In this issue we're talking about a scenario where the build command is (something similar to) dune build --release ....

telling dune to build with -O3 necessitates setting things up to ensure that an flambda compiler is used,

Perhaps, but diml was talking about setting a default of -O2.

@ghost
Copy link

ghost commented Apr 20, 2020

Well, the scope of this discussion is definitely bigger that Dune and Dune shouldn't decide what to do on its own. I was thinking to start a thread on discuss.

Regarding following the compiler defaults, given that the compiler doesn't have a notion of "release" and "development" mode, it seems expected to me that Dune doesn't follow the compiler defaults. For instance, maybe -O3 is good for releases because it produces fast binaries but bad for incremental development as it makes compilation slower.

It seems that to me that any tool that people use to produce release binaries should be able to set such defaults. So opam and esy should probably be able to change the default globally. However, they shouldn't change it in the development environment. i.e. packages built by opam install ... should be optimised but maybe not the ones you work on. And if people use dune directly to produce release binaries, then Dune should know about such defaults as well. There is indeed a problem if the different tools have different ideas of what the good defaults are. Ideally, we all agree on the right set of defaults and all use the same.

@yawaramin
Copy link
Contributor Author

Opam has the build config to invoke the build, and esy has build and buildDev configs for exactly the production / development build scenarios you mentioned. (Maybe opam does too, I'm not familiar.)

Agree that opam should install optimized packages. I don't think opam has a notion of a 'default' build command though, from what I understand the build command for the package is whatever the 'build' config is in the opam file. Maybe if opam were to do project scaffolding it might set up a default build command but I think scaffolding is being handled by Dune, right?

Esy's 'default' build config is dune build -p #{self.name}, btw. I think both esy and opam would recommend setting this or a similar command.

@ghost
Copy link

ghost commented Apr 21, 2020

I guess what I had in mind is that opam would tell Dune, or for what is worse any build system to compile OCaml code with -O3. Such a thing could be communicated by opam via OCAMLPARAM as @jberdine suggests for instance. Although, I'm not too sure if opam supports that at the moment. More precisely, there is a mechanism to setup OCAMLPARAM globally, but that would also affect your shell when you do opam config env, which doesn't seem desirable.

@yawaramin
Copy link
Contributor Author

yawaramin commented Apr 21, 2020

To be honest I don't think opam will want to override the configured build command in package opam files and tell Dune more exact flags through a side channel 🙂 but it's definitely worth a thread in discuss.

Edit: in any case I think no matter what opam and Dune decide to do in future, Dune will continue to have the -p/--profile release flags, so it's worth documenting them explicitly.

@jberdine
Copy link
Contributor

I think that having opam communicate default flags to dune is a good idea, but using an environment variable is too coarse. That is, it seems that each opam switch ought to have its own default flags. When dune is building several contexts in parallel which are associated with different opam switches, the defaults for each switch should be communicated to dune for use in the corresponding contexts.

@ghost
Copy link

ghost commented Apr 23, 2020

We might need a file in the <prefix>/etc directory

@rgrinberg rgrinberg added the docs Documentation improvements label Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation improvements
Projects
None yet
Development

No branches or pull requests

3 participants