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

Allow list expansion in preprocessor arguments #5820

Closed
wants to merge 2 commits into from
Closed

Allow list expansion in preprocessor arguments #5820

wants to merge 2 commits into from

Conversation

Firobe
Copy link
Contributor

@Firobe Firobe commented Jun 3, 2022

This PR allows templates to expand into multiple values in preprocessor flags, like:

 (preprocess (pps my_ppx -- arg1 arg2 %{read-lines:my-ppx-flags} arg8 arg9))

This makes it consistent with similar usage of read-lines for run to pass correctly separated arguments.

@rgrinberg
Copy link
Member

Thanks for the PR. We need a few more changes to merge it though:

  • A test demonstrating the issue
  • Guarding the new behavior behind a version check. The old behavior should be preserved if the project version is 3.2 or less.

If you have any questions on how to do the above, please don't hesitate to ask.

@Firobe
Copy link
Contributor Author

Firobe commented Jun 13, 2022

Hey @rgrinberg, I have a test but I'm not against some help to retrieve the current dune lang version, in particular how to escape the Decoder monad if using that interface? I imagine we have to pass it from elsewhere, but I'm not sure where :)

@rgrinberg
Copy link
Member

You can obtain the version from Scope.project scope |> Dune_project.dune_version. I think the caller has the scope so you should be able to pass in the version to the function you're modifying.

@Firobe
Copy link
Contributor Author

Firobe commented Jun 14, 2022

Thank you! I think the PR is ready to be reviewed again.

For the test, I tested the new behavior in a separate test for the (yet non-existent) version 3.3, after checking that the behavior actually works if there is no version guard. I'm not sure if you have a process for writing tests for the version currently in development.

Firobe added 2 commits June 14, 2022 17:47
Only enably new behavior for dune above 3.2

Signed-off-by: Virgile Robles <[email protected]>
Signed-off-by: Virgile Robles <[email protected]>
@rgrinberg rgrinberg added this to the 3.3.0 milestone Jun 14, 2022
@rgrinberg
Copy link
Member

Merged. Thanks.

@rgrinberg rgrinberg closed this Jun 14, 2022
kit-ty-kate pushed a commit to ocaml/opam-repository that referenced this pull request Jun 17, 2022
…ne-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.3.0)

CHANGES:

- Sandbox preprocessing, lint, and dialect rules by default. All these rules
  now require precise dependency specifications (ocaml/dune#5807, @rgrinberg)

- Allow list expansion in the `pps` specification for preprocessing (ocaml/dune#5820,
  @Firobe)

- Add warnings 67-69 to dune's default set of warnings. These are warnings of
  the form "unused X.." (ocaml/dune#5844, @rgrinbreg)

- Introduce project "composition" for coq theories. Coq theories in separate
  projects can now refer to each other when in the same workspace (ocaml/dune#5784,
  @Alitzer, @rgrinberg)

- Fix hint message for ``data_only_dirs`` that wrongly mentions the unknown
  constructor ``data_only`` (ocaml/dune#5803, @lambdaxdotx)

- Fix creating sandbox directory trees by getting rid of buggy memoization
  (@5794, @rgrinberg, @snowleopard)

- Handle directory dependencies in sandboxed rules. Previously, the parents of
  these directory dependencies weren't created. (ocaml/dune#5754, @rgrinberg)

- Set the exit code to 130 when dune is terminated with a signal (ocaml/dune#5769, fixes
  ocaml/dune#5757)

- Support new locations of unix, str, dynlink in OCaml >= 5.0 (ocaml/dune#5582, @dra27)

- The ``coq.theory`` stanza now produces rules for running ``coqdoc``. Given a
  theory named ``mytheory``, the directory targets ``mytheory.html/`` and
  ``mytheory.tex/`` or additionally the aliases `@doc` and `@doc-latex` will
  build the HTML and LaTeX documentation repsectively. (ocaml/dune#5695, fixes ocaml/dune#3760,
  @Alizter)

- Coq theories marked as `(boot)` cannot depend on other theories
  (ocaml/dune#5867, @ejgallego)

- Ignore `bigarray` in `(libraries)` with OCaml >= 5.0. (ocaml/dune#5526, fixes ocaml/dune#5494,
  @moyodiallo)

- Start with :standard when building the ctypes generated foreign stubs so that
  we include important compiler flags, such as -fPIC (ocaml/dune#5816, fixes ocaml/dune#5809).
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

Successfully merging this pull request may close these issues.

2 participants