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.

Sign up for GitHub

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

Jump to bottom

[install] Allow users to specify docdir and etcdir #4744

Merged
merged 1 commit into from
Jun 19, 2021
Merged

[install] Allow users to specify docdir and etcdir #4744

merged 1 commit into from
Jun 19, 2021

Conversation

ejgallego
Copy link
Collaborator

Fixes #4723

This is an important fix for those using Dune to build, for example,
Debian packages, as docdir is there prefix/share/doc/package,
similarly for etc.

Thus, I request for approval to ship this in 2.9.

I have written the patch very conservatively, as to allow easy
backporting, but once we release 2.9 it would be convenient to
refactor this code so overridable directories are placed into a
record.

Signed-off-by: Emilio Jesus Gallego Arias e+git@x80.org

@ejgallego ejgallego requested review from rgrinberg, bobot and a user and removed request for rgrinberg and bobot June 16, 2021 17:38
@ejgallego ejgallego added this to the 2.9 milestone Jun 16, 2021
@ejgallego
Copy link
Collaborator Author

@glondu I believe this should have a non-trivial impact on the Debian packages, I guess as of today you have to go and fix the doc install paths manually, correct?

rgrinberg
rgrinberg approved these changes Jun 16, 2021
View reviewed changes
Copy link
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

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

The code looks good Emilio. How about we just treat all directories the same way and make them all overridable? e.g. share. I don't see the downsides of doing this, and the upside is that it's one less thing to remember for users. It's rather annoying to maintain a map in your head between overridable directories and versions of dune.

@ejgallego
Copy link
Collaborator Author

ejgallego commented Jun 16, 2021 •
edited
Loading

How about we just treat all directories the same way and make them all overridable? e.g. share. I don't see the downsides of doing this, and the upside is that it's one less thing to remember for users. It's rather annoying to maintain a map in your head between overridable directories and versions of dune.

Indeed, I think it makes sense, and I would have tried something like that if not for the nature of the PR to be a hotfix; note this comment by @bobot, and issue #680 , so the only real question is how do we split work to do this among 2.9 and 3.0 , and converge on #680 of course.

I guess that adding a couple more --XXXdir for 2.9 is OK, whereas for 3.0 we could redesign the command line interface too. WDYT?

There is still one directory which is special it seems, namely --libdir as we need to support multiple directories.

@bobot
Copy link
Collaborator

bobot commented Jun 17, 2021 •
edited
Loading

Could you try the full version, perhaps in another branch, since we all want it at the end in order to see if it is really too big for 2.9?

(For libdir we should perhaps separate the search path from the default destination path also in the command-line.)

@ejgallego
Copy link
Collaborator Author

Could you try the full version, perhaps in another branch, since we all want it at the end in order to see if it is really too big for 2.9?

Would the "full" version involve changing cmdline flags?

If so we may better postpone it to 3.0 ; I'm afraid it is too late anyways as I didn't even start coding a full version.

@ejgallego
Copy link
Collaborator Author

ejgallego commented Jun 18, 2021 •
edited
Loading

Given that 2.9 should appear as soon as the cherry picking from the sites PR is done, I will merge this as-is and backport, but place a comment in the code.

Hopefully the situation should be uniformized if #680 can be solved for 3.0.

ejgallego added a commit to ejgallego/dune that referenced this pull request Jun 18, 2021
@ejgallego
[2.9] [install] Allow users to specify docdir and etcdir
3054ec2 
Backport of ocaml#4744

Signed-off-by: Emilio Jesus Gallego Arias <e+git@x80.org>
@ejgallego ejgallego mentioned this pull request Jun 18, 2021
Merged
@rgrinberg
Copy link
Member

Then I think the PR is fine as is.

ejgallego added a commit to ejgallego/dune that referenced this pull request Jun 18, 2021
@ejgallego
[2.9] [install] Allow users to specify docdir and etcdir
622ee82 
Backport of ocaml#4744

Signed-off-by: Emilio Jesus Gallego Arias <e+git@x80.org>
@ejgallego ejgallego mentioned this pull request Jun 18, 2021
Open
@ejgallego
[install] Allow users to specify docdir and etcdir
fc56085 
Fixes ocaml#4723

This is an important fix for those using Dune to build, for example,
Debian packages, as docdir is there `prefix/share/doc/package`,
similarly for `etc`.

Thus, I request for approval to ship this in 2.9.

I have written the patch very conservatively, as to allow easy
backporting, but once we release 2.9 it would be convenient to
refactor this code so overridable directories are placed into a
record.

Signed-off-by: Emilio Jesus Gallego Arias <e+git@x80.org>
@ejgallego ejgallego merged commit 792e967 into ocaml:main Jun 19, 2021
@ejgallego ejgallego deleted the install+extra_dirs branch June 19, 2021 13:06
ejgallego added a commit that referenced this pull request Jun 19, 2021
@ejgallego
[2.9] [install] Allow users to specify docdir and etcdir (#4754)
e21f45a 
Backport of #4744

Signed-off-by: Emilio Jesus Gallego Arias <e+git@x80.org>
@ejgallego ejgallego mentioned this pull request Jun 21, 2021
Merged
ejgallego added a commit to ejgallego/opam-repository that referenced this pull request Jun 21, 2021
@ejgallego
[new release] dune-action-plugin, dune-glob, dune-site, dune-configur…
da0714f 
…ator, dune-private-libs, dune and dune-build-info (2.9-rc1)

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)

- Handle renaming of `coq.kernel` library to `coq-core.kernel` in Coq 8.14 (ocaml/dune#4713, @proux01)

- 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)
ejgallego added a commit to ejgallego/opam-repository that referenced this pull request Jun 29, 2021
@ejgallego
[new release] dune-action-plugin, dune-glob, dune-site, dune-configur…
a13f842 
…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)
ejgallego added a commit to ejgallego/opam-repository that referenced this pull request Jul 1, 2021
@ejgallego
[new release] dune-action-plugin, dune-glob, dune-site, dune-configur…
96c14a5 
…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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgrinberg rgrinberg rgrinberg approved these changes

@bobot bobot Awaiting requested review from bobot

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.9
Development

Successfully merging this pull request may close these issues.

[install] dune install seems to lack enough configuration options as to accommodate Linux's FSH
3 participants
@ejgallego @bobot @rgrinberg