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

dune 2.8.0 test issue #4110

Closed
kit-ty-kate opened this issue Jan 13, 2021 · 6 comments
Closed

dune 2.8.0 test issue #4110

kit-ty-kate opened this issue Jan 13, 2021 · 6 comments

Comments

@kit-ty-kate
Copy link
Member

Issues detected in ocaml/opam-repository#17967

The CI hasn't finished, but so far at least one issue already arose. Here is a test case:

$ cat dune
(rule
 (with-stdout-to
  test1.output
  (run cat test1.expected)))

(rule
 (alias runtest)
 (action
  (diff test1.expected test1.output)))
$ dune runtest
         cat test1.output (exit 1)
(cd _build/default && /usr/bin/cat test1.expected) > _build/default/test1.output
/usr/bin/cat: test1.expected: No such file or directory

This pattern is used in drom and other packages from ocamlpro.

Specifications

@rgrinberg
Copy link
Member

I have no idea how this worked before, the rule that produces test1.output does not specify a dependency on test1.expected. So it must have worked by accident previously?

@rgrinberg
Copy link
Member

I confirmed that fixing the rule:


(rule
 (deps test1.expected)
 (action
  (with-stdout-to
   test1.output
   (run cat test1.expected))))

fixes the problem

@kit-ty-kate
Copy link
Member Author

Oh, I always thought dune automatically inferred the file dependency from the arguments.
So I'll just make those packages incompatible with dune 2.8.0 (for the tests only). Thanks a lot!

@rgrinberg
Copy link
Member

Oh, I always thought dune automatically inferred the file dependency from the arguments.

This works only for primitive actions that dune understands, like with-stdout-to in that example.

You could also rewrite the above to be:

(rule (with-stdout-to test1.output (echo %{read:test1.expected})))

and then dune will be smart enough to infer the dependency. But since cat is just a random binary in the original rule, dune assumes knowing about its dependencies or targets.

@nojb
Copy link
Collaborator

nojb commented Jan 14, 2021

You could also rewrite the above to be:

(rule (with-stdout-to test1.output (echo %{read:test1.expected})))

Or even simpler

(rule (with-stdout-to test1.output (cat test1.expected)))

@bobot
Copy link
Collaborator

bobot commented Jan 14, 2021

You can also use run cat %{dep:test1.expected} to add dependencies directly in an action.

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