Skip to content

Fix error when copy_files source does not exist#1120

Merged
emillon merged 1 commit intomasterfrom
fix-1099
Aug 13, 2018
Merged

Fix error when copy_files source does not exist#1120
emillon merged 1 commit intomasterfrom
fix-1099

Conversation

@emillon
Copy link
Copy Markdown
Collaborator

@emillon emillon commented Aug 10, 2018

See #1099.

| Error (_pos, msg) ->
Loc.fail (String_with_vars.loc def.glob) "invalid glob: %s" msg
in
if not (Path.exists src_in_src) then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use File_tree from the super context here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that works. Does it add caching?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. It will also work when the path that you're checking is a target of another rule. But I'm not sure if that's useful here.

Loc.fail (String_with_vars.loc def.glob) "invalid glob: %s" msg
in
let file_tree = Super_context.file_tree sctx in
if not (File_tree.exists file_tree src_in_src) then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when src_in_src is a valid file? We might get a confusing error in that case. Perhaps it's worth adding a test case for this too.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. I exposed and used dir_exists instead.

Closes #1099

Signed-off-by: Etienne Millon <me@emillon.org>
@emillon emillon merged commit 1cd5050 into master Aug 13, 2018
@emillon emillon deleted the fix-1099 branch August 13, 2018 12:51
@ghost
Copy link
Copy Markdown

ghost commented Aug 13, 2018

BTW, we could also complain when a glob matches nothing. This is in general an error.

rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Sep 14, 2018
CHANGES:

- Ignore stderr output when trying to find out the number of jobs
  available (ocaml/dune#1118, fix ocaml/dune#1116, @diml)

- Fix error message when the source directory of `copy_files` does not exist.
  (ocaml/dune#1120, fix ocaml/dune#1099, @emillon)

- Highlight error locations in error messages (ocaml/dune#1121, @emillon)

- Display actual stanza when package is ambiguous (ocaml/dune#1126, fix ocaml/dune#1123, @emillon)

- Add `dune unstable-fmt` to format `dune` files. The interface and syntax are
  still subject to change, so use with caution. (ocaml/dune#1130, fix ocaml/dune#940, @emillon)

- Improve error message for `dune utop` without a library name (ocaml/dune#1154, fix
  ocaml/dune#1149, @emillon)

- Fix parsing `ocamllex` stanza in jbuild files (ocaml/dune#1150, @rgrinberg)

- Highlight multi-line errors (ocaml/dune#1131, @anuragsoni)

- Do no try to generate shared libraries when this is not supported by
  the OS (ocaml/dune#1165, fix ocaml/dune#1051, @diml)

- Fix `Flags.write_{sexp,lines}` in configurator by avoiding the use of
  `Stdune.Path` (ocaml/dune#1175, fix ocaml/dune#1161, @rgrinberg)

- Add support for `findlib.dynload`: when linking an executable using
  `findlib.dynload`, automatically record linked in libraries and
  findlib predicates (ocaml/dune#1172, @bobot)

- Add support for promoting a selected list of files (ocaml/dune#1192, @diml)

- Add an emacs mode providing helpers to promote correction files
  (ocaml/dune#1192, @diml)

- Improve message suggesting to remove parentheses (ocaml/dune#1196, fix ocaml/dune#1173, @emillon)

- Add `(wrapped (transition "..message.."))` as an option that will generate
  wrapped modules but keep unwrapped modules with a deprecation message to
  preserve compatibility. (ocaml/dune#1188, fix ocaml/dune#985, @rgrinberg)

- Fix the flags passed to the ppx rewriter when using `staged_pps` (ocaml/dune#1218, @diml)

- Add `(env var)` to add a dependency to an environment variable.
  (ocaml/dune#1186, @emillon)

- Add a simple version of a polling mode: `dune build -w` keeps
  running and restarts the build when something change on the
  filesystem (ocaml/dune#1140, @kodek16)

- Cleanup the way we detect the library search path. We no longer call
  `opam config var lib` in the default build context (ocaml/dune#1226, @diml)

- Make test stanzas honor the -p flag. (ocaml/dune#1236, fix ocaml/dune#1231, @emillon)

- Test stanzas take an optional (action) field to customize how they run (ocaml/dune#1248,
  ocaml/dune#1195, @emillon)

- Add support for private modules via the `private_modules` field (ocaml/dune#1241, fix
  ocaml/dune#427, @rgrinberg)

- Add support for passing arguments to the OCaml compiler via a
  response file when the list of arguments is too long (ocaml/dune#1256, @diml)

- Do not print diffs by default when running inside dune (ocaml/dune#1260, @diml)

- Interpret `$ dune build dir` as building the default alias in `dir`. (ocaml/dune#1259,
  @rgrinberg)

- Make the `dynlink` library available without findlib installed (ocaml/dune#1270, fix
  ocaml/dune#1264, @rgrinberg)
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