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

Can't find the syntax to use %{ext_obj} in rule's targets? #1189

Closed
ttamttam opened this issue Aug 29, 2018 · 8 comments · Fixed by ocaml/opam-repository#12693
Closed

Can't find the syntax to use %{ext_obj} in rule's targets? #1189

ttamttam opened this issue Aug 29, 2018 · 8 comments · Fixed by ocaml/opam-repository#12693

Comments

@ttamttam
Copy link
Contributor

If I do something like

         (targets something%{ext_obj})
     ....)

I get:

dune build something.o
File "dune", line 2, characters 15-29:
Error: Atom or quoted string expected

while the following is working:

         (targets something.o)
     ....)

I could not find th right syntax for this?

@rgrinberg
Copy link
Member

We just didn't add support for this yet. If you'd like to work on this, we can give you some pointers on how to add this feature.

@emillon
Copy link
Collaborator

emillon commented Aug 29, 2018

As a side note, does the target appear in the action? If that's the case it might be possible to use a special form to declare the target.

@ttamttam
Copy link
Contributor Author

@emillon I'm not sure to understand?

@rgrinberg I think I could give a try on it if you can give me some pointers.

@rgrinberg
Copy link
Member

Preliminaries

Dune holds all stanza definitions in https://github.com/ocaml/dune/blob/master/src/dune_file.mli#L1

This is where we hold the definition for user defined rules and the targets field (see the Targets sub module).

Anything that accepts %{var} substitutions is called a template and is represented as String_with_vars.t. Notice how the dependency field (Dep_conf.t) accepts such templates as arguments.

Action Plan

The target's type constructors needs to be modified to accept templates as arguments rather than strings. Once this is done, we should change the rule generation to expand the templates before generating the targets. Expansion is done through the family of functions defined in the super context: https://github.com/ocaml/dune/blob/master/src/super_context.mli#L83

One thing to note is that we can only expand "static" variables here. Dune must know the entire list of targets up front, that is, before the rules have been generated. Luckily for us, %{ext_obj} is one such static variable that comes from the context (Context.t).

If you have any questions, feel free to ask.

@dra27
Copy link
Member

dra27 commented Sep 10, 2018

Note that this is related to #690 - (install stanzas need the same support.

@ttamttam
Copy link
Contributor Author

I had a look. More for learning than because I really need the functionality. I'll come back in some days (weeks) with some questions…

@rgrinberg
Copy link
Member

nojb fixed this

@ttamttam
Copy link
Contributor Author

Thanks to @nojb!

rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Sep 23, 2018
CHANGES:

- Support colors on Windows (ocaml/dune#1290, @diml)

- Allow `dune.configurator` and `base` to be used together (ocaml/dune#1291, fix
  ocaml/dune#1167, @diml)

- Support interrupting and restarting builds on file changes (ocaml/dune#1246,
  @kodek16)

- Fix findlib-dynload support with byte mode only (ocaml/dune#1295, @bobot)

- Make `dune rules -m` output a valid makefile (ocaml/dune#1293, @diml)

- Expand variables in `(targets ..)` field (ocaml/dune#1301, ocaml/dune#1320, fix ocaml/dune#1189, @nojb,
  @rgrinberg, @diml)

- Fix a race condition on Windows that was introduced in 1.2.0
  (ocaml/dune#1304, fix ocaml/dune#1303, @diml)

- Fix the generation of .merlin files to account for private modules
  (@rgrinberg, fix ocaml/dune#1314)

- Exclude the local opam switch directory (`_opam`) from the list of watched
  directories (ocaml/dune#1315, @dysinger)

- Fix compilation of the module generated for `findlib.dynload`
  (ocaml/dune#1317, fix ocaml/dune#1310, @diml)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Sep 23, 2018
CHANGES:

- Support colors on Windows (ocaml/dune#1290, @diml)

- Allow `dune.configurator` and `base` to be used together (ocaml/dune#1291, fix
  ocaml/dune#1167, @diml)

- Support interrupting and restarting builds on file changes (ocaml/dune#1246,
  @kodek16)

- Fix findlib-dynload support with byte mode only (ocaml/dune#1295, @bobot)

- Make `dune rules -m` output a valid makefile (ocaml/dune#1293, @diml)

- Expand variables in `(targets ..)` field (ocaml/dune#1301, ocaml/dune#1320, fix ocaml/dune#1189, @nojb,
  @rgrinberg, @diml)

- Fix a race condition on Windows that was introduced in 1.2.0
  (ocaml/dune#1304, fix ocaml/dune#1303, @diml)

- Fix the generation of .merlin files to account for private modules
  (@rgrinberg, fix ocaml/dune#1314)

- Exclude the local opam switch directory (`_opam`) from the list of watched
  directories (ocaml/dune#1315, @dysinger)

- Fix compilation of the module generated for `findlib.dynload`
  (ocaml/dune#1317, fix ocaml/dune#1310, @diml)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Sep 24, 2018
CHANGES:

- Support colors on Windows (ocaml/dune#1290, @diml)

- Allow `dune.configurator` and `base` to be used together (ocaml/dune#1291, fix
  ocaml/dune#1167, @diml)

- Support interrupting and restarting builds on file changes (ocaml/dune#1246,
  @kodek16)

- Fix findlib-dynload support with byte mode only (ocaml/dune#1295, @bobot)

- Make `dune rules -m` output a valid makefile (ocaml/dune#1293, @diml)

- Expand variables in `(targets ..)` field (ocaml/dune#1301, ocaml/dune#1320, fix ocaml/dune#1189, @nojb,
  @rgrinberg, @diml)

- Fix a race condition on Windows that was introduced in 1.2.0
  (ocaml/dune#1304, fix ocaml/dune#1303, @diml)

- Fix the generation of .merlin files to account for private modules
  (@rgrinberg, fix ocaml/dune#1314)

- Exclude the local opam switch directory (`_opam`) from the list of watched
  directories (ocaml/dune#1315, @dysinger)

- Fix compilation of the module generated for `findlib.dynload`
  (ocaml/dune#1317, fix ocaml/dune#1310, @diml)

- Lift restriction on `copy_files` and `copy_files#` stanzas that files to be
  copied should be in a subdirectory of the current directory.
  (ocaml/dune#1323, fix ocaml/dune#911, @nojb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants