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

[RFC] Lift subdirectory restriction on copy_files stanza? #1323

Merged
5 commits merged into from
Sep 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
- Fix compilation of the module generated for `findlib.dynload`
(#1317, fix #1310, @diml)

- Lift restriction on `copy_files` and `copy_files#` stanzas that files to be
copied should be in a subdirectory of the current directory.
(#1323, fix #911, @nojb)

1.2.1 (17/09/2018)
------------------

Expand Down
11 changes: 7 additions & 4 deletions src/dune_file.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1808,6 +1808,7 @@ end
module Copy_files = struct
type t = { add_line_directive : bool
; glob : String_with_vars.t
; syntax_version : Syntax.Version.t
}

let dparse = String_with_vars.dparse
Expand Down Expand Up @@ -1894,11 +1895,13 @@ module Stanzas = struct
(let%map x = Alias_conf.dparse in
[Alias x])
; "copy_files",
(let%map glob = Copy_files.dparse in
[Copy_files {add_line_directive = false; glob}])
(let%map glob = Copy_files.dparse
and syntax_version = Syntax.get_exn Stanza.syntax in
[Copy_files {add_line_directive = false; glob; syntax_version}])
; "copy_files#",
(let%map glob = Copy_files.dparse in
[Copy_files {add_line_directive = true; glob}])
(let%map glob = Copy_files.dparse
and syntax_version = Syntax.get_exn Stanza.syntax in
[Copy_files {add_line_directive = true; glob; syntax_version}])
; "include",
(let%map loc = loc
and fn = relative_file in
Expand Down
1 change: 1 addition & 0 deletions src/dune_file.mli
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ module Copy_files : sig
type t =
{ add_line_directive : bool
; glob : String_with_vars.t
; syntax_version : Syntax.Version.t
}
end

Expand Down
6 changes: 1 addition & 5 deletions src/simple_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,7 @@ let copy_files sctx ~dir ~scope ~src_dir (def: Copy_files.t) =
let src_glob = SC.expand_vars_string sctx ~dir def.glob ~scope in
Path.relative src_dir src_glob ~error_loc:loc
in
(* The following condition is required for merlin to work.
Additionally, the order in which the rules are evaluated only
ensures that [sources_and_targets_known_so_far] returns the
right answer for sub-directories only. *)
if not (Path.is_descendant glob_in_src ~of_:src_dir) then
if def.syntax_version < (1, 3) && not (Path.is_descendant glob_in_src ~of_:src_dir) then
Copy link
Member

Choose a reason for hiding this comment

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

Just a note that we usually wrap our code to 80 chars. I'm not sure if it's over the limit here, but something to keep in mind.

Errors.fail loc "%s is not a sub-directory of %s"
(Path.to_string_maybe_quoted glob_in_src) (Path.to_string_maybe_quoted src_dir);
let glob = Path.basename glob_in_src in
Expand Down
12 changes: 9 additions & 3 deletions test/blackbox-tests/test-cases/copy_files/run.t
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
$ dune build test.exe .merlin --display short --debug-dependency-path
$ dune build --root test1 test.exe .merlin --display short --debug-dependency-path
Entering directory 'test1'
ocamllex lexers/lexer1.ml
ocamldep .test.eobjs/lexer1.ml.d
ocamldep .test.eobjs/test.ml.d
Expand All @@ -12,7 +13,12 @@
ocamlopt .test.eobjs/lexer1.{cmx,o}
ocamlc .test.eobjs/test.{cmi,cmo,cmt}
ocamlopt .test.eobjs/test.{cmx,o}
ocamlopt test.exe
$ dune build @bar-source --display short
ocamlopt test
$ dune build --root test1 @bar-source --display short
Entering directory 'test1'
#line 1 "include/bar.h"
int foo () {return 42;}
$ dune build --root test2 @foo/cat
Entering directory 'test2'
# 1 "dummy.txt"
hello
1 change: 1 addition & 0 deletions test/blackbox-tests/test-cases/copy_files/test2/dune
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(rule (with-stdout-to dummy.txt (echo "hello")))
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(lang dune 1.3)
5 changes: 5 additions & 0 deletions test/blackbox-tests/test-cases/copy_files/test2/foo/dune
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
(copy_files# ../dummy.txt)

(alias
(name cat)
(action (cat %{dep:dummy.txt})))