Syntax of variables in actions

[ghost]

We recently had a discussion at Jane Street about the syntax of variables in the jbuild files read by Jenga. I think the problems raised apply to Dune as well, so I'm opening this ticket to discuss whether we think the same change is worth doing in Dune and if yes, how we should do it.

Current situation

Currently Dune expands a few variables in user actions. The syntax of variables is ${var} or $(var). For instance:

• ${CC} or $(CC) is replaced by the C compiler in use
• ${@} or $(@) is replaced by the targets of the rule
• ${bin:prog} is replaced by prog searched in the PATH
• ...

When a variable is unknown to Dune, it is left as it. The syntax and the behavior directly come from the Jane Street jenga rules.

Someone made the following remarks:

• the syntax is not great when used inside a (bash "...") or (system "...") action, since the shell interprets these forms as well
• Using @, ^ or < to denote targets and dependencies is fine for someone familiar to make, but it's not particularly clear otherwise

I think these remarks make sense.

Proposal

The proposal is to change the syntax to the following one: %{var}. Since this syntax is not used by shells, Dune can just fail when encountering an unknown variable.

Additionally, we could do the following replacements to make things more explicit:

• replace @ by targets
• replace < by first-dep
• replace ^ by deps

Opinions?

[rgrinberg]

What about supporting the new syntax in dune files and the old syntax in jbuild files?

I agree with the change in general. Though I think we should keep support for the make aliases, even if we add user friendly aliases.

[ghost]

What about supporting the new syntax in dune files and the old syntax in jbuild files?

That's a possibility indeed, and that should be easy to remember for users.

I was initially thinking that the dune-project file would control the syntax and features of both dune and jbuild file inside the project, but we can indeed assume that jbuild files always use the old syntax. That's a simpler model indeed.

I agree with the change in general. Though I think we should keep support for the make aliases, even if we add user friendly aliases.

That seems reasonable to me

[ghost]

/cc @Chris00 as well since you have been looking at syntax quite a bit

[Chris00]

I am fine with %{var}.

Though I think we should keep support for the make aliases, even if we add user friendly aliases.

In my opinion, it depends which point of view we adopt. If it is to make the files easier to read for someone coming “from the outside”, then targets and the like should be adopted and @,... be rejected. Someone writing the file will (hopefully) read the documentation and will have to learn the conventions (whether they are deps, all-deps, ^,...). In that context, the make names could be kept and the more mnemonic new names be added. I'm not sure however that having both names is a plus, I would rather tend to consider it a downside as it adds to the cognitive burden of the reader. If we want to change the variable names, let's use the jbuild → dune transition to do a clean break. Also, maybe, instead of first-dep, we should introduce a more general nth element construct such as %{deps[1]}?

[ghost]

I tend to agree that having only one name is better. And %{deps} and %{targets} are intuitive.

Also, maybe, instead of first-dep, we should introduce a more general nth element construct such as %{deps[1]}?

That seems good to me. 0-based or 1-based? :)

[rgrinberg]

Jérémie Dimino <notifications@github.com> writes:

That seems good to me. 0-based or 1-based? :)

0 based of course :)

[ghost]

BTW, with this change we will need a way to quote % characters inside quoted strings. I suggest to simply use \% since this is the syntax we use to quote everything.

Technically this means that we will have to do the parsing of variables directly in the s-expression parser. This will require a bit of refactoring, however I think it is worth doing since it can't easily be done afterwards without breaking things.

[rgrinberg]

I'm a bit worried about that because usexp isn't for internal use only anymore. We expect users (such as those of configurator) to use it as well. I think they will be unpleasantly surprised by this complication. Another escaping scheme is %% which might be a bit less invasive. I'd still prefer something uniform if possible however.

[ghost]

BTW, someone mentioned that instead of indexes, we could also use names. For instance :x file1 :y file2 and then use %{x} and %{y} in the action. In general using names is nicer than indexes.

[rgrinberg]

Yeah, that sounds like it will be fine as well. Would one be able to bind multiple dependencies to the same name? It's common enough for the set of dependencies to be made of 2 groups that you just want to pass to a different flag in the command.

[ghost]

As long as we can have a nice syntax for it that seems fine.

[ghost]

Closing this issue since the big part of the work has been done. The renaming of variabels is tracked by #842. Possibly one thing we should do before the 1.0.0 release is decide on a syntax for naming dependencies, so that we can at least reserve the syntax to avoid a breaking change.

[rgrinberg]

Possibly one thing we should do before the 1.0.0 release is decide on a syntax for naming dependencies, so that we can at least reserve the syntax to avoid a breaking change.

Why should we remove the indexed access then? If we don't implement named deps, then how would users access the first dep then.

[rgrinberg]

As long as we can have a nice syntax for it that seems fine.

I think allowing list valued bindings should be enough :x (foo bar) :y baz

[ghost]

Why should we remove the indexed access then? If we don't implement named deps, then how would users access the first dep then.

We could add a %{first-dep} variable. But it doesn't seem to bad to have both indexed and named access. It is clear what %{dep[0]} means

I think allowing list valued bindings should be enough :x (foo bar) :y baz

There is an ambiguity however: does :x (glob_files y) means the two dependencies glob_files and y or the single (glob_files y) dependency specification?

[rgrinberg]

We could add a %{first-dep} variable. But it doesn't seem to bad to have both indexed and named access. It is clear what %{dep[0]} means

Okay. It's just that in your comment you said instead of indexed lookups. I agree that having both is alright

There is an ambiguity however: does :x (glob_files y) means the two dependencies glob_files and y or the single (glob_files y) dependency specification?

Good point. I suppose we can just have a list constructor. In this case we won't even need a concept of list valued bindings. As a list would be a valid Dep_conf.t

[ghost]

Good point. I suppose we can just have a list constructor. In this case we won't even need a concept of list valued bindings. As a list would be a valid Dep_conf.t

Good idea