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

Watermaking in dune build #1539

Open
Niols opened this issue Nov 14, 2018 · 16 comments
Open

Watermaking in dune build #1539

Niols opened this issue Nov 14, 2018 · 16 comments

Comments

@Niols
Copy link
Contributor

Niols commented Nov 14, 2018

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.

  • is there a reason why the replacement takes place only in dune-release and dune subst and not in dune build @install?
  • is there a way for me to apply dune subst on all my files during the build process?

-- Niols

@andreypopp
Copy link
Member

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

@ghost
Copy link

ghost commented Nov 14, 2018

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
Copy link
Contributor Author

Niols commented Nov 14, 2018

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:

  • If the strings to put in place of the placeholders contain %, the behaviour would be different depending on where the replacement takes place (but I guess that the same problem exists in dune-release?)
  • Is it possible (and easy) in Dune to run a PPX like this one that will need enough information (like access to the dune-project and *.opam files)?

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
Copy link

ghost commented Nov 15, 2018

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
Copy link
Contributor Author

Niols commented Nov 15, 2018

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
Copy link

ghost commented Nov 15, 2018

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
Copy link
Contributor

Khady commented Nov 16, 2018

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
Copy link

ghost commented Nov 19, 2018

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:

  • using the vcs info from the vendoring project
  • considering that the vcs info for the vendored project are missing
  • when vendoring the project, take a snapshot of the vcs info and store them in a file at the root of the vendored project

@NathanReb
Copy link
Collaborator

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
Copy link
Member

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
Copy link
Contributor Author

Niols commented Jan 11, 2019

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
Copy link
Collaborator

NathanReb commented Jan 11, 2019

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
Copy link
Contributor Author

Niols commented Jan 11, 2019

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

@ghost
Copy link

ghost commented Jan 14, 2019

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
Copy link

ghost commented Jan 15, 2019

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants