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

[2.9] fix sites installation with opam #4774

Merged
merged 5 commits into from
Jun 29, 2021
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
10 changes: 10 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@ unreleased - 2.9 branch
- Fix a bug where instrumentation flags would be added even if the
instrumentatation was disabled (@nojb, #4770)

- Fix #4682: option `-p` takes now precedence on environement variable
`DUNE_PROFILE` (#4730, #4774, @bobot, reported by @dra27 #4632)

- Fix installation with opam of package with dune sites. The `.install` file is
now produced by a local `dune install` during the build phase (#4730, #4645,
@bobot, reported by @kit-ty-kate #4198)

- Fix multiple issues in the sites feature (#4730, #4645 @bobot, reported by @Lelio-Brun
#4219, by @Kakadu #4325, by @toots #4415)

2.8.5 (28/03/2021)
------------------

Expand Down
108 changes: 45 additions & 63 deletions bin/common.ml
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,10 @@ module Options_implied_by_dash_p = struct
let doc =
"Promote the generated <package>.install files to the source tree"
in
Arg.(value & flag & info [ "promote-install-files" ] ~docs ~doc)
Arg.(
last
& opt_all ~vopt:true bool [ false ]
& info [ "promote-install-files" ] ~docs ~doc)
in
{ root
; only_packages = None
Expand All @@ -339,36 +342,34 @@ module Options_implied_by_dash_p = struct
; promote_install_files
}

let release_options =
{ root = Some "."
; only_packages = None
; ignore_promoted_rules = true
; config_file = No_config
; profile = Some Profile.Release
; default_target =
Arg.Dep.alias_rec ~dir:Path.Local.root Dune_engine.Alias.Name.install
; always_show_command_line = true
; promote_install_files = true
}

let dash_dash_release =
let+ (_ : bool) =
Arg.(
value & flag
& info [ "release" ] ~docs ~docv:"PACKAGES"
~doc:
"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.")
in
release_options
Arg.(
value
& alias
[ "--root"
; "."
; "--ignore-promoted-rules"
; "--no-config"
; "--profile"
; "release"
; "--always-show-command-line"
; "--promote-install-files"
; "--default-target"
; "@install"
]
& info [ "release" ] ~docs ~docv:"PACKAGES"
~doc:
"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.")

let options =
let+ t = one_of options dash_dash_release
let+ t = options
and+ _ = dash_dash_release
and+ only_packages =
let+ names =
Arg.(
Expand All @@ -390,55 +391,36 @@ module Options_implied_by_dash_p = struct
{ t with only_packages }

let dash_p =
let+ pkgs, args =
Term.with_used_args
Arg.(
value
& opt (some packages) None
& info
[ "p"; "for-release-of-packages" ]
~docs ~docv:"PACKAGES"
~doc:
"Shorthand for $(b,--release --only-packages PACKAGE). You \
must use this option in your $(i,<package>.opam) files, in \
order to build only what's necessary when your project \
contains multiple packages as well as getting reproducible \
builds.")
in
{ release_options with
only_packages =
Option.map pkgs ~f:(fun names ->
Only_packages.create ~names ~command_line_option:(List.hd args))
}
Term.with_used_args
Arg.(
value
& alias_opt (fun s -> [ "--release"; "--only-packages"; s ])
& info
[ "p"; "for-release-of-packages" ]
~docs ~docv:"PACKAGES"
~doc:
"Shorthand for $(b,--release --only-packages PACKAGE). You must \
use this option in your $(i,<package>.opam) files, in order to \
build only what's necessary when your project contains multiple \
packages as well as getting reproducible builds.")

let term =
let+ t = one_of options dash_p
let+ t = options
and+ _ = dash_p
and+ profile =
let doc =
"Build profile. $(b,dev) if unspecified or $(b,release) if -p is set."
in
Arg.(
value
& opt (some profile) None
last
& opt_all (some profile) [ None ]
& info [ "profile" ] ~docs
~env:(Arg.env_var ~doc "DUNE_PROFILE")
~doc:
(Printf.sprintf
"Select the build profile, for instance $(b,dev) or \
$(b,release). The default is $(b,%s)."
(Profile.to_string Dune_rules.Profile.default)))
and+ no_promote_install_files =
let doc =
"Do not promote the generated <package>.install files to the source \
tree"
in
Arg.(value & flag & info [ "no-promote-install-files" ] ~docs ~doc)
in
let t =
{ t with
promote_install_files =
(not no_promote_install_files) && t.promote_install_files
}
in
match profile with
| None -> t
Expand Down
5 changes: 3 additions & 2 deletions otherlibs/site/test/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,16 @@ Test with an opam like installation
name
"-j"
jobs
"--no-promote-install-files"
"--promote-install-files"
"false"
"@install"
"@runtest" {with-test}
"@doc" {with-doc}
]
["dune" "install" "-p" name "--create-install-files" name]
]

$ dune build -p a --no-promote-install-files @install
$ dune build -p a --promote-install-files "false" @install

$ test -e a/a.install
[1]
Expand Down
2 changes: 1 addition & 1 deletion src/dune_rules/opam_create.ml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ let default_build_command =
{|
[
[ "dune" "subst" "--root" "." ] {dev}
[ "dune" "build" "-p" name "-j" jobs "--no-promote-install-files"
[ "dune" "build" "-p" name "-j" jobs "--promote-install-files" "false"
"@install"
"@runtest" {with-test}
"@doc" {with-doc}
Expand Down
19 changes: 19 additions & 0 deletions test/blackbox-tests/test-cases/cmdline/profile.t/run.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Bug #4632

$ dune build -p
dune: option `-p' needs an argument
Usage: dune build [OPTION]... [TARGET]...
Try `dune build --help' or `dune --help' for more information.
[1]

$ dune build --root . --verbose 2>&1 | grep "; profile"
; profile = Dyn

$ dune build -p foo --verbose 2>&1 | grep "; profile"
; profile = Release

$ DUNE_PROFILE="bar" dune build --root . --verbose 2>&1 | grep "; profile"
; profile = User_defined "bar"

$ DUNE_PROFILE="bar" dune build -p foo --verbose 2>&1 | grep "; profile"
; profile = Release
4 changes: 2 additions & 2 deletions test/blackbox-tests/test-cases/github3530.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ When an empty string is passed to `-p`, we get a nice error message.

$ echo '(lang dune 2.0)' > dune-project
$ dune build -p ''
dune: option `-p': Invalid package name: ""
dune: option `--only-packages': Invalid package name: ""
Usage: dune build [OPTION]... [TARGET]...
Try `dune build --help' or `dune --help' for more information.
[1]

This can happen in a list as well:

$ dune build -p 'a,b,'
dune: option `-p': Invalid package name: ""
dune: option `--only-packages': Invalid package name: ""
Usage: dune build [OPTION]... [TARGET]...
Try `dune build --help' or `dune --help' for more information.
[1]
8 changes: 4 additions & 4 deletions test/blackbox-tests/test-cases/misc.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ Test that incompatible options are properly reported
[1]

$ dune build -p toto --root .
dune: Cannot use --root and -p simultaneously
dune: option `--root' cannot be repeated
Usage: dune build [OPTION]... [TARGET]...
Try `dune build --help' or `dune --help' for more information.
[1]

$ dune build --for-release-of-packages toto --root .
dune: Cannot use --root and --for-release-of-packages simultaneously
dune: option `--root' cannot be repeated
Usage: dune build [OPTION]... [TARGET]...
Try `dune build --help' or `dune --help' for more information.
[1]
Expand All @@ -30,13 +30,13 @@ Test that incompatible options are properly reported
[1]

$ dune build -p toto --release
dune: Cannot use --release and -p simultaneously
dune: option `--root' cannot be repeated
Usage: dune build [OPTION]... [TARGET]...
Try `dune build --help' or `dune --help' for more information.
[1]

$ dune build --release --root .
dune: Cannot use --root and --release simultaneously
dune: option `--root' cannot be repeated
Usage: dune build [OPTION]... [TARGET]...
Try `dune build --help' or `dune --help' for more information.
[1]
Expand Down
22 changes: 22 additions & 0 deletions vendor/.ocamlformat-ignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
cmdliner/src/cmdliner_arg.ml
cmdliner/src/cmdliner_arg.mli
cmdliner/src/cmdliner_base.ml
cmdliner/src/cmdliner_base.mli
cmdliner/src/cmdliner_cline.ml
cmdliner/src/cmdliner_cline.mli
cmdliner/src/cmdliner_docgen.ml
cmdliner/src/cmdliner_docgen.mli
cmdliner/src/cmdliner_info.ml
cmdliner/src/cmdliner_info.mli
cmdliner/src/cmdliner_manpage.ml
cmdliner/src/cmdliner_manpage.mli
cmdliner/src/cmdliner.ml
cmdliner/src/cmdliner.mli
cmdliner/src/cmdliner_msg.ml
cmdliner/src/cmdliner_msg.mli
cmdliner/src/cmdliner_suggest.ml
cmdliner/src/cmdliner_suggest.mli
cmdliner/src/cmdliner_term.ml
cmdliner/src/cmdliner_term.mli
cmdliner/src/cmdliner_trie.ml
cmdliner/src/cmdliner_trie.mli
10 changes: 10 additions & 0 deletions vendor/cmdliner/src/cmdliner.mli
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,16 @@ module Arg : sig
{b Note.} Environment variable lookup is unsupported for
for these arguments. *)


val alias : string list -> info -> bool t
(** [alias l i] is a [flag i] except the arguments [l] are also parsed as
if they appeared in place of the option. *)

val alias_opt : (string -> string list) -> info -> bool t
(** [alias l i] is a [flag i] except the arguments [l arg] are also parsed as
if they appeared in place of the option. [arg] is the possible argument
given on the command line *)

val opt : ?vopt:'a -> 'a conv -> 'a -> info -> 'a t
(** [opt vopt c v i] is an ['a] argument defined by the value of
an optional argument that may appear {e at most} once on the command
Expand Down
37 changes: 37 additions & 0 deletions vendor/cmdliner/src/cmdliner_arg.ml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,35 @@ let list_to_args f l =
let add acc v = Cmdliner_info.Args.add (f v) acc in
List.fold_left add Cmdliner_info.Args.empty l

let alias_opt aliases a =
let a = Cmdliner_info.arg_make_opt ~absent:Err ~kind:Opt a in
let aliases = (fun f -> function
| None -> Error (Cmdliner_msg.err_opt_value_missing f)
| Some o -> Ok (aliases o)) in
let a = Cmdliner_info.arg_aliases ~aliases a in
if Cmdliner_info.arg_is_pos a then invalid_arg err_not_opt else
let convert ei cl = match Cmdliner_cline.opt_arg cl a with
| [] -> try_env ei a Cmdliner_base.env_bool_parse ~absent:false
| [_, _, None] -> Ok true
| [_, f, Some v] -> Ok true
| (_, f, _) :: (_ ,g, _) :: _ -> err (Cmdliner_msg.err_opt_repeated f g)
in
arg_to_args a, convert

let alias aliases a =
let aliases = (fun f -> function
| Some v -> Error (Cmdliner_msg.err_flag_value f v)
| None -> Ok aliases) in
let a = Cmdliner_info.arg_aliases ~aliases a in
if Cmdliner_info.arg_is_pos a then invalid_arg err_not_opt else
let convert ei cl = match Cmdliner_cline.opt_arg cl a with
| [] -> try_env ei a Cmdliner_base.env_bool_parse ~absent:false
| [_, _, None] -> Ok true
| [_, f, Some v] -> err (Cmdliner_msg.err_flag_value f v)
| (_, f, _) :: (_ ,g, _) :: _ -> err (Cmdliner_msg.err_opt_repeated f g)
in
arg_to_args a, convert

let flag a =
if Cmdliner_info.arg_is_pos a then invalid_arg err_not_opt else
let convert ei cl = match Cmdliner_cline.opt_arg cl a with
Expand Down Expand Up @@ -290,6 +319,14 @@ let last (args, convert) =
in
args, convert

let last_or_none (args, convert) =
let convert ei cl = match convert ei cl with
| Ok [] -> Ok None
| Ok l -> Ok (Some (List.hd (List.rev l)))
| Error _ as e -> e
in
args, convert

(* Predefined arguments *)

let man_fmts =
Expand Down
2 changes: 2 additions & 0 deletions vendor/cmdliner/src/cmdliner_arg.mli
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ val flag : info -> bool t
val flag_all : info -> bool list t
val vflag : 'a -> ('a * info) list -> 'a t
val vflag_all : 'a list -> ('a * info) list -> 'a list t
val alias : string list -> info -> bool t
val alias_opt : (string -> string list) -> info -> bool t
val opt : ?vopt:'a -> 'a converter -> 'a -> info -> 'a t
val opt_all : ?vopt:'a -> 'a converter -> 'a list -> info -> 'a list t

Expand Down
5 changes: 5 additions & 0 deletions vendor/cmdliner/src/cmdliner_cline.ml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ let parse_opt_args ~peek_opts optidx cl args =
| v :: rest -> if is_opt v then None, args else Some v, rest
in
let arg = O ((k, name, value) :: opt_arg cl a) in
let errs,args =
match Cmdliner_info.arg_alias a name value with
| Ok l -> errs,l@args
| Error err -> err::errs,args
in
loop errs (k + 1) (Amap.add a arg cl) pargs args
| `Not_found when peek_opts -> loop errs (k + 1) cl pargs args
| `Not_found ->
Expand Down
Loading