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

dune install -p <pkg> requires every packages from the project to be installed, not just <pkg> #4814

Closed
kit-ty-kate opened this issue Jul 16, 2021 · 8 comments · Fixed by #9879 or ocaml/opam-repository#25240
Labels
accepted accepted proposals bug
Milestone

Comments

@kit-ty-kate
Copy link
Member

Expected Behavior

With 3 packages into one repository, 2 of the packages available globally, I would expect dune install -p pkg3 to succeed and to only install pkg3.

Actual Behavior

Fails with:

Error: The following <package>.install are missing:
- _build/default/current_ocluster.install
- _build/default/ocluster-api.install
Hint: try running: dune build @install

and if we do dune build @install as hinted, all 3 packages are install/reinstalled (overriding globally installed packages, breaking the expectation of -p)

Reproduction

FROM ocaml/opam:debian
RUN git clone git://github.com/ocurrent/ocluster.git
WORKDIR ocluster
RUN git submodule update --init
RUN sudo ln -f /usr/bin/opam-2.1 /usr/bin/opam
RUN opam pin add -n .
RUN opam update --depexts
RUN opam install dune.2.8.5
RUN opam install --deps-only ocluster
RUN opam exec -- dune build -p ocluster
RUN opam exec -- dune install -p ocluster

Specifications

  • Version of dune (output of dune --version): 2.8.5
@kit-ty-kate kit-ty-kate changed the title dune install -p does not work anymore dune install -p <pkg> requires every packages from the project to be installed, not just <pkg> Jul 18, 2021
@rgrinberg rgrinberg added the bug label Jul 27, 2021
@bobot bobot self-assigned this Sep 8, 2021
@bobot
Copy link
Collaborator

bobot commented Sep 9, 2021

It is indeed not taken into account and it should if it is an available option. But I wonder if there is a meaningfull difference between dune install ocluster and dune install --only-package ocluster, and I don't see which effect --release should have on the install command.

@bobot
Copy link
Collaborator

bobot commented Nov 21, 2021

@kit-ty-kate Do you think it would be OK to remove -p from the install command, or making dune install -p foo an alias of dune install foo.

@kit-ty-kate
Copy link
Member Author

Sure, an alias would be perfectly fine I think. However I think the hint given on errors should also change accordingly.
If dune install <pkg> or dune install -p <pkg> is given, the hint should be:

Hint: try running: dune build -p <pkg> @install

not

Hint: try running: dune build @install

@bobot
Copy link
Collaborator

bobot commented Nov 21, 2021

I'm not sure for the hint. I agree the current is wrong in some case, but the proposed one is also wrong in others (when the other libraries are not installed).

So we can have dune install -p <pkg> -> dune build -p <pkg> @install and dune install <pkg> -> dune build @install. But since dune install -p <pkg> and dune install <pkg> should be equivalent, I would prefer to write Hint: try running: dune build [-p <pkg>] @install, what do you think?

@fangyi-zhou
Copy link
Contributor

Hi, got hit by this today and it took my quite a while to understand what went wrong. I think it's worth implementing the hint of some sort to help people understand this issue.

@bobot
Copy link
Collaborator

bobot commented Jul 13, 2022

Finally the alias proposition seems the most sensible. So MR would be accepted for dune install -p <pkg> an alias for dune install <pkg>.

@bobot bobot added the accepted accepted proposals label Jul 13, 2022
fangyi-zhou added a commit to fangyi-zhou/dune that referenced this issue Jul 13, 2022
fangyi-zhou added a commit to fangyi-zhou/dune that referenced this issue Jul 13, 2022
Signed-off-by: Fangyi Zhou <[email protected]>
fangyi-zhou added a commit to fangyi-zhou/dune that referenced this issue Jul 14, 2022
Signed-off-by: Fangyi Zhou <[email protected]>
@emillon emillon linked a pull request Jul 19, 2022 that will close this issue
emillon added a commit that referenced this issue Jul 19, 2022
* Add a test case for #4814

Signed-off-by: Fangyi Zhou <[email protected]>

* install: improve hint when install file is missing

Signed-off-by: Fangyi Zhou <[email protected]>

Co-authored-by: Etienne Millon <[email protected]>
@rgrinberg
Copy link
Member

Yes, the bug is that we're accepting -p as an argument at all.

@emillon
Copy link
Collaborator

emillon commented Feb 1, 2024

FTR, it's going to be more difficult than expected because dune sites uses that in their installation instructions:
https://github.com/ocaml/dune/blob/3.13.0/src/dune_rules/opam_create.ml#L62

@rgrinberg rgrinberg added this to the 3.14.0 milestone Feb 2, 2024
@rgrinberg rgrinberg linked a pull request Feb 2, 2024 that will close this issue
emillon added a commit to emillon/opam-repository that referenced this issue Feb 9, 2024
CHANGES:

### Added

- Introduce a `(dynamic_include ..)` stanza. This is like `(include foo)` but
  allows `foo` to be the target of a rule. Currently, there are some
  limitations on the stanzas that can be generated. For example, public
  executables, libraries are currently forbidden. (ocaml/dune#9913, @rgrinberg)

- Introduce `$ dune promotion list` to print the list of available promotions.
  (ocaml/dune#9705, @moyodiallo)

- If Sherlodoc is installed, add a search bar in generated HTML docs (ocaml/dune#9772,
  @EmileTrotignon)

- Add `only_sources` field to `copy_files` stanza (ocaml/dune#9827, fixes ocaml/dune#9709,
  @jchavarri)

- The `(foreign_library)` stanza now supports the `(enabled_if)` field. (ocaml/dune#9914,
  @nojb)

### Fixed

- Fix `$ dune install -p` incorrectly recognizing packages that are supposed to
  be filtered (ocaml/dune#9879, fixes ocaml/dune#4814, @rgrinberg)

- subst: correctly handle opam files in opam/ subdirectory (ocaml/dune#9895, fixes ocaml/dune#9862,
  @emillon)

- Odoc private rules are not set up if a library is not available due to
  `enabled_if` (ocaml/dune#9897, @rgrinberg and @jchavarri)

### Changed

- When dune language 3.14 is enabled, resolve the binary in `(run %{bin:..}
  ..)` from where the binary is built. (ocaml/dune#9708, @rgrinberg)

- boot: remove single-command bootstrap. This was an alternative bootstrap
  strategy that was used in certain conditions. Removal makes the bootstrap a
  bit slower on Linux when only a single core is available, but bootstrap is
  now reproducible in all cases. (ocaml/dune#9735, fixes ocaml/dune#9507, @emillon)
emillon added a commit to emillon/opam-repository that referenced this issue Feb 12, 2024
CHANGES:

### Added

- Introduce a `(dynamic_include ..)` stanza. This is like `(include foo)` but
  allows `foo` to be the target of a rule. Currently, there are some
  limitations on the stanzas that can be generated. For example, public
  executables, libraries are currently forbidden. (ocaml/dune#9913, @rgrinberg)

- Introduce `$ dune promotion list` to print the list of available promotions.
  (ocaml/dune#9705, @moyodiallo)

- If Sherlodoc is installed, add a search bar in generated HTML docs (ocaml/dune#9772,
  @EmileTrotignon)

- Add `only_sources` field to `copy_files` stanza (ocaml/dune#9827, fixes ocaml/dune#9709,
  @jchavarri)

- The `(foreign_library)` stanza now supports the `(enabled_if)` field. (ocaml/dune#9914,
  @nojb)

### Fixed

- Fix `$ dune install -p` incorrectly recognizing packages that are supposed to
  be filtered (ocaml/dune#9879, fixes ocaml/dune#4814, @rgrinberg)

- subst: correctly handle opam files in opam/ subdirectory (ocaml/dune#9895, fixes ocaml/dune#9862,
  @emillon)

- Odoc private rules are not set up if a library is not available due to
  `enabled_if` (ocaml/dune#9897, @rgrinberg and @jchavarri)

### Changed

- When dune language 3.14 is enabled, resolve the binary in `(run %{bin:..}
  ..)` from where the binary is built. (ocaml/dune#9708, @rgrinberg)

- boot: remove single-command bootstrap. This was an alternative bootstrap
  strategy that was used in certain conditions. Removal makes the bootstrap a
  bit slower on Linux when only a single core is available, but bootstrap is
  now reproducible in all cases. (ocaml/dune#9735, fixes ocaml/dune#9507, @emillon)
nberth pushed a commit to nberth/opam-repository that referenced this issue Jun 18, 2024
CHANGES:

### Added

- Introduce a `(dynamic_include ..)` stanza. This is like `(include foo)` but
  allows `foo` to be the target of a rule. Currently, there are some
  limitations on the stanzas that can be generated. For example, public
  executables, libraries are currently forbidden. (ocaml/dune#9913, @rgrinberg)

- Introduce `$ dune promotion list` to print the list of available promotions.
  (ocaml/dune#9705, @moyodiallo)

- If Sherlodoc is installed, add a search bar in generated HTML docs (ocaml/dune#9772,
  @EmileTrotignon)

- Add `only_sources` field to `copy_files` stanza (ocaml/dune#9827, fixes ocaml/dune#9709,
  @jchavarri)

- The `(foreign_library)` stanza now supports the `(enabled_if)` field. (ocaml/dune#9914,
  @nojb)

### Fixed

- Fix `$ dune install -p` incorrectly recognizing packages that are supposed to
  be filtered (ocaml/dune#9879, fixes ocaml/dune#4814, @rgrinberg)

- subst: correctly handle opam files in opam/ subdirectory (ocaml/dune#9895, fixes ocaml/dune#9862,
  @emillon)

- Odoc private rules are not set up if a library is not available due to
  `enabled_if` (ocaml/dune#9897, @rgrinberg and @jchavarri)

### Changed

- When dune language 3.14 is enabled, resolve the binary in `(run %{bin:..}
  ..)` from where the binary is built. (ocaml/dune#9708, @rgrinberg)

- boot: remove single-command bootstrap. This was an alternative bootstrap
  strategy that was used in certain conditions. Removal makes the bootstrap a
  bit slower on Linux when only a single core is available, but bootstrap is
  now reproducible in all cases. (ocaml/dune#9735, fixes ocaml/dune#9507, @emillon)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment