ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.63k stars 407 forks source link

Watermaking in `dune build` #1539

Open Niols opened 6 years ago

Niols commented 6 years ago

Hello,

I wanted to use watermarking (in the doc) in my project but I was expecting it to happen in dune build @install. Apparently not.

-- Niols

andreypopp commented 6 years ago

My understanding is that during build process vcs info might not be available (building from the release tarball).

ghost commented 6 years ago

I guess we never considered it. In the release tarball, the VCS info are indeed not available, however the version number could be set in the opam file or somewhere else (I believe dune-release does that).

The idea would basically be to rewrite files on the fly as we copy them to _build. There are however a few issue with this approach: if the files inside _build are not the same as the file in the source tree, then error locations are going to be wrong.

Niols commented 6 years ago

The idea would basically be to rewrite files on the fly as we copy them to _build. There are however a few issue with this approach: if the files inside _build are not the same as the file in the source tree, then error locations are going to be wrong.

I would like such a feature. But it's not necessarily a Dune feature. And you're right about locations in the source tree. Which makes me think of a PPX that would read OCaml strings in the document and replace the placeholders by the same values. But there are a few scary things:

Or, considering that this a mainly for small replacements (like the name or the version of the package), one can say that the location issue is not really a severe issue and decide to still apply dune subst. But is there a way to tell Dune to apply dune subst in the whole _build directory after the copy but before further processing?

ghost commented 5 years ago

If it's only for the ml code, what about simply generating a plain .ml file with the value expanded? i.e. generate something that looks like this:

let version = "..."
let vcs_id = "..."
...
Niols commented 5 years ago

That's my usage yes. And that's a good idea, I could just add a Dune rule to generate a file like this (and maybe a small OPAM plugin?)…

ghost commented 5 years ago

I guess we could add a new action that would perform the same substitutions as dune subst on a single file. Then you'd write:

(rule (with-stdout-to info.ml (dune-subst file.ml.in)))

where file.ml.in would contain:

let version = "%%VERSION%%"
let vcs_commit_id = "%%VCS_COMMIT_ID%%"
...
Khady commented 5 years ago

How would it work with nested projects/workspace that can be in different git/hg/... repos? I have the feeling that such a feature could replace a ugly hack I had to write to include version information in our projects

ghost commented 5 years ago

The information would be obtained from the project the call to dune-subst is part of. When a project is vendored and its .git/.hg is not kept, then there is the choice between:

NathanReb commented 5 years ago

I just ran into that exact situation and the dune-subst action would indeed be very nice for taking care of that.

I'd be happy to contribute a PR if you still think that'd be an improvement to dune!

rgrinberg commented 5 years ago

One thing to keep in mind is that using this feature will hurt incremental builds. I wonder if it makes no make the dune-subst action a no-op in the dev profile.

Niols commented 5 years ago

If it were to hurt incremental builds, it could make sense to make it a no-op in the dev profile. But why would it? As long as the .opam (or any other file containing metadata) doesn't change, this shouldn't have any impact on the incremental build, isn't that right?

NathanReb commented 5 years ago

Well if you commit anything even to a README you need to rerun dune subst since the substitute for %%VERSION%% will be different for instance because it relies on git describe. I don't know if there's a smart way to handle that or if it's even worth bothering but it seems like it can indeed be pretty hurtful to incremental builds.

Niols commented 5 years ago

Hum, it's true that every single commit would invalidate all the files containing %%VERSION%%, this is indeed pretty annoying.

ghost commented 5 years ago

Yh, there is the same issue with #880. I'm not sure the same behaviour will suit all users, so we could have an option to simply mask the exact VCS info wherever we use them. We can document all this in the dune-subst action documentation, so that users know what to expect.

ghost commented 5 years ago

Anyway, regarding the implementation, it will become easier once we have properly integrated the memoization system everywhere as this requires memoizing a few things such as the output of git describe .... We'll describe how to proceed once this is done, which should be in a couple of weeks.

kit-ty-kate commented 2 years ago

I like @jeremiedimino’s proposal to replace dune subst with a (dune-subst …) stanza very much. Are there some remaining roadblocks to implement it?