Add (alias ...), (mode ...) fields to (copy_files ...) stanza by nojb · Pull Request #3631 · ocaml/dune

nojb · 2020-07-15T19:17:18Z

What the title says:

(alias ...) field to specify an alias to which to attach the copy_files stanza's targets.
(mode ...) field with the same semantics as the one appearing in (rule ...).

The full syntax looks like:

(copy_files
 (alias foo)
 (mode promote-until-clean)
 (glob <glob>))

(maybe glob should be called names).

What do you think?

nojb · 2020-07-15T19:19:13Z

This looks also related #3503 and (in a lesser way) #1244

nojb · 2020-07-15T19:25:05Z

src/dune/dune_file.ml

+
+  let decode =
+    peek_exn >>= function
+    | List _ -> Dune_lang.Syntax.since Stanza.syntax (2, 7) >>> fields long_form


This gives

Error: 'copy_files' is only available since version 2.7 of the dune language. Please update your dune-project file to have (lang dune 2.7).

as error message, which is not exactly what we want; suggestions appreciated.

How about:

Error: (alias) and (mode) are available in this position only since version 2.7 of the dune language.
Please update your dune-project file to have (lang dune 2.7)

You can also have a look at how (enabled_if) behaves, we added support for it in more positions later on, so the error message might be similar.

Thanks for the hint, I did something similar to what is done in other places.

rgrinberg

I'm not sure how to fix the error message issue, but it doesn't seem like a blocker. This needs a CHANGES/doc update as well.

rgrinberg · 2020-07-15T19:42:25Z

src/dune/simple_rules.ml

+    ~f:(fun alias ->
+      let alias = Alias.make alias ~dir in
+      Rules.Produce.Alias.add_deps alias targets)
+    def.alias;


Could you move def.alias to be the first argumnt? Makes it simpler to read.

Thanks, it is fixed.

nojb · 2020-07-15T19:49:48Z

Thanks for the lightning-quick review @rgrinberg! Do you think we should rename (glob ...) by (names ...) (or something else), or is glob OK?

nojb · 2020-07-15T19:57:06Z

Doc updated.

rgrinberg · 2020-07-15T22:51:54Z

glob seems alright to me. Let's see if @jeremiedimino has another suggestion.

ghost · 2020-07-16T07:30:56Z

Usually, we use names to denote files when we don't put the full filenames and let Dune infer the filenames from the context. For instance (c_names foo bar) to denote foo.c bar.c. In this case, I'd just use files .... It's consistent with the field names for install or cinaps.

Signed-off-by: Nicolás Ojeda Bär <[email protected]>

nojb · 2020-07-16T08:48:56Z

Renamed (glob ...) into (files ...).

Signed-off-by: Nicolás Ojeda Bär <[email protected]>

nojb · 2020-07-16T11:15:52Z

I fixed the error message and pushed a small test as well. On my side this is good to go.

ghost · 2020-07-16T13:28:34Z

Cool, let's wait to see if @rgrinberg wants to comment on the naming, but otherwise it looks good to merge!

rgrinberg · 2020-07-16T20:21:44Z

Looks good to me. I kind of wish the stanza was called copy though. We'll fix it in 3.0 I suppose :)

nojb · 2020-07-17T04:45:06Z

Merged, thanks!

@NathanReb

…lugin, dune-private-libs and dune-glob (2.7.0) CHANGES: - Write intermediate files in a `.mdx` folder for each `mdx` stanza to prevent the corresponding actions to be executed as part of the `@all` alias (ocaml/dune#3659, @NathanReb) - Read Coq flags from `env` (ocaml/dune#3547 , fixes ocaml/dune#3486, @gares) - Allow bisect_ppx to be enabled/disabled via dune-workspace. (ocaml/dune#3404, @stephanieyou) - Formatting of dune files is now done in the executing dune process instead of in a separate process. (ocaml/dune#3536, @nojb) - Add a `--debug-artifact-substution` flag to help debug problem with version not being captured by `dune-build-info` (ocaml/dune#3589, @jeremiedimino) - Allow the use of the `context_name` variable in the `enabled_if` fields of executable(s) and install stanzas. (ocaml/dune#3568, fixes ocaml/dune#3566, @voodoos) - Fix compatibility with OCaml 4.12.0 when compiling empty archives; no .a file is generated. (ocaml/dune#3576, @dra27) - `$ dune utop` no longer tries to load optional libraries that are unavailable (ocaml/dune#3612, fixes ocaml/dune#3188, @anuragsoni) - Fix dune-build-info on 4.10.0+flambda (ocaml/dune#3599, @emillon, @jeremiedimino). - Allow multiple libraries with `inline_tests` to be defined in the same directory (ocaml/dune#3621, @rgrinberg) - Run exit hooks in jsoo separate compilation mode (ocaml/dune#3626, fixes ocaml/dune#3622, @rgrinberg) - Add (alias ...), (mode ...) fields to (copy_fields ...) stanza (ocaml/dune#3631, @nojb) - (copy_files ...) now supports copying files from outside the workspace using absolute file names (ocaml/dune#3639, @nojb) - Dune does not use `ocamlc` as an intermediary to call C compiler anymore. Configuration flags `ocamlc_cflags` and `ocamlc_cppflags` are always prepended to the compiler arguments. (ocaml/dune#3565, fixes ocaml/dune#3346, @voodoos) - Revert the build optimization in ocaml/dune#2268. This optimization slows down building individual executables when they're part of an `executables` stanza group (ocaml/dune#3644, @rgrinberg) - Use `{dev}` rather than `{pinned}` in the generated `.opam` file. (ocaml/dune#3647, @kit-ty-kate) - Insert correct extension name when editing `dune-project` files. Previously, dune would just insert the stanza name. (ocaml/dune#3649, fixes ocaml/dune#3624, @rgrinberg) - Fix crash when evaluating an `mdx` stanza that depends on unavailable packages. (ocaml/dune#3650, @craigfe) - Fix typo in `cache-check-probablity` field in dune config files. This field now requires 2.7 as it wasn't usable before this version. (ocaml/dune#3652, @edwintorok) - Add `"odoc" {with-doc}` to the dependencies in the generated `.opam` files. (ocaml/dune#3667, @kit-ty-kate) - Do not allow user actions to capture dune's stdin (ocaml/dune#3677, fixes ocaml/dune#3672, @rgrinberg) - `(subdir ...)` stanzas can now appear in dune files used via `(include ...)`. (ocaml/dune#3676, @nojb)

@NathanReb

…lugin, dune-private-libs and dune-glob (2.7.0) CHANGES: - Write intermediate files in a `.mdx` folder for each `mdx` stanza to prevent the corresponding actions to be executed as part of the `@all` alias (ocaml/dune#3659, @NathanReb) - Read Coq flags from `env` (ocaml/dune#3547 , fixes ocaml/dune#3486, @gares) - Add instrumentation framework to toggle instrumentation by `bisect_ppx`, `landmarks`, etc, via dune-workspace and/or the command-line. (ocaml/dune#3404, ocaml/dune#3526 @stephanieyou, @nojb) - Formatting of dune files is now done in the executing dune process instead of in a separate process. (ocaml/dune#3536, @nojb) - Add a `--debug-artifact-substution` flag to help debug problem with version not being captured by `dune-build-info` (ocaml/dune#3589, @jeremiedimino) - Allow the use of the `context_name` variable in the `enabled_if` fields of executable(s) and install stanzas. (ocaml/dune#3568, fixes ocaml/dune#3566, @voodoos) - Fix compatibility with OCaml 4.12.0 when compiling empty archives; no .a file is generated. (ocaml/dune#3576, @dra27) - `$ dune utop` no longer tries to load optional libraries that are unavailable (ocaml/dune#3612, fixes ocaml/dune#3188, @anuragsoni) - Fix dune-build-info on 4.10.0+flambda (ocaml/dune#3599, @emillon, @jeremiedimino). - Allow multiple libraries with `inline_tests` to be defined in the same directory (ocaml/dune#3621, @rgrinberg) - Run exit hooks in jsoo separate compilation mode (ocaml/dune#3626, fixes ocaml/dune#3622, @rgrinberg) - Add (alias ...), (mode ...) fields to (copy_fields ...) stanza (ocaml/dune#3631, @nojb) - (copy_files ...) now supports copying files from outside the workspace using absolute file names (ocaml/dune#3639, @nojb) - Dune does not use `ocamlc` as an intermediary to call C compiler anymore. Configuration flags `ocamlc_cflags` and `ocamlc_cppflags` are always prepended to the compiler arguments. (ocaml/dune#3565, fixes ocaml/dune#3346, @voodoos) - Revert the build optimization in ocaml/dune#2268. This optimization slows down building individual executables when they're part of an `executables` stanza group (ocaml/dune#3644, @rgrinberg) - Use `{dev}` rather than `{pinned}` in the generated `.opam` file. (ocaml/dune#3647, @kit-ty-kate) - Insert correct extension name when editing `dune-project` files. Previously, dune would just insert the stanza name. (ocaml/dune#3649, fixes ocaml/dune#3624, @rgrinberg) - Fix crash when evaluating an `mdx` stanza that depends on unavailable packages. (ocaml/dune#3650, @craigfe) - Fix typo in `cache-check-probablity` field in dune config files. This field now requires 2.7 as it wasn't usable before this version. (ocaml/dune#3652, @edwintorok) - Add `"odoc" {with-doc}` to the dependencies in the generated `.opam` files. (ocaml/dune#3667, @kit-ty-kate) - Do not allow user actions to capture dune's stdin (ocaml/dune#3677, fixes ocaml/dune#3672, @rgrinberg) - `(subdir ...)` stanzas can now appear in dune files used via `(include ...)`. (ocaml/dune#3676, @nojb)

nojb force-pushed the copy_files_long_form branch from 881f055 to cab6244 Compare July 15, 2020 19:24

nojb commented Jul 15, 2020

View reviewed changes

rgrinberg approved these changes Jul 15, 2020

View reviewed changes

nojb force-pushed the copy_files_long_form branch from cab6244 to 37436a5 Compare July 15, 2020 19:56

nojb requested a review from emillon as a code owner July 15, 2020 19:56

nojb force-pushed the copy_files_long_form branch from 64f973c to 480d859 Compare July 16, 2020 08:46

nojb added 4 commits July 16, 2020 10:47

Add (alias ...), (mode ...) fields to (copy_files ...) stanza

89e5474

Signed-off-by: Nicolás Ojeda Bär <[email protected]>

CHANGES.md

a6b4079

Signed-off-by: Nicolás Ojeda Bär <[email protected]>

Update doc

43c0fe9

Signed-off-by: Nicolás Ojeda Bär <[email protected]>

Rename (glob ...) intso (files ...)

9332983

Signed-off-by: Nicolás Ojeda Bär <[email protected]>

nojb force-pushed the copy_files_long_form branch from 480d859 to 9332983 Compare July 16, 2020 08:48

nojb added 2 commits July 16, 2020 13:10

Better error message when in version < 2.7

5a856f2

Signed-off-by: Nicolás Ojeda Bär <[email protected]>

Update tests

e22a424

Signed-off-by: Nicolás Ojeda Bär <[email protected]>

Merge branch 'master' into copy_files_long_form

2d54c00

nojb merged commit f83aff7 into ocaml:master Jul 17, 2020

nojb deleted the copy_files_long_form branch July 17, 2020 04:45

voodoos mentioned this pull request Jul 21, 2020

Add a copy_dir stanza #3643

Closed

3 tasks

rgrinberg mentioned this pull request Aug 14, 2020

[new release] dune-build-info, dune, dune-configurator, dune-action-plugin, dune-private-libs and dune-glob (2.7.0) ocaml/opam-repository#17005

Merged

Conversation

nojb commented Jul 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nojb commented Jul 15, 2020

Uh oh!

nojb Jul 15, 2020

Choose a reason for hiding this comment

Uh oh!

emillon Jul 16, 2020

Choose a reason for hiding this comment

Uh oh!

nojb Jul 16, 2020

Choose a reason for hiding this comment

Uh oh!

rgrinberg left a comment

Choose a reason for hiding this comment

Uh oh!

rgrinberg Jul 15, 2020

Choose a reason for hiding this comment

Uh oh!

nojb Jul 15, 2020

Choose a reason for hiding this comment

Uh oh!

nojb commented Jul 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nojb commented Jul 15, 2020

Uh oh!

rgrinberg commented Jul 15, 2020

Uh oh!

ghost commented Jul 16, 2020

Uh oh!

nojb commented Jul 16, 2020

Uh oh!

nojb commented Jul 16, 2020

Uh oh!

ghost commented Jul 16, 2020

Uh oh!

rgrinberg commented Jul 16, 2020

Uh oh!

nojb commented Jul 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nojb commented Jul 15, 2020 •

edited

Loading

nojb commented Jul 15, 2020 •

edited

Loading