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

Warn when dune-project does not exist #5343

Merged
merged 3 commits into from
Feb 8, 2022
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
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ Unreleased
- Dune no longer automatically create or edit `dune-project` files
(#4239, fixes #4108, @jeremiedimino)

- Warn if `dune-project` is not found (fatal in release mode) (#5343, @emillon)

- Cleanup temporary files after running `$ dune exec`. (#4260, fixes #4243,
@rgrinberg)

Expand Down
26 changes: 22 additions & 4 deletions bin/common.ml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type t =
; workspace_config : Dune_rules.Workspace.Clflags.t
; cache_debug_flags : Dune_engine.Cache_debug_flags.t
; report_errors_config : Dune_engine.Report_errors_config.t
; require_dune_project_file : bool
}

let capture_outputs t = t.capture_outputs
Expand Down Expand Up @@ -204,6 +205,11 @@ let init ?log_file c =
Clflags.promote_install_files := c.promote_install_files;
Clflags.always_show_command_line := c.always_show_command_line;
Clflags.ignore_promoted_rules := c.ignore_promoted_rules;
Clflags.on_missing_dune_project_file :=
if c.require_dune_project_file then
Error
else
Warn;
Dune_util.Log.info
[ Pp.textf "Workspace root: %s"
(Path.to_absolute_filename Path.root |> String.maybe_quoted)
Expand Down Expand Up @@ -313,6 +319,7 @@ module Options_implied_by_dash_p = struct
; default_target : Arg.Dep.t
; always_show_command_line : bool
; promote_install_files : bool
; require_dune_project_file : bool
}

let docs = copts_sect
Expand Down Expand Up @@ -413,6 +420,12 @@ module Options_implied_by_dash_p = struct
last
& opt_all ~vopt:true bool [ false ]
& info [ "promote-install-files" ] ~docs ~doc)
and+ require_dune_project_file =
let doc = "Fail if a dune-project file is missing." in
Arg.(
last
& opt_all ~vopt:true bool [ false ]
& info [ "require-dune-project-file" ] ~docs ~doc)
in
{ root
; only_packages = No_restriction
Expand All @@ -422,6 +435,7 @@ module Options_implied_by_dash_p = struct
; default_target
; always_show_command_line
; promote_install_files
; require_dune_project_file
}

let dash_dash_release =
Expand All @@ -436,6 +450,7 @@ module Options_implied_by_dash_p = struct
; "release"
; "--always-show-command-line"
; "--promote-install-files"
; "--require-dune-project-file"
; "--default-target"
; "@install"
]
Expand All @@ -444,10 +459,11 @@ module Options_implied_by_dash_p = struct
"Put $(b,dune) into a reproducible $(i,release) mode. This is in \
fact a shorthand for $(b,--root . --ignore-promoted-rules \
--no-config --profile release --always-show-command-line \
--promote-install-files --default-target @install). You should \
use this option for release builds. For instance, you must use \
this option in your $(i,<package>.opam) files. Except if you \
already use $(b,-p), as $(b,-p) implies this option.")
--promote-install-files --default-target @install \
--require-dune-project-file). You should use this option for \
release builds. For instance, you must use this option in your \
$(i,<package>.opam) files. Except if you already use $(b,-p), as \
$(b,-p) implies this option.")

let options =
let+ t = options
Expand Down Expand Up @@ -872,6 +888,7 @@ let term ~default_root_is_cwd =
; default_target
; always_show_command_line
; promote_install_files
; require_dune_project_file
} =
Options_implied_by_dash_p.term
and+ x =
Expand Down Expand Up @@ -1042,6 +1059,7 @@ let term ~default_root_is_cwd =
}
; cache_debug_flags
; report_errors_config
; require_dune_project_file
}

let set_rpc t rpc = { t with rpc = Some rpc }
Expand Down
1 change: 1 addition & 0 deletions bin/init.ml
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ let term =
& info [ "pkg" ] ~docv ~doc)
in
let _config = Common.init common_term in
Dune_engine.Clflags.on_missing_dune_project_file := Dune_engine.Clflags.Ignore;
let open Component in
let context = Init_context.make path in
let common : Options.Common.t = { name; libraries; pps } in
Expand Down
7 changes: 7 additions & 0 deletions src/dune_engine/clflags.ml
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,10 @@ let always_show_command_line = ref false
let promote_install_files = ref false

let ignore_promoted_rules = ref false

type on_missing_dune_project_file =
| Error
| Warn
| Ignore

let on_missing_dune_project_file = ref Warn
8 changes: 8 additions & 0 deletions src/dune_engine/clflags.mli
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,11 @@ val promote_install_files : bool ref

(** Whether we are ignoring rules with [(mode promote)] *)
val ignore_promoted_rules : bool ref

type on_missing_dune_project_file =
| Error
| Warn
| Ignore

(** Desired behavior when dune project file is absent *)
val on_missing_dune_project_file : on_missing_dune_project_file ref
4 changes: 0 additions & 4 deletions src/dune_engine/dune_project.ml
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,6 @@ let default_dune_language_version =
let get_dune_lang () =
{ (Lang.get_exn "dune") with version = !default_dune_language_version }

type created_or_already_exist =
| Created
| Already_exist

module Extension = struct
type 'a t = 'a Univ_map.Key.t

Expand Down
4 changes: 0 additions & 4 deletions src/dune_engine/dune_project.mli
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,6 @@ val anonymous :
(** "dune-project" *)
val filename : string

type created_or_already_exist =
| Created
| Already_exist

(** Default language version to use for projects that don't have a
[dune-project] file. The default value is the latest version of the dune
language. *)
Expand Down
40 changes: 40 additions & 0 deletions src/dune_engine/source_tree.ml
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,44 @@ end = struct
(visited, init)
end

let ensure_dune_project_file_exists =
let memo =
let module Input = struct
type t = [ `Is_error of bool ] * Dune_project.t

let equal (a1, p1) (a2, p2) =
Poly.equal a1 a2 && Dune_project.equal p1 p2

let hash = Tuple.T2.hash Poly.hash Dune_project.hash

let to_dyn (`Is_error b, project) =
Dyn.(pair bool Dune_project.to_dyn) (b, project)
end in
Memo.create "ensure-dune-project-file-exists"
~input:(module Input)
(fun (`Is_error is_error, project) ->
let open Memo.Build.O in
let+ exists =
Dune_project.file project |> Path.source |> Fs_memo.file_exists
in
if not exists then
User_warning.emit ~is_error
~hints:
[ Pp.text
"generate the project file with: $ dune init project <name>"
]
[ Pp.text
"No dune-project file has been found. A default one is \
assumed but the project might break when dune is upgraded. \
Please create a dune-project file."
rgrinberg marked this conversation as resolved.
Show resolved Hide resolved
])
in
fun inp ->
match !Clflags.on_missing_dune_project_file with
| Ignore -> Memo.Build.return ()
| Warn -> Memo.exec memo (`Is_error false, inp)
| Error -> Memo.exec memo (`Is_error true, inp)

let dune_file ~(dir_status : Sub_dirs.Status.t) ~path ~files ~project =
let file_exists =
if dir_status = Data_only then
Expand Down Expand Up @@ -511,6 +549,8 @@ end = struct
| None, Some (path, _) -> Some path
in
Memo.Build.Option.map file ~f:(fun file ->
let open Memo.Build.O in
let* () = ensure_dune_project_file_exists project in
let file_exists = Option.is_some file_exists in
let from_parent = Option.map from_parent ~f:snd in
Dune_file.load file ~file_exists ~project ~from_parent)
Expand Down
4 changes: 4 additions & 0 deletions test/blackbox-tests/test-cases/coq/github3624.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ that the error message is good when the coq extension is not enabled.
> (name foo))
> EOF
$ dune build
Warning: No dune-project file has been found. A default one is assumed but
the project might break when dune is upgraded. Please create a dune-project
file.
Hint: generate the project file with: $ dune init project <name>
File "dune", line 1, characters 0-24:
1 | (coq.theory
2 | (name foo))
Expand Down
24 changes: 24 additions & 0 deletions test/blackbox-tests/test-cases/dune-init.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ Can init a public library
Can build the public library

$ (cd _test_lib_dir && touch test_lib.opam && dune build)
Warning: No dune-project file has been found. A default one is assumed but
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused: I thought the plan was for dune init do the right thing, i.e. create the dune-project file too. Why isn't it created here?

Copy link
Member

Choose a reason for hiding this comment

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

dune init creates the project once it is called with the project argument.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I see. I think we should suggest that and test that our suggestion works.

Copy link
Member

Choose a reason for hiding this comment

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

I added a suggestion. dune init project is already tested elsewhere, it seems overkill to extract a command out of a printed hint and run it (if that's what you meant)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I only meant that we should test that what we suggest works (not that we need to extract the suggestion, I agree that's an overkill).

Copy link
Member

Choose a reason for hiding this comment

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

We could also make the name argument to dune init project optional. Would make it easier for users to silence the warning.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed.

the project might break when dune is upgraded. Please create a dune-project
file.
Hint: generate the project file with: $ dune init project <name>
$ cat ./_test_lib_dir/dune
(library
(public_name test_lib)
Expand Down Expand Up @@ -68,10 +72,18 @@ Can init a public executable
Can build an executable

$ (cd _test_bin_dir && touch test_bin.opam && dune build)
Warning: No dune-project file has been found. A default one is assumed but
the project might break when dune is upgraded. Please create a dune-project
file.
Hint: generate the project file with: $ dune init project <name>

Can run the created executable

$ (cd _test_bin_dir && dune exec test_bin)
Warning: No dune-project file has been found. A default one is assumed but
the project might break when dune is upgraded. Please create a dune-project
file.
Hint: generate the project file with: $ dune init project <name>
Hello, World!

Clean up the executable tests
Expand Down Expand Up @@ -145,10 +157,18 @@ Can init a library and dependent executable in a combo project
Can build the combo project

$ (cd _test_lib_exe_dir && touch test_bin.opam && dune build)
Warning: No dune-project file has been found. A default one is assumed but
the project might break when dune is upgraded. Please create a dune-project
file.
Hint: generate the project file with: $ dune init project <name>

Can run the combo project

$ (cd _test_lib_exe_dir && dune exec test_bin)
Warning: No dune-project file has been found. A default one is assumed but
the project might break when dune is upgraded. Please create a dune-project
file.
Hint: generate the project file with: $ dune init project <name>
Hello, World!

Clean up the combo project
Expand All @@ -175,6 +195,10 @@ Can add multiple libraries in the same directory
Can build the multiple library project

$ (cd _test_lib && touch test_lib1.opam && dune build)
Warning: No dune-project file has been found. A default one is assumed but
the project might break when dune is upgraded. Please create a dune-project
file.
Hint: generate the project file with: $ dune init project <name>

Clan up the multiple library project

Expand Down
4 changes: 4 additions & 0 deletions test/blackbox-tests/test-cases/github1529.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ Reproduction case for #1529: using an extension when no dune-project
file is present.

$ dune build @install 2>&1 | sed "s/(lang dune .*)/(lang dune <version>)/" | sed "s/(using menhir .*)/(using menhir <version>)/"
Warning: No dune-project file has been found. A default one is assumed but
the project might break when dune is upgraded. Please create a dune-project
file.
Hint: generate the project file with: $ dune init project <name>
File "dune", line 1, characters 0-25:
1 | (menhir (modules parser))
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
1 change: 1 addition & 0 deletions test/blackbox-tests/test-cases/github4682.t/dune-project
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(lang dune 2.8)
3 changes: 3 additions & 0 deletions test/blackbox-tests/test-cases/github4684.t
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ build failure:

$ echo "module X = Root.Unix" > main.ml
$ touch main.opam
$ cat >dune-project <<EOF
> (lang dune 2.8)
> EOF
$ cat >dune <<EOF
> (library
> (name main)
Expand Down
3 changes: 3 additions & 0 deletions test/blackbox-tests/test-cases/github4936.t
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
C stubs and the tests stanza

$ touch e.ml stubs.c
$ cat > dune-project << EOF
> (lang dune 2.0)
> EOF
$ cat > dune << EOF
> (test
> (name e)
Expand Down
44 changes: 44 additions & 0 deletions test/blackbox-tests/test-cases/no-dune-project.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
When no dune-project file is present, a warning is printed.

$ touch dune
$ dune build
Warning: No dune-project file has been found. A default one is assumed but
the project might break when dune is upgraded. Please create a dune-project
file.
Hint: generate the project file with: $ dune init project <name>

In release mode, this is fatal.

$ dune build --release
Error: No dune-project file has been found. A default one is assumed but the
project might break when dune is upgraded. Please create a dune-project file.
Hint: generate the project file with: $ dune init project <name>
[1]

This corresponds to a flag:

$ dune build --require-dune-project-file
Error: No dune-project file has been found. A default one is assumed but the
project might break when dune is upgraded. Please create a dune-project file.
Hint: generate the project file with: $ dune init project <name>
[1]

Test case: warning should be emitted

$ mkdir nested-case && cd nested-case
$ mkdir a && touch a/dune
$ dune build
Warning: No dune-project file has been found. A default one is assumed but
the project might break when dune is upgraded. Please create a dune-project
file.
Hint: generate the project file with: $ dune init project <name>
$ cd ..

Test case: warning should not be emitted

$ mkdir another-case && cd another-case
$ mkdir a && touch a/dune
$ echo "(lang dune 3.0)" > a/dune-project
$ cp -R a b
$ dune build
$ cd ..
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
Depending on a promoted file works.

$ cat > dune-project <<EOF
> (lang dune 2.0)
> EOF

$ cat > dune <<EOF
> (rule
> (mode promote)
Expand Down