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

(optional) in executables doesn't work #5621

Open
talex5 opened this issue Apr 26, 2022 · 15 comments
Open

(optional) in executables doesn't work #5621

talex5 opened this issue Apr 26, 2022 · 15 comments

Comments

@talex5
Copy link

talex5 commented Apr 26, 2022

My project has two packages, prometheus and prometheus-app, in one repository. prometheus can be installed on OCaml 4.03, but prometheus-app requires a later version.

For CI, I want to be able to dune build --only-packages prometheus on 4.03. However, this tries to build an example that needs prometheus-app.

I tried using:

(executable
  (name example)
  (package prometheus-app)

but that fails with

Error: This field is useless without a (public_name ...) field.

Adding a (public_name) does fix it, but I don't want the example to be installed.

I then tried using (optional), but that doesn't seem to have any effect. e.g.

$ cat example/dune 
(executable
  (name test)
  (optional)
  (libraries nosuchlib))
$ dune build
File "example/dune", line 4, characters 13-22:
4 |   (libraries nosuchlib))
Error: Library "nosuchlib" not found.

I also tried using (enabled_if "%{lib-available:nosuchlib}%"), but it says this isn't one of the allowed variables.

Expected Behavior

An (optional) executable shouldn't be built if something it depends on isn't present.

Actual Behavior

Dune tries to build it anyway.

Reproduction

dune init exe main
echo '(lang dune 3.0)' > dune-project
echo '(executable (name main) (optional) (libraries idontexist))' > dune
dune build

Specifications

  • Version of dune (output of dune --version): 3.0.3
  • Version of ocaml (output of ocamlc --version): 4.14.0
  • Operating system (distribution and version): Debian 11.3
@rgrinberg
Copy link
Member

I think you can just add your executable to the install stanza:

(alias
 (name install)
 (enabled_if ..)
 (deps ./example.exe))

But I don't understand why you want an example executables to be built when building the installation artifacts. Seems like you're just slowing down builds for your end users by building unnecessary stuff. If you just need this for CI, just build the executable explicitly with a different target.

@talex5
Copy link
Author

talex5 commented Apr 27, 2022

But I don't understand why you want an example executables to be built when building the installation artifacts. Seems like you're just slowing down builds for your end users by building unnecessary stuff. If you just need this for CI, just build the executable explicitly with a different target.

Indeed I don't want it built with just @install, but I do want to build the examples with a plain dune build when developing. The CI is doing dune build @install @check @runtest, so it seems reasonable to build the examples in that case. But I still need some way to attach the executable to a package so it's only built when testing that package.

@rgrinberg
Copy link
Member

So it sounds like you need:

(alias
 (name runtest)
 (package prometheus-app)
 (deps ./example.exe))

@talex5
Copy link
Author

talex5 commented Apr 29, 2022

How would that work? If I just add the alias to the existing dune file, then dune build --only-packages prometheus still tries to build it. And if I remove the (executable) stanza, then dune exec -- ./examples/example.exe doesn't work.

@rgrinberg
Copy link
Member

I'm not sure I understand the issue. The alias above builds ./example.exe only when building the tests target for the prometheus-app package. Isn't that what you're looking for?

@talex5
Copy link
Author

talex5 commented May 6, 2022

Here's the full behaviour I want:

1: users can run the example from a checkout:

git clone https://github.com/mirage/prometheus.git
cd prometheus
dune exec -- examples/example.exe --listen-prometheus=9090

2: the example executable is never installed

opam install prometheus-app
example
fish: Unknown command: example    # correct behaviour

3: the executable is not built unless its package (prometheus-app) is selected:

dune build --only-packages prometheus                 # should succeed, skipping the build
dune build --only-packages prometheus,prometheus-app  # should build the example

@rgrinberg
Copy link
Member

the executable is not built unless its package (prometheus-app) is selected:

This is the part that I don't understand. If the executable isn't public, why do you insist on building it when the prometheus-app package is built?

Nevertheless, it's possible to do this:

(alias
 (name install)
 (package prometheus-app)
 (deps ./example.exe))

By the way, your example relies on unstable/undocumented behavior. I assume you meant -p prometheus for the first command and -p prometheus-app for the second command. Which would be the actual invocations present in the opam file.

@rgrinberg
Copy link
Member

Of course my suggestion above goes against my earlier advice which you seemed to agree with

Reading this thread again, I feel like we're going in circles and not communicating our points across. At this point, the most productive thing would be for me to step back and let another maintainer step in and try and help you.

@rgrinberg
Copy link
Member

rgrinberg commented May 6, 2022

Reading the thread again, I think I understand the source of confusion. From these statements:

I want to be able to dune build --only-packages prometheus on 4.03. However, this tries to build an example that needs prometheus-app.

the executable is not built unless its package (prometheus-app) is selected:

You seem to think that --only-packages is there for you to "select" packages in some way. That's not how it works. From the doc:

           Ignore stanzas referring to a package that is not in PACKAGES.
           PACKAGES is a comma-separated list of package names. Note that this
           has the same effect as deleting the relevant stanzas from dune
           files. It is mostly meant for releases. During development, it is
           likely that what you want instead is to build a particular
           <package>.install target.

This command line is only for making releases. It is not for whatever other purpose you might rightfully assume from the suggestive wording (for which I apologize for).

To build a particular package you should organize your project such that every package has its own directory. For example, your prometheus project might be as follows:

$ ls
dune-project prometheus.opam prometheus-app.opam prometheus/ prometheus-app/
$ ls prometheus
src/ test/
$ ls prometheus-app
src/ examples/ test/

With this layout, you can now select packages to your heart's content:

$ dune build @prometheus/check
$ dune runtest prometheus-app
$ dune build prometheus-app/examples/

Etc.

I hope that was helpful.

@talex5
Copy link
Author

talex5 commented May 13, 2022

To build a particular package you should organize your project such that every package has its own directory.

This works for manual runs, but I'm looking for something that will work with ocaml-ci. The CI can see from the opam files which packages are compatible with each test platform, and it needs a way to tell dune to build only those packages.

--only-packages seems ideal, except that dune doesn't let you tag certain things with a package.

Apart from testing things more on older versions, the more pressing problem is related to OCaml 5. Many projects want to add support for effects (e.g. mirage/ocaml-cohttp#857 adds a cohttp_eio package). But then the CI can only test on the (pre-release) effects version of the compiler; all other platforms stop being tested.

@rgrinberg
Copy link
Member

except that dune doesn't let you tag certain things with a package.

What are those things?

@talex5
Copy link
Author

talex5 commented May 13, 2022

except that dune doesn't let you tag certain things with a package.

What are those things?

Such as an example executable that shouldn't be installed (has no public name). For that, it gives the error:

Error: This field is useless without a (public_name ...) field.

I guess the problem is that for CI we use dune build because we want to check everything that can be built, whereas dune's filtering is only designed for things needed by opam (install and test).

talex5 added a commit to talex5/prometheus that referenced this issue May 19, 2022
@sim642
Copy link

sim642 commented May 22, 2022

This is yet another occurrence of #4065, and I too find myself annoyed by this very issue time and time again.

In addition to optionally building example binaries, this also applies to benchmarking binaries, which depend on some benchmarking library that the rest of the package really does not. Moreover, I want the benchmark executable to depend on other libraries that provide similar functionality in order to measure and ensure performance parity. But I absolutely would not want my library to depend on every other library that has similar functionality.

Regarding this issue, I would like to repeat and emphasize this comment: #4065 (comment). For the executable stanza the documentation states:

  • (optional) is the same as the corresponding field of library.

But in light of #4065 and this issue, that is plainly false. On a non-public library, (optional) actually makes the library optional when dune build is run (this is easily checked by temporarily changing the executable to a library, which makes it useless, but stops dune from complaining). So the behavior of (optional) is different for executables and libraries, and as the numerous issues about this prove, this is inconsistent and confusing to users.

Couldn't the semantics of (optional) be changed to match the documentation, and users' intuition and expectation? As I understand, its only current use is for public/installable executables. Allowing it to behave the same way for all executables wouldn't change how it currently behaves for public ones, or does it?

@emillon
Copy link
Collaborator

emillon commented Apr 17, 2023

Hi,
I agree that the semantics of (optional) are not great. However, in the first message you mention that (enabled_if %{lib-available:nosuchlib}) would work for you. This is something that also came up in other contexts and that the dune engine should now be able to support.
Do you confirm that it would work for you?

@talex5
Copy link
Author

talex5 commented Apr 21, 2023

It's not ideal, but it might do as a work-around.

What I really want is to be able to attach examples to packages, but not have them installed. I can attach a (package ...) to a test stanza, which seems to define an executable implicitly, but I can't do that with (executable) for some reason.

Actually, this seems to solve my problem:

(test
  (name example)
  (package prometheus-app)
  (deps ./example.exe)
  (action (progn))
  (libraries prometheus prometheus-app.unix cmdliner cohttp-lwt cohttp-lwt-unix))

Now:

  • dune build and dune runtest build the example but don't run it
  • dune exec runs it
  • dune build and dune runtest with --only-packages prometheus doesn't try to build

This is the behaviour I was looking for :-)

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

No branches or pull requests

4 participants