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

env-vars, paths and dune files #2465

Open
bobot opened this issue Jul 29, 2019 · 14 comments
Open

env-vars, paths and dune files #2465

bobot opened this issue Jul 29, 2019 · 14 comments

Comments

@bobot
Copy link
Collaborator

bobot commented Jul 29, 2019

I would like to add paths (added in #2426) in dune files. But the design choices are scattered among different issues and merge request, so I would like to summarize them and see how paths in dune fills in.

Firstly env-vars and paths can be defined in dune-workspace file and env-vars in dune files. Indeed contrary to what the documentation says they could already be used in dune files.

Currently env-vars that are used for executing a command depends on the directory where the rule/exec is defined/ran. The env-vars used are the one of the directory and all its parent with usual inheritance rule (the parent has a smaller priority). We don't take the env-vars of all the scopes because we don't want to look at scopes we don't need to, in order to scale to big repositories. Moreover the priority rule to apply is not clear.

Still this design is different from the binaries and libraries one. Indeed all the public binaries and libraries built are available inside all the directories. So the environment variables that accompanied them should also.

Since paths can be appended (the priority rule is less problematic), we could choose a design more in line with binaries and libraries. We could use the paths from all the dune-files which are currently installed or more easily that are in the Artifacts.t value of Super_context.t.

At first we can forbid environment variable used and modified by dune (e.g PATH, OCAMLPATH) to be changed in dune file.

@ghost
Copy link

ghost commented Jul 30, 2019

Thanks for digging into this :)

We don't take the env-vars of all the scopes because we don't want to look at scopes we don't need to, in order to scale to big repositories. Moreover the priority rule to apply is not clear.

In my head, that was actually about semantic. If you allow such settings to cross project boundaries, then the interpretation of dune files depend on the workspace they are in. This might make it harder to reason about a project on its own.

Maybe that's ok for environment variables though, I'm not sure. I guess it all depends how these are used. Do you have use cases in mind for modifying paths in dune files?

At first we can forbid environment variable used and modified by dune (e.g PATH, OCAMLPATH) to be changed in dune file.

That seems like a net win over the current world. Modifying PATH via env-vars in a dune file is clearly dodgy right now.

@rgrinberg
Copy link
Member

One thing that I forgot to mention is the binaries field in env. To me this is seems like a much saner approach to introducing shorthand names for binaries rather than importing them wholesale via PATH. Have you looked into it at all?

I also forgot to mention this in @nojb's PR. Does this field help at all? You should be able to override ocamlc this way, and if not then we should simply fix that.

@nojb
Copy link
Collaborator

nojb commented Jul 30, 2019

One thing that I forgot to mention is the binaries field in env. To me this is seems like a much saner approach to introducing shorthand names for binaries rather than importing them wholesale via PATH. Have you looked into it at all?

I thought about this when working in #2426, but it was not enough for my use-case: on Windows for example the native compiler toolchain does not typically live in the PATH (because it is usual to work with several versions at once) and one needs to add them explicitly. Adding each tool individually using binaries is not very practical.

I also forgot to mention this in @nojb's PR. Does this field help at all? You should be able to override ocamlc this way, and if not then we should simply fix that.

As far as I remember, indeed it is the case that one cannot currently use binaries to override ocamlc. But it shouldn't be very hard to fix (and similarly for ocamlfind).

@nojb
Copy link
Collaborator

nojb commented Jul 30, 2019

Modifying PATH via env-vars in a dune file is clearly dodgy right now.

As far as I remember, it works in the sense that it is passed in the environment of launched programs, but it is not taken into account for resolving binaries. This could be fixed, but I didn't do that when working on #2426 because it would not be easy to append to PATH using env-vars. In fact maybe we should emit an error or a warning if the user tries to set PATH using env-vars.

@bobot
Copy link
Collaborator Author

bobot commented Jul 30, 2019

Do you have use cases in mind for modifying paths in dune files?

I don't want to modify PATH, indeed the binaries field is better. My use case is just for tools that, as ocamlfind, use variables (like OCAMLPATH) to find extra data. For example coq as COQPATH, and frama-c as FRAMA_EXTRA_SHARE. Later we could have a more integrated way to deal with these directories, like the relocation library #1185 but the semantic and mechanism will be similar. So I prefer to start simple.

If you allow such settings to cross project boundaries, then the interpretation of dune files depend on the workspace they are in.

Since this paths variable would be append only in dune file, they are used in the same way.

I haven't finished to code but the documentation would be, inside the env stanza:

- ``(paths (<var1> <val1>) .. (<varN> <valN>))`` allows to update the value of
  any ``PATH``-like variables in this context. If ``PATH`` itself is modified in
  this way, its value will be used to resolve binaries in the workspace,
  including finding the compiler and related tools. These variables will also be
  passed as part of the environment to any program launched by ``dune``. For
  each variable, the value is specified using the :ref:`ordered-set-language`.
  In ``dune-workspace`` files the given value replace the environment variables
  received by dune at start-up and `:default` is the value of the latter. In
  ``dune`` files the values are appended, contrary to `env-vars` the
  specification of all the `dune` files considered are used. The order in which the path are appended 
  in non-specified for different `dune` files. Relative paths are interpreted with
  respect to the directory of the file they are written in. At the moment, the
  environnement variable `PATH` and `OCAMLPATH` are only supported in
  ``dune-workspace`` files.

I don't like that the semantic is a little different in dune-workspace and dune files but they make sense in the way both are used.

Another question I haven't answered is should we have a way to specify that the path should not be absolutized in the source directory, but in the build directory; or should it be the default.

EDIT: the advantage for build directory is it forces to give good dependencies.

@rgrinberg
Copy link
Member

I thought about this when working in #2426, but it was not enough for my use-case: on Windows for example the native compiler toolchain does not typically live in the PATH (because it is usual to work with several versions at once) and one needs to add them explicitly.

binaries precisely does that however. It adds an individual binary to PATH regardless where it lives.

Adding each tool individually using binaries is not very practical.

Good point but I think that for OCaml we could have added special support via some sort of ocaml field in env for example. As it stands right now, things are a bit dodgy that you this will not override the ocamlc that dune uses but will override other rules.

@bobot
Copy link
Collaborator Author

bobot commented Jul 30, 2019

As far as I remember, it works in the sense that it is passed in the environment of launched programs, but it is not taken into account for resolving binaries.

In that case, I don't understand correctly the documentation:

If PATH itself is modified in this way, its value will be used to resolve binaries in the workspace, including finding the compiler and related tools.

@rgrinberg
Copy link
Member

No because the compiler and related tools are resolved when creating the Context.t. This is done before any env stanzas are evaluated and hence not taken into account.

@nojb
Copy link
Collaborator

nojb commented Jul 30, 2019

As far as I remember, it works in the sense that it is passed in the environment of launched programs, but it is not taken into account for resolving binaries.

In that case, I don't understand correctly the documentation:

If PATH itself is modified in this way, its value will be used to resolve binaries in the workspace, including finding the compiler and related tools.

This doc refers only to (paths ...), my comment above refers to (env-vars ...).

@bobot
Copy link
Collaborator Author

bobot commented Jul 30, 2019

Ok, I see but we can remove this difference. I propose that both env-vars and paths in dune-workspace are used by dune and they are not used by dune itself in dune files.

@ghost
Copy link

ghost commented Jul 31, 2019

In dune files the values are appended, contrary to env-vars the specification of all the dune files considered are used

Does that correspond to the dune files between between the root of the current and the current directory?

@bobot
Copy link
Collaborator Author

bobot commented Jul 31, 2019

No it corresponds to all the dune file of the workspace, i.e. all the stanza. I pushed in #2483 for clarity.

@ghost
Copy link

ghost commented Aug 1, 2019

In the Coq case, what would be the full story? I don't know the detail, but my guess is that COQPATH is library installation site, is that correct? (/cc @ejgallego)

@ejgallego
Copy link
Collaborator

In the Coq case, what would be the full story? I don't know the detail, but my guess is that COQPATH is library installation site, is that correct? (/cc @ejgallego)

Yup, it is roughly equivalent to ocamlpath.

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