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

Parallel build issue in 1.9.1 #2061

Closed
kit-ty-kate opened this issue Apr 16, 2019 · 5 comments
Closed

Parallel build issue in 1.9.1 #2061

kit-ty-kate opened this issue Apr 16, 2019 · 5 comments

Comments

@kit-ty-kate
Copy link
Member

When building cppo.1.6.0 on opam on a machine with 72 cores with OCaml 4.02.3, the build has a 1/24 chance to fail with:

#=== ERROR while compiling cppo.1.6.0 =========================================#
# context              2.0.3 | linux/x86_64 | ocaml-base-compiler.4.02.3 | file:///home/opam/opam-repository
# path                 ~/.opam/4.02.3/.opam-switch/build/cppo.1.6.0
# command              ~/.opam/4.02.3/bin/jbuilder build -p cppo -j 71
# exit-code            1
# env-file             ~/.opam/log/cppo-1-fd6dea.env
# output-file          ~/.opam/log/cppo-1-fd6dea.out
### output ###
# [...]
# Warning: Aliases must not have targets, this target will be ignored.
# This will become an error in the future.
# File "_build/.dune/default/src/jbuild", line 5, characters 0-25:
# 5 | (ocamlyacc (cppo_parser))
#     ^^^^^^^^^^^^^^^^^^^^^^^^^
# Warning: File cppo_parser.mli is both generated by a rule and present in the source tree.
# As a result, the rule is currently ignored, however this will become an error in the future.
# To keep the current behavior and get rid of this warning, add a field (fallback) to the rule.
#       ocamlc src/.cppo_main.eobjs/byte/cppo_parser.{cmi,cmti} (exit 2)
# (cd _build/default && /home/opam/.opam/4.02.3/bin/ocamlc.opt -w -40 -g -bin-annot -I src/.cppo_main.eobjs/byte -no-alias-deps -o src/.cppo_main.eobjs/byte/cppo_parser.cmi -c -intf src/cppo_parser.mli)
# File "src/cppo_parser.mli", line 2, characters 13-27:
# Error: Unbound module Cppo_types

- Note: You can use "dune upgrade" to convert your project to dune.
- File "test/jbuild", line 96, characters 26-39:
- 96 |   (action (with-stdout-to test.cppo.out (run ${bin:cppo} ${<})))))
-                                ^^^^^^^^^^^^^
- Warning: Aliases must not have targets, this target will be ignored.
- This will become an error in the future.
- File "_build/.dune/default/src/jbuild", line 5, characters 0-25:
- 5 | (ocamlyacc (cppo_parser))
-     ^^^^^^^^^^^^^^^^^^^^^^^^^
- Warning: File cppo_parser.mli is both generated by a rule and present in the source tree.
- As a result, the rule is currently ignored, however this will become an error in the future.
- To keep the current behavior and get rid of this warning, add a field (fallback) to the rule.
-       ocamlc src/.cppo_main.eobjs/byte/cppo_parser.{cmi,cmti} (exit 2)
- (cd _build/default && /home/opam/.opam/4.02.3/bin/ocamlc.opt -w -40 -g -bin-annot -I src/.cppo_main.eobjs/byte -no-alias-deps -o src/.cppo_main.eobjs/byte/cppo_parser.cmi -c -intf src/cppo_parser.mli)
- File "src/cppo_parser.mli", line 2, characters 13-27:
- Error: Unbound module Cppo_types

Here is the content of the jbuild file in src:

(* -*- tuareg -*- *)
#require "unix"

let version =
  let ic = open_in "../VERSION" in
  let version = input_line ic in
  close_in ic;
  version

let () = Printf.ksprintf Jbuild_plugin.V1.send {|
(jbuild_version 1)

(ocamllex (cppo_lexer))
(ocamlyacc (cppo_parser))

(rule
 ((targets (cppo_version.ml))
  (action
   (with-stdout-to ${@}
    (echo "let cppo_version = \"%s\"")))))

(executable
 ((name cppo_main)
  (package cppo)
  (public_name cppo)
  (libraries (unix str))))
|} version

and here is the content of src:

-rw-r--r-- cppo_command.ml
-rw-r--r-- cppo_eval.ml
-rw-r--r-- cppo_lexer.mll
-rw-r--r-- cppo_main.ml
-rw-r--r-- cppo_parser.mli
-rw-r--r-- cppo_parser.mly
-rw-r--r-- cppo_types.ml
-rw-r--r-- jbuild

I probably don't have enough knowledge in dune but it all seems fine to me, there shouldn't be any problem.
I'm also starting to suspect ocaml/opam-repository#13954 and ocaml/opam-repository#13980 might also be related to this problem.
I'm not sure those problems were there before 1.9.1 but maybe I'm wrong. I think I at least saw the problem on Camomile before but I'm not sure.

@ghost
Copy link

ghost commented Apr 17, 2019

Would it be possible to see the _build/log in this case?

@ghost
Copy link

ghost commented Apr 17, 2019

I believe the problem is the following:

  • (ocamlyacc (cppo_parser)) produces both cppo_parser.mli and cppo_parser.ml
  • cppo_parser.mli is present in the source tree

This is no longer allowed in dune files, however it is in jbuild files to be compatible with the original behaviour of jbuilder. I believe the following happens:

  • the rule to copy cppo_parser.mli from the source tree to _build kicks in
  • ocamldep is called on cppo_parser.mli
  • at the same time, the ocamlyacc rule kicks in
  • ocalmdep sees an invalid file, silently produces nothing and exit with code 0
  • dune thinks cppo_parser.mli has no dependency and as a result doesn't try to build cppo_types.mli before cppo_parser.mli

I'll see if we can improve dune's behavior in this case, but in the meantime a quick fix is to run: rm -f src/cppo_parser.mli just before calling jbuilder

@ghost ghost self-assigned this Apr 17, 2019
@kit-ty-kate
Copy link
Member Author

menhir seems to also have the same behaviour and I wasn't even aware that they would overwrite the .mli file if it's already present. I did the mistake in one of my project too apparently.

@kit-ty-kate
Copy link
Member Author

Thanks a lot for looking into this!

@ghost
Copy link

ghost commented Apr 17, 2019

No problem. The handling of this case was just dodgy. It's good that we finally clean it up.

ghost pushed a commit that referenced this issue Apr 23, 2019
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
@ghost ghost closed this as completed in f5c0849 Apr 23, 2019
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue May 30, 2019
CHANGES:

- Restricted the set of variables available for expansion in the destination
  filename of `install` stanza to simplify implementation and avoid dependency
  cycles. (ocaml/dune#2073, @aalekseyev, @diml)

- [menhir] call menhir from context root build_dir (ocaml/dune#2067, @ejgallego,
  review by @diml, @rgrinberg)

- [coq] Add `coq.pp` stanza to help with pre-processing of grammar
  files (ocaml/dune#2054, @ejgallego, review by @rgrinberg)

- Add a new more generic form for the *promote* mode: `(promote
  (until-clean) (into <dir>))` (ocaml/dune#2068, @diml)

- Allow to promote only a subset of the targets via `(promote (only
  <pred>))`. For instance: `(promote (only *.mli))` (ocaml/dune#2068, @diml)

- Improve the behavior when a strict subset of the targets of a rule is already
  in the source tree for projects using the dune language < 1.10 (ocaml/dune#2068, fixes
  ocaml/dune#2061, @diml)

- With lang dune >= 1.10, rules in standard mode are no longer allowed to
  produce targets that are present in the source tree. This has been a warning
  for long enough (ocaml/dune#2068, @diml)

- Allow %{...} variables in pps flags (ocaml/dune#2076, @mlasson review by @diml and
  @aalekseyev).

- Add a 'cookies' option to ppx_rewriter/deriver flags in library stanzas. This
  allow to specify cookie requests from variables expanded at each invocation of
  the preprocessor. (ocaml/dune#2106, @mlasson @diml)

- Add more opam metadata and use it to generate `.opam` files. In particular, a
  `package` field has been added to specify package specific information.
  (ocaml/dune#2017, ocaml/dune#2091, @avsm, @jonludlam, @rgrinberg)

- Clean up the special support for `findlib.dynload`. Before, Dune would simply
  match on the library name. Now, we only match on the findlib package name when
  the library doesn't come from Dune. Someone writing a library called
  `findlib.dynload` with Dune would have to add `(special_builtin_support
  findlib_dynload)` to trigger the special behavior. (ocaml/dune#2115, @diml)

- Install the `future_syntax` preprocessor as `ocaml-syntax-shims.exe` (ocaml/dune#2125,
  @rgrinberg)

- Hide full command on errors and warnings in development and show them in CI.
  (detected using the `CI` environment variable). Commands for which the
  invocation might be omitted must output an error prefixed with `File `. Add an
  `--always-show-command-line` option to disable this behavior and always show
  the full command. (ocaml/dune#2120, fixes ocaml/dune#1733, @rgrinberg)

- In `dune-workspace` files, add the ability to choose the host context and to
  create duplicates of the default context with different settings. (ocaml/dune#2098,
  @TheLortex, review by @diml, @rgrinberg and @aalekseyev)

- Add support for hg in `dune subst` (ocaml/dune#2135, @diml)

- Don't build documentation for implementations of virtual libraries (ocaml/dune#2141,
  fixes ocaml/dune#2138, @jonludlam)

- Fix generation of the `-pp` flag in .merlin (ocaml/dune#2142, @rgrinberg)

- Make `dune subst` add a `(version ...)` field to the `dune-project`
  file (ocaml/dune#2148, @diml)

- Add the `%{os_type}` variable, which is a short-hand for
  `%{ocaml-config:os_type}` (ocaml/dune#1764, @diml)

- Allow `enabled_if` fields in `library` stanzas, restricted to the
  `%{os_type}`, `%{model}`, `%{architecture}`, `%{system}` variables (ocaml/dune#1764,
  ocaml/dune#2164 @diml, @rgrinberg)

- Fix `chdir` on external and source paths. Dune will also fail gracefully if
  the external or source path does not exist (ocaml/dune#2165, fixes ocaml/dune#2158, @rgrinberg)

- Support the `.cc` extension fro C++ sources (ocaml/dune#2195, fixes ocaml/dune#83, @rgrinberg)

- Run `ocamlformat` relative to the context root. This improves the locations of
  errors. (ocaml/dune#2196, fixes ocaml/dune#1370, @rgrinberg)

- Fix detection of `README`, `LICENSE`, `CHANGE`, and `HISTORY` files. These
  would be undetected whenever the project was nested in another workspace.
  (ocaml/dune#2194, @rgrinberg)

- Fix generation of `.merlin` whenever there's more than one stanza with the
  same ppx preprocessing specification (ocaml/dune#2209 ,fixes ocaml/dune#2206, @rgrinberg)

- Fix generation of `.meriln` in the presence of the `copy_files` stanza and
  preprocessing specifications of other stanazs. (ocaml/dune#2211, fixes ocaml/dune#2206,
  @rgrinberg)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Jun 4, 2019
CHANGES:

- Restricted the set of variables available for expansion in the destination
  filename of `install` stanza to simplify implementation and avoid dependency
  cycles. (ocaml/dune#2073, @aalekseyev, @diml)

- [menhir] call menhir from context root build_dir (ocaml/dune#2067, @ejgallego,
  review by @diml, @rgrinberg)

- [coq] Add `coq.pp` stanza to help with pre-processing of grammar
  files (ocaml/dune#2054, @ejgallego, review by @rgrinberg)

- Add a new more generic form for the *promote* mode: `(promote
  (until-clean) (into <dir>))` (ocaml/dune#2068, @diml)

- Allow to promote only a subset of the targets via `(promote (only
  <pred>))`. For instance: `(promote (only *.mli))` (ocaml/dune#2068, @diml)

- Improve the behavior when a strict subset of the targets of a rule is already
  in the source tree for projects using the dune language < 1.10 (ocaml/dune#2068, fixes
  ocaml/dune#2061, @diml)

- With lang dune >= 1.10, rules in standard mode are no longer allowed to
  produce targets that are present in the source tree. This has been a warning
  for long enough (ocaml/dune#2068, @diml)

- Allow %{...} variables in pps flags (ocaml/dune#2076, @mlasson review by @diml and
  @aalekseyev).

- Add a 'cookies' option to ppx_rewriter/deriver flags in library stanzas. This
  allow to specify cookie requests from variables expanded at each invocation of
  the preprocessor. (ocaml/dune#2106, @mlasson @diml)

- Add more opam metadata and use it to generate `.opam` files. In particular, a
  `package` field has been added to specify package specific information.
  (ocaml/dune#2017, ocaml/dune#2091, @avsm, @jonludlam, @rgrinberg)

- Clean up the special support for `findlib.dynload`. Before, Dune would simply
  match on the library name. Now, we only match on the findlib package name when
  the library doesn't come from Dune. Someone writing a library called
  `findlib.dynload` with Dune would have to add `(special_builtin_support
  findlib_dynload)` to trigger the special behavior. (ocaml/dune#2115, @diml)

- Install the `future_syntax` preprocessor as `ocaml-syntax-shims.exe` (ocaml/dune#2125,
  @rgrinberg)

- Hide full command on errors and warnings in development and show them in CI.
  (detected using the `CI` environment variable). Commands for which the
  invocation might be omitted must output an error prefixed with `File `. Add an
  `--always-show-command-line` option to disable this behavior and always show
  the full command. (ocaml/dune#2120, fixes ocaml/dune#1733, @rgrinberg)

- In `dune-workspace` files, add the ability to choose the host context and to
  create duplicates of the default context with different settings. (ocaml/dune#2098,
  @TheLortex, review by @diml, @rgrinberg and @aalekseyev)

- Add support for hg in `dune subst` (ocaml/dune#2135, @diml)

- Don't build documentation for implementations of virtual libraries (ocaml/dune#2141,
  fixes ocaml/dune#2138, @jonludlam)

- Fix generation of the `-pp` flag in .merlin (ocaml/dune#2142, @rgrinberg)

- Make `dune subst` add a `(version ...)` field to the `dune-project`
  file (ocaml/dune#2148, @diml)

- Add the `%{os_type}` variable, which is a short-hand for
  `%{ocaml-config:os_type}` (ocaml/dune#1764, @diml)

- Allow `enabled_if` fields in `library` stanzas, restricted to the
  `%{os_type}`, `%{model}`, `%{architecture}`, `%{system}` variables (ocaml/dune#1764,
  ocaml/dune#2164 @diml, @rgrinberg)

- Fix `chdir` on external and source paths. Dune will also fail gracefully if
  the external or source path does not exist (ocaml/dune#2165, fixes ocaml/dune#2158, @rgrinberg)

- Support the `.cc` extension fro C++ sources (ocaml/dune#2195, fixes ocaml/dune#83, @rgrinberg)

- Run `ocamlformat` relative to the context root. This improves the locations of
  errors. (ocaml/dune#2196, fixes ocaml/dune#1370, @rgrinberg)

- Fix detection of `README`, `LICENSE`, `CHANGE`, and `HISTORY` files. These
  would be undetected whenever the project was nested in another workspace.
  (ocaml/dune#2194, @rgrinberg)

- Fix generation of `.merlin` whenever there's more than one stanza with the
  same ppx preprocessing specification (ocaml/dune#2209 ,fixes ocaml/dune#2206, @rgrinberg)

- Fix generation of `.merlin` in the presence of the `copy_files` stanza and
  preprocessing specifications of other stanazs. (ocaml/dune#2211, fixes ocaml/dune#2206,
  @rgrinberg)

- Run `refmt` from the context's root directory. This improves error messages in
  case of syntax errors. (ocaml/dune#2223, @rgrinberg)

- In .merlin files, don't pass `-dump-ast` to the `future_syntax` preprocessor.
  Merlin doesn't seem to like it when binary AST is generated by a `-pp`
  preprocessor. (ocaml/dune#2236, @aalekseyev)

- `dune install` will verify that all files mentioned in all .install files
  exist before trying to install anything. This prevents partial installation of
  packages (ocaml/dune#2230, @rgrinberg)
This issue was closed.
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

1 participant