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.

Sign up for GitHub

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

Jump to bottom

[question] enabled_if in executable stanza? #1690

Closed
ejgallego opened this issue Dec 19, 2018 · 13 comments · Fixed by #3137 or ocaml/opam-repository#15845
Closed

[question] enabled_if in executable stanza? #1690

ejgallego opened this issue Dec 19, 2018 · 13 comments · Fixed by #3137 or ocaml/opam-repository#15845
Assignees
@voodoos

Comments

@ejgallego
Copy link
Collaborator

Would it make sense to add the optional / enabled_if field to the executable stanza?

The context is that in the default build of Coq, we may want to skip building some optional binaries [such as CoqIDE] if the right libraries are not present. Note that we are working on a composed build thus this seems like the simplest approach.
@ejgallego ejgallego mentioned this issue Dec 19, 2018
Merged
@ghost
Copy link

ghost commented Dec 19, 2018

This is related to #1445. That seems fine to me.

@ejgallego
Copy link
Collaborator Author

Thanks @diml , I may try to give it a go then.

@rgrinberg
Copy link
Member

rgrinberg commented Dec 19, 2018 •
edited
Loading

I have no objection to enabled_if on executables, but why not split coqide into a separate OCaml package?

@ejgallego
Copy link
Collaborator Author

It is already on its own package, however in a composed build some targets such as @check would still error, right?

@ejgallego
Copy link
Collaborator Author

Note that indeed I am not strongly defending this; maybe a better system for package subsets [so we could write "build all .install but coqide.install"] could work better.

The thing indeed is that the default target for Coq is usually the composed build [as we are fixing plugins]

@rgrinberg
Copy link
Member

Yup, that's indeed a problem but enabled_if really seems like the wrong solution. When one installs coqide directly, evaluating some predicates should certainly not be required.

Currently my proposal for this feature is something like: #1642

So you'd be able to do something like: $ dune build check:available.

The question is, how to create such a target? We basically would like to exclude any targets from an alias that depend on a missing optional library somehow.

@ejgallego
Copy link
Collaborator Author

Indeed if we just kept (optional) for libs an filter out not buildable targets that should be a nice solution.

@bobot
Copy link
Collaborator

bobot commented Jul 11, 2019

@ejgallego Do you have a solution to your problem, that is currently implemented?

@ejgallego
Copy link
Collaborator Author

@ejgallego Do you have a solution to your problem, that is currently implemented?

@bobot for now we are using different packages and do dune build pkg.install to workaround that in the CI setups that are lacking the dependencies.

I need to think a bit more about this but first I'd like to get the whole Coq universe building under Dune, and see what would be the best option to allow missing dependencies and still have an usable build.

@rgrinberg
Copy link
Member

We can add enabled_if to executables because it would improve the experience for @all.

@ejgallego ejgallego mentioned this issue Jul 23, 2019
Closed
@ejgallego
Copy link
Collaborator Author

(optional) is now admitted in (executable ...) stanzas, that's enough for our use case, but I guess that enabled_if could still be useful for cross-OS use cases.

@ghost
Copy link

ghost commented Feb 12, 2020

Adding enabled_if to executables seems fine to me

@rgrinberg
Copy link
Member

@voodoos this is a good thing to contribute if you have time

@voodoos voodoos self-assigned this Feb 13, 2020
@voodoos voodoos mentioned this issue Feb 14, 2020
Merged
@rgrinberg rgrinberg mentioned this issue Feb 15, 2020
Merged
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Feb 15, 2020
@rgrinberg
[new release] dune-build-info, dune, dune-configurator, dune-action-p…
946b24e 
…lugin, dune-private-libs and dune-glob (2.3.0)

CHANGES:

- Improve validation and error handling of arguments to `dune init` (ocaml/dune#3103, fixes
  ocaml/dune#3046, @shonfeder)

- `dune init exec NAME` now uses the `NAME` argument for private modules (ocaml/dune#3103,
  fixes ocaml/dune#3088, @shonfeder)

- Avoid linear walk to detect children, this should greatly improve
  performance when a target has a large number of dependencies (ocaml/dune#2959,
  @ejgallego, @aalekseyev, @Armael)

- [coq] Add `(boot)` option to `(coq.theories)` to enable bootstrap of
  Coq's stdlib (ocaml/dune#3096, @ejgallego)

- [coq] Deprecate `public_name` field in favour of `package` (ocaml/dune#2087, @ejgallego)

- Better error reporting for "data only" and "vendored" dirs. Using these with
  anything else than a strict subdirectory or `*` will raise an error. The
  previous behavior was to just do nothing  (ocaml/dune#3056, fixes ocaml/dune#3019, @voodoos)

- Fix bootstrap on bytecode only switches on windows or where `-j1` is set.
  (ocaml/dune#3112, @xclerc, @rgrinberg)

- Allow `enabled_if` fields in `executable(s)` stanzas (ocaml/dune#3137, fixes ocaml/dune#1690
  @voodoos)

- Do not fail if `ocamldep`, `ocamlmklib`, or `ocaml` are absent. Wait for them
  to be used to fail (ocaml/dune#3138, @rgrinberg)

- Introduce a `strict_package_deps` mode that verifies that dependencies between
  packages in the workspace are specified correctly. (@rgrinberg, ocaml/dune#3117)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Feb 15, 2020
@rgrinberg
[new release] dune-build-info, dune, dune-configurator, dune-action-p…
849b1a4 
…lugin, dune-private-libs and dune-glob (2.3.0)

CHANGES:

- Improve validation and error handling of arguments to `dune init` (ocaml/dune#3103, fixes
  ocaml/dune#3046, @shonfeder)

- `dune init exec NAME` now uses the `NAME` argument for private modules (ocaml/dune#3103,
  fixes ocaml/dune#3088, @shonfeder)

- Avoid linear walk to detect children, this should greatly improve
  performance when a target has a large number of dependencies (ocaml/dune#2959,
  @ejgallego, @aalekseyev, @Armael)

- [coq] Add `(boot)` option to `(coq.theories)` to enable bootstrap of
  Coq's stdlib (ocaml/dune#3096, @ejgallego)

- [coq] Deprecate `public_name` field in favour of `package` (ocaml/dune#2087, @ejgallego)

- Better error reporting for "data only" and "vendored" dirs. Using these with
  anything else than a strict subdirectory or `*` will raise an error. The
  previous behavior was to just do nothing  (ocaml/dune#3056, fixes ocaml/dune#3019, @voodoos)

- Fix bootstrap on bytecode only switches on windows or where `-j1` is set.
  (ocaml/dune#3112, @xclerc, @rgrinberg)

- Allow `enabled_if` fields in `executable(s)` stanzas (ocaml/dune#3137, fixes ocaml/dune#1690
  @voodoos)

- Do not fail if `ocamldep`, `ocamlmklib`, or `ocaml` are absent. Wait for them
  to be used to fail (ocaml/dune#3138, @rgrinberg)

- Introduce a `strict_package_deps` mode that verifies that dependencies between
  packages in the workspace are specified correctly. (@rgrinberg, ocaml/dune#3117)

- Make sure the `@all` alias is defined when no `dune` file is present
  in a directory (ocaml/dune#2946, fix ocaml/dune#2927, @diml)
mgree pushed a commit to mgree/opam-repository that referenced this issue Feb 19, 2020
@rgrinberg @mgree
[new release] dune-build-info, dune, dune-configurator, dune-action-p…
9848870 
…lugin, dune-private-libs and dune-glob (2.3.0)

CHANGES:

- Improve validation and error handling of arguments to `dune init` (ocaml/dune#3103, fixes
  ocaml/dune#3046, @shonfeder)

- `dune init exec NAME` now uses the `NAME` argument for private modules (ocaml/dune#3103,
  fixes ocaml/dune#3088, @shonfeder)

- Avoid linear walk to detect children, this should greatly improve
  performance when a target has a large number of dependencies (ocaml/dune#2959,
  @ejgallego, @aalekseyev, @Armael)

- [coq] Add `(boot)` option to `(coq.theories)` to enable bootstrap of
  Coq's stdlib (ocaml/dune#3096, @ejgallego)

- [coq] Deprecate `public_name` field in favour of `package` (ocaml/dune#2087, @ejgallego)

- Better error reporting for "data only" and "vendored" dirs. Using these with
  anything else than a strict subdirectory or `*` will raise an error. The
  previous behavior was to just do nothing  (ocaml/dune#3056, fixes ocaml/dune#3019, @voodoos)

- Fix bootstrap on bytecode only switches on windows or where `-j1` is set.
  (ocaml/dune#3112, @xclerc, @rgrinberg)

- Allow `enabled_if` fields in `executable(s)` stanzas (ocaml/dune#3137, fixes ocaml/dune#1690
  @voodoos)

- Do not fail if `ocamldep`, `ocamlmklib`, or `ocaml` are absent. Wait for them
  to be used to fail (ocaml/dune#3138, @rgrinberg)

- Introduce a `strict_package_deps` mode that verifies that dependencies between
  packages in the workspace are specified correctly. (@rgrinberg, ocaml/dune#3117)

- Make sure the `@all` alias is defined when no `dune` file is present
  in a directory (ocaml/dune#2946, fix ocaml/dune#2927, @diml)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@voodoos voodoos

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@bobot @rgrinberg @voodoos @ejgallego