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

FLG -ppx missing #2206

Closed
Khady opened this issue May 28, 2019 · 6 comments · Fixed by #2208 or ocaml/opam-repository#14239
Closed

FLG -ppx missing #2206

Khady opened this issue May 28, 2019 · 6 comments · Fixed by #2208 or ocaml/opam-repository#14239

Comments

@Khady
Copy link
Contributor

Khady commented May 28, 2019

We have a project that was using dune 1.8 and was working fine. Since we upgraded to 1.9.3, the generation of the .merlin file is broken. the FLG directive is missing. And dune does not print a warning.

The code to reproduce:

https://gist.github.com/Khady/4ffc822f6b127c1bd4862701de0868f1

rgrinberg added a commit to rgrinberg/jbuilder that referenced this issue May 28, 2019
Previously, if the pps were all ppx and equivalent, then they would
be dropped.

Fix ocaml#2206

Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
@Khady
Copy link
Contributor Author

Khady commented May 29, 2019

#2208 fixes the testcase from this issue. Thanks.

But we also have another problem on master/your branch. We have this dune file (that is not at the root of the project, if it matters). It has 2 occurrences of preprocess that are the same. Despite those 2 proprocess being identical, dune is unhappy. dune_gen.sexp does not contain any library or executable. Only some rules for atdgen as pasted bellow.

Everything was working fine under dune 1.8.

Compilation message:

File "api/src/dune", line 24, characters 13-43:
24 |  (preprocess (pps lwt_ppx ppx_deriving.std)))
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning: .merlin generated is inaccurate. Cannot mix preprocessed and non preprocessed specificiations.
Split the stanzas into different directories or silence this warning by adding (allow_approximate_merlin) to your dune-project.
File "api/src/dune", line 30, characters 13-43:
30 |  (preprocess (pps lwt_ppx ppx_deriving.std)))
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning: .merlin generated is inaccurate. Cannot mix preprocessed and non preprocessed specificiations.
Split the stanzas into different directories or silence this warning by adding (allow_approximate_merlin) to your dune-project.

The dune file:

(copy_files# "gen/*.ml{i,}")
(copy_files# "request/*")
(copy_files# "shared/*")
(copy_files# "keywordsExplorer/*")

(library
 (name apicore)
 (modules (:standard \ main))
 (flags (:standard -w -33)) ; because of EsMapping in atd files
 (libraries
  threads                       ; must be first
  ahrefskit
  angstrom
  atdgen
  bigstringaf
  billing.billing_client
  csv
  devkit
  elastic
  extlib
  fmt
  yojson
 )
 (preprocess (pps lwt_ppx ppx_deriving.std)))

(executable
 (name main)
 (modules main)
 (libraries extlib apicore ahrefskit.version)
 (preprocess (pps lwt_ppx ppx_deriving.std)))

;;; Dune files generation for atdgen
(rule
 (targets dune_gen.sexp.gen)
 (action (with-stdout-to dune_gen.sexp.gen (run ./gen_dune.sh)))
 (deps (glob_files *.atd)))

(alias
  (name gen_dune)
  (action
    (diff dune_gen.sexp dune_gen.sexp.gen)))

(include dune_gen.sexp)

(data_only_dirs gen)

One of the rules in dune_gen.sexp:

(rule
  (targets config_t.mli config_t.ml config_j.mli config_j.ml)
  (deps config.atd)
  (action (progn (run %{bin:atdgen} -j -j-std %{deps}) (run %{bin:atdgen} -t %{deps}))))

@rgrinberg
Copy link
Member

Does dune_gen.sexp define any executable, test, or library stanazs?

rgrinberg added a commit to rgrinberg/jbuilder that referenced this issue May 29, 2019
Previously, if the pps were all ppx and equivalent, then they would
be dropped.

Fix ocaml#2206

Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
rgrinberg added a commit to rgrinberg/jbuilder that referenced this issue May 29, 2019
Previously, if the pps were all ppx and equivalent, then they would
be dropped.

Fix ocaml#2206

Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
@Khady
Copy link
Contributor Author

Khady commented May 29, 2019

No, only rules that similar to the one pasted previously. There is one rule per atd file in the project. That's it.

Also to be more precise:

  • 1.8 works fine
  • 1.9.3 does not generate the FLG -ppx directive and does not display a warning
  • master does not generate the FLG -ppx directive but DOES display a warning

@rgrinberg
Copy link
Member

And what about my PR?

@Khady
Copy link
Contributor Author

Khady commented May 29, 2019

Same as master

@rgrinberg
Copy link
Member

Okay, thanks. I understand what the issue is.

rgrinberg added a commit to rgrinberg/jbuilder that referenced this issue May 29, 2019
Previously, if the pps were all ppx and equivalent, then they would
be dropped.

Fix ocaml#2206

Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
rgrinberg added a commit to rgrinberg/jbuilder that referenced this issue May 29, 2019
Previously, if the pps were all ppx and equivalent, then they would
be dropped.

Fix ocaml#2206

Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
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)
mlasson pushed a commit to mlasson/dune that referenced this issue Jul 19, 2019
Previously, if the pps were all ppx and equivalent, then they would
be dropped.

Fix ocaml#2206

Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants