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

Documentation suggestion: scope of `env` stanzas #3279

Open
jberdine opened this issue Mar 18, 2020 · 4 comments
Open

Documentation suggestion: scope of `env` stanzas #3279

jberdine opened this issue Mar 18, 2020 · 4 comments

Comments

@jberdine
Copy link

@jberdine jberdine commented Mar 18, 2020

This is just a report of behavior that was, to me, a surprise.

My, perhaps flawed, mental model is that the dune-workspace file has broadest effect, then the dune-project file(s), and then the individual dune files. I was surprised to discover that it was possible to define contexts in dune-workspace using profile names that were only defined in the dune file. Based on my understanding, this violates definition-before-use, so I was surprised. Maybe it is just me who is confused.

For context, the reason to move these definitions out of dune-workspace is that when copying the project directory (containing dune-workspace) into another project and building as a vendored dependency, the dune-workspace file seems to be ignored, while the dune file is not.

@diml

This comment has been minimized.

Copy link
Member

@diml diml commented Mar 19, 2020

That seems like the right mental model to me. What do you mean by defining contexts in dune-workspace using profile names that were only defined in the dune file?

The idea is that all profiles exist, and we can match on the currently active profile in dune files.

@jberdine

This comment has been minimized.

Copy link
Author

@jberdine jberdine commented Mar 19, 2020

For example, dune-workspace has:

(context (opam (switch sledge) (name dbg) (profile dbg) (merlin)))
(context (opam (switch sledge) (name dbg-opt) (profile dbg-opt)))
(context (opam (switch sledge) (name opt) (profile opt)))

to indicate which profile each context should be build with. Then dune has:

(env
 (dbg
  (flags
   (-w +a-4-9-18-40-42-44-48@50-66 -strict-formats -strict-sequence
     -short-paths -bin-annot -keep-locs -keep-docs -opaque))
  (env-vars
   (PPX_TRACE_ENABLED 1))
  (inline_tests enabled))
 (dbg-opt
  (flags
   (-w -a -noassert -unboxed-types))
  (ocamlopt_flags (-O3))
  (env-vars
   (PPX_TRACE_ENABLED 1))
  (inline_tests disabled))
 (_
  (flags
   (-w -a -noassert -unboxed-types))
  (ocamlopt_flags (-O3))
  (env-vars
   (PPX_TRACE_ENABLED 0))
  (inline_tests disabled)))

to define what each profile means.

If I put that env stanza in dune-workspace, the normal build works fine. But when building as a vendored subdir, the context in effect will/could be something completely different like default, so the catchall case of the env stanza is needed, and so the build does not work if the env stanza is moved from dune to dune-workspace.

@diml

This comment has been minimized.

Copy link
Member

@diml diml commented Mar 24, 2020

Hmm, I see. I'm not sure what can be done here. In general, the idea is that the scope of dune and dune-project files is wider than the one of dune-workspace files. More precisely, in the dune-workspace file you might write things that only work in your setup such as (switch sledge).

That's why dune-workspace files are not composable and we generally don't recommend committing them in repositories.

@diml

This comment has been minimized.

Copy link
Member

@diml diml commented Mar 30, 2020

I thought a bit more about this, and maybe the issue boils down to the fact that we have both "build contexts" and "profiles". i.e., if they were the same, we could effectively define the various profiles in the dune-project and they would be immediately available. In the dune-workspace, we would then only list the profiles we want to build by default, and which opam switch we want to use as a backend.

We could conflate the two notions in version 3 of the language.

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.