Fix sites installation with opam #4730

bobot · 2021-06-14T08:08:34Z

Cherrypick from 2.9 (wrong way sorry) #4645

ghost · 2021-06-14T09:33:02Z

We already have --promote-install-files. Can we have --promote-install-files=no instead of --no-promote-install-files? We already have a lot of options, so it seems better to augment existing ones rather than add new similar ones.

FTR, we patched cmdliner in ~~#4718~~ #4678 to support "optional parameters" properly. i.e. you can change --promote-install-files to take an optional parameter, and in this case you have to write --promote-install-files=<value> if you want to pass a parameter, so that there is no ambiguity.

bobot · 2021-06-14T11:01:52Z

Can we have --promote-install-files=no instead of --no-promote-install-files?

Good idea, I will do it. @ejgallego I will have an additional little commit for 2.9. Is it okay?

bobot · 2021-06-14T12:32:00Z

In fact, now that I remember, I did it this way because currently we can't supersede -p with --promote-install-files (there can be only one_of them). We talked about strengthening this code but not for 2.9. In any case allowing every boolean option to be overridden using --no- is useful and common (e.g wget). We can have that as a general rule (and automatic for every flag).

ejgallego · 2021-06-14T12:34:19Z

Can we have --promote-install-files=no instead of --no-promote-install-files?

Good idea, I will do it. @ejgallego I will have an additional little commit for 2.9. Is it okay?

No problem, thanks!

ghost · 2021-06-14T14:32:31Z

Hmm, removing command line options is difficult since the cli is not versioned. We shouldn't introduce an option that we know we are going to remove.

In any case allowing every boolean option to be overridden using --no- is useful and common (e.g wget). We can have that as a general rule (and automatic for every flag).

Optional parameters seems better to me. We started using them for other options such as --watch and it is working pretty well. I don't understand why it wouldn't work here. Plus optional parameters are more versatile since they allows more states than just yes or no.

bobot · 2021-06-15T15:22:23Z

To do that I had to fix our alias option problem. So 72d774e handles correctly alias options. So it modifies Cmdliner (is it okay to modify our version before the modification is accepted in Cmdliner? @rgrinberg) . It also fixes #4632 . It is a little bigger than what I would have wanted for late commits but it is a good improvement.

ghost · 2021-06-16T14:48:58Z

is it okay to modify our version before the modification is accepted in Cmdliner?

Yes, we have already cross that bridge. However, we have https://github.com/ocaml-dune/cmdliner. @rgrinberg, have you been pushing patches to the ocaml-dune/cmdliner fork and then updating the copy in vendor?

ejgallego · 2021-06-18T17:41:34Z

I'm adding the 2.9 milestone because I understand bits of this will have to be backported too.

rgrinberg · 2021-06-21T20:20:16Z

have you been pushing patches to the ocaml-dune/cmdliner fork and then updating the copy in vendor?

Yes. Patches to cmdliner should be pushed there and then updated in this repo.

ghost · 2021-06-22T08:59:13Z

Ack. I pushed the small patch added in #4678 to github.com/ocaml-dune/cmdliner (/cc @aalekseyev).

bobot · 2021-06-24T13:51:47Z

So I'm going to merge it. Thanks @jeremiedimino for the cherry-pick.

Signed-off-by: François Bobot <[email protected]>

Fixes ocaml#4415 Signed-off-by: François Bobot <[email protected]>

instead of Path.Build.t Signed-off-by: François Bobot <[email protected]>

Signed-off-by: François Bobot <[email protected]>

- Substitute the files in an intermediate directory if needed - Backward compatible Fixes ocaml#4198 ocaml#4212 Signed-off-by: François Bobot <[email protected]>

Signed-off-by: François Bobot <[email protected]>

Fixes ocaml#4325 Signed-off-by: François Bobot <[email protected]>

Signed-off-by: François Bobot <[email protected]>

a name matching "$ocaml_keyword_*" is converted into name^"_" Fixes ocaml#4219 Signed-off-by: François Bobot <[email protected]>

So that opening and closing be evaluated directly Signed-off-by: François Bobot <[email protected]>

Signed-off-by: François Bobot <[email protected]>

options that expand to others. Uses it for `--release` and `-p`. Fixes ocaml#4682 , the options are really used Signed-off-by: François Bobot <[email protected]>

Signed-off-by: François Bobot <[email protected]>

@emillon

…ator, dune-private-libs, dune and dune-build-info (2.9.0) CHANGES: - Add `(enabled_if ...)` to `(mdx ...)` (ocaml/dune#4434, @emillon) - Add support for instrumentation dependencies (ocaml/dune#4210, fixes ocaml/dune#3983, @nojb) - Add the possibility to use `locks` with the cram tests stanza (ocaml/dune#4480, @voodoos) - Allow to set up merlin in a variant of the default context (ocaml/dune#4145, @TheLortex, @voodoos) - Add `(package ...)` to `(mdx ...)` (ocaml/dune#4691, fixes ocaml/dune#3756, @emillon) - Handle renaming of `coq.kernel` library to `coq-core.kernel` in Coq 8.14 (ocaml/dune#4713, @proux01) - Fix generation of merlin configuration when using `(include_subdirs unqualified)` on Windows (ocaml/dune#4745, @nojb) - Fix bug for the install of Coq native files when using `(include_subdirs qualified)` (ocaml/dune#4753, @ejgallego) - Allow users to specify install target directories for `doc` and `etc` sections. We add new options `--docdir` and `--etcdir` to both Dune's configure and `dune install` command. (ocaml/dune#4744, fixes ocaml/dune#4723, @ejgallego, thanks to @JasonGross for reporting this issue) - Fix issue where Dune would ignore `(env ... (coq (flags ...)))` declarations appearing in `dune` files (ocaml/dune#4749, fixes ocaml/dune#4566, @ejgallego @rgrinberg) - Disable some warnings on Coq 8.14 and `(lang coq (>= 0.3))` due to the rework of the Coq "native" compilation system (ocaml/dune#4760, @ejgallego) - Fix a bug where instrumentation flags would be added even if the instrumentatation was disabled (@nojb, ocaml/dune#4770) - Fix ocaml/dune#4682: option `-p` takes now precedence on environement variable `DUNE_PROFILE` (ocaml/dune#4730, ocaml/dune#4774, @bobot, reported by @dra27 ocaml/dune#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 (ocaml/dune#4730, ocaml/dune#4645, @bobot, reported by @kit-ty-kate ocaml/dune#4198) - Fix multiple issues in the sites feature (ocaml/dune#4730, ocaml/dune#4645 @bobot, reported by @Lelio-Brun ocaml/dune#4219, by @Kakadu ocaml/dune#4325, by @toots ocaml/dune#4415)

@emillon

…ator, dune-private-libs, dune and dune-build-info (2.9.0) CHANGES: - Add `(enabled_if ...)` to `(mdx ...)` (ocaml/dune#4434, @emillon) - Add support for instrumentation dependencies (ocaml/dune#4210, fixes ocaml/dune#3983, @nojb) - Add the possibility to use `locks` with the cram tests stanza (ocaml/dune#4480, @voodoos) - Allow to set up merlin in a variant of the default context (ocaml/dune#4145, @TheLortex, @voodoos) - Add `(package ...)` to `(mdx ...)` (ocaml/dune#4691, fixes ocaml/dune#3756, @emillon) - Handle renaming of `coq.kernel` library to `coq-core.kernel` in Coq 8.14 (ocaml/dune#4713, @proux01) - Fix generation of merlin configuration when using `(include_subdirs unqualified)` on Windows (ocaml/dune#4745, @nojb) - Fix bug for the install of Coq native files when using `(include_subdirs qualified)` (ocaml/dune#4753, @ejgallego) - Allow users to specify install target directories for `doc` and `etc` sections. We add new options `--docdir` and `--etcdir` to both Dune's configure and `dune install` command. (ocaml/dune#4744, fixes ocaml/dune#4723, @ejgallego, thanks to @JasonGross for reporting this issue) - Fix issue where Dune would ignore `(env ... (coq (flags ...)))` declarations appearing in `dune` files (ocaml/dune#4749, fixes ocaml/dune#4566, @ejgallego @rgrinberg) - Disable some warnings on Coq 8.14 and `(lang coq (>= 0.3))` due to the rework of the Coq "native" compilation system (ocaml/dune#4760, @ejgallego) - Fix a bug where instrumentation flags would be added even if the instrumentatation was disabled (@nojb, ocaml/dune#4770) - Fix ocaml/dune#4682: option `-p` takes now precedence on environement variable `DUNE_PROFILE` (ocaml/dune#4730, ocaml/dune#4774, @bobot, reported by @dra27 ocaml/dune#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 (ocaml/dune#4730, ocaml/dune#4645, @bobot, reported by @kit-ty-kate ocaml/dune#4198) - Fix multiple issues in the sites feature (ocaml/dune#4730, ocaml/dune#4645 @bobot, reported by @Lelio-Brun ocaml/dune#4219, by @Kakadu ocaml/dune#4325, by @toots ocaml/dune#4415)

Introduced in Dune 2.9, Opam files are generated with this line: ``` build: [ ["dune" "subst" "--root" "."] {dev} ``` However, `dune subst` doesn't have the `--root` option, which cause the Opam installation of packages generated by Dune with `(lang 2.9)` to fail. Revert to the previous line. This was added in commit 88d8b0e in PR ocaml#4730. Signed-off-by: Antonin Décimo <[email protected]>

Introduced in Dune 2.9, Opam files are generated with this line: ``` build: [ ["dune" "subst" "--root" "."] {dev} ``` However, `dune subst` doesn't have the `--root` option, which cause the Opam installation of packages generated by Dune with `(lang 2.9)` to fail. Revert to the previous line. This was added in commit 88d8b0e in PR #4730. Signed-off-by: Antonin Décimo <[email protected]>

Introduced in Dune 2.9, Opam files are generated with this line: ``` build: [ ["dune" "subst" "--root" "."] {dev} ``` However, `dune subst` doesn't have the `--root` option, which cause the Opam installation of packages generated by Dune with `(lang 2.9)` to fail. Revert to the previous line. This was added in commit 88d8b0e in PR ocaml#4730. Signed-off-by: Antonin Décimo <[email protected]>

bobot force-pushed the fix_sites_installation_with_opam branch from c372233 to 1ebf02e Compare June 14, 2021 09:20

bobot force-pushed the fix_sites_installation_with_opam branch 3 times, most recently from e2331e8 to f0486ae Compare June 15, 2021 15:39

bobot force-pushed the fix_sites_installation_with_opam branch from 37da607 to 4c69713 Compare June 17, 2021 08:44

ejgallego added this to the 2.9 milestone Jun 18, 2021

ejgallego mentioned this pull request Jun 24, 2021

Dune sites doesn't work when installed by opam #4212

Closed

bobot added 10 commits June 24, 2021 15:57

[Install/Uninstall] Use an enum instead of a string

9328235

Signed-off-by: François Bobot <[email protected]>

[Site] generate_site_module fails with -p

1582055

Signed-off-by: François Bobot <[email protected]>

[Sites] delay the computation of content of generate_sites_module

58ef72b

Fixes ocaml#4415 Signed-off-by: François Bobot <[email protected]>

[Install.Entry] Generalize the functions to Path.t

2205d66

instead of Path.Build.t Signed-off-by: François Bobot <[email protected]>

[Artifact_substitution] Allow to test the presence of a placeholder

1bf1b36

Signed-off-by: François Bobot <[email protected]>

[Sites] Fix installation through opam

88d8b0e

- Substitute the files in an intermediate directory if needed - Backward compatible Fixes ocaml#4198 ocaml#4212 Signed-off-by: François Bobot <[email protected]>

[Sites] Fix an error message for generate_sites_module

8e2ca90

Signed-off-by: François Bobot <[email protected]>

[Sites] Better localisation of error

47f8943

Fixes ocaml#4325 Signed-off-by: François Bobot <[email protected]>

[Sites] add failing test with sites name which is an OCaml keyword

80c8424

Signed-off-by: François Bobot <[email protected]>

[Sites] sanitize site name in generated modules

cc27c7c

a name matching "$ocaml_keyword_*" is converted into name^"_" Fixes ocaml#4219 Signed-off-by: François Bobot <[email protected]>

bobot added 5 commits June 24, 2021 15:57

[Artifact_substitution] Wrap some channel opening into a fiber

0b5c74c

So that opening and closing be evaluated directly Signed-off-by: François Bobot <[email protected]>

Add reproduction case for ocaml#4682

206e270

Signed-off-by: François Bobot <[email protected]>

[Cmdliner] Add alias option

b6e467f

options that expand to others. Uses it for `--release` and `-p`. Fixes ocaml#4682 , the options are really used Signed-off-by: François Bobot <[email protected]>

Replace --no-promote-install-files to --promote-install-files

003d53c

Signed-off-by: François Bobot <[email protected]>

Disable ocamlformat for cmdliner

e15308a

Signed-off-by: François Bobot <[email protected]>

bobot force-pushed the fix_sites_installation_with_opam branch from 4c69713 to e15308a Compare June 24, 2021 14:01

bobot merged commit cbe1df2 into ocaml:main Jun 24, 2021

bobot mentioned this pull request Jun 24, 2021

[2.9] fix sites installation with opam #4774

Merged

ejgallego mentioned this pull request Jun 29, 2021

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

Merged

MisterDA mentioned this pull request Jul 11, 2021

[2.9] Opam file generation is broken: don't use subst --root in Opam files #4806

Merged

bobot mentioned this pull request Jul 19, 2021

Optional argument for --promote-install-files breaks CLI compatibility #4816

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix sites installation with opam #4730

Fix sites installation with opam #4730

bobot commented Jun 14, 2021

ghost commented Jun 14, 2021 •

edited by ghost

Loading

bobot commented Jun 14, 2021

bobot commented Jun 14, 2021

ejgallego commented Jun 14, 2021

ghost commented Jun 14, 2021

bobot commented Jun 15, 2021 •

edited

Loading

ghost commented Jun 16, 2021

ejgallego commented Jun 18, 2021

rgrinberg commented Jun 21, 2021

ghost commented Jun 22, 2021

bobot commented Jun 24, 2021 •

edited

Loading

Fix sites installation with opam #4730

Fix sites installation with opam #4730

Conversation

bobot commented Jun 14, 2021

ghost commented Jun 14, 2021 • edited by ghost Loading

bobot commented Jun 14, 2021

bobot commented Jun 14, 2021

ejgallego commented Jun 14, 2021

ghost commented Jun 14, 2021

bobot commented Jun 15, 2021 • edited Loading

ghost commented Jun 16, 2021

ejgallego commented Jun 18, 2021

rgrinberg commented Jun 21, 2021

ghost commented Jun 22, 2021

bobot commented Jun 24, 2021 • edited Loading

ghost commented Jun 14, 2021 •

edited by ghost

Loading

bobot commented Jun 15, 2021 •

edited

Loading

bobot commented Jun 24, 2021 •

edited

Loading