Skip to content

Add 'install --formula', rewrite '--deps-only' #4975

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

Merged
merged 8 commits into from
May 3, 2022

Conversation

AltGr
Copy link
Member

@AltGr AltGr commented Dec 23, 2021

No description provided.

@AltGr AltGr marked this pull request as draft December 23, 2021 17:20
@rjbou rjbou added this to the 2.2.0~alpha milestone Jan 3, 2022
@kit-ty-kate kit-ty-kate linked an issue Jan 7, 2022 that may be closed by this pull request
@kit-ty-kate kit-ty-kate added the PR: WIP Not for merge at this stage label Jan 18, 2022
@AltGr
Copy link
Member Author

AltGr commented Feb 3, 2022

Should be finished, just needs some git history cleanup + tests

@AltGr AltGr force-pushed the install-formula branch 2 times, most recently from e723653 to 8957e01 Compare February 7, 2022 14:15
@AltGr AltGr marked this pull request as ready for review February 11, 2022 10:50
@AltGr AltGr changed the title WIP: Add 'install --formula', rewrite '--deps-only' Add 'install --formula', rewrite '--deps-only' Feb 11, 2022
@AltGr AltGr removed the PR: WIP Not for merge at this stage label Feb 18, 2022
@rjbou rjbou requested review from rjbou and kit-ty-kate and removed request for rjbou February 18, 2022 11:01
@rjbou
Copy link
Collaborator

rjbou commented Feb 18, 2022

test the test from #4914 handled

@kit-ty-kate kit-ty-kate added the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Feb 18, 2022
@rjbou rjbou removed the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Feb 22, 2022
Copy link
Member

@kit-ty-kate kit-ty-kate left a comment

Choose a reason for hiding this comment

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

Apart from the minor bracket interogation above LGTM.

Copy link
Collaborator

@rjbou rjbou left a comment

Choose a reason for hiding this comment

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

all good! thanks!

@rjbou rjbou force-pushed the install-formula branch from 8c03bbc to f4d9011 Compare March 16, 2022 18:58
@rjbou
Copy link
Collaborator

rjbou commented Mar 16, 2022

rebased

@rjbou
Copy link
Collaborator

rjbou commented Mar 17, 2022

closes #4914

@rjbou rjbou added the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Mar 22, 2022
@rjbou rjbou removed the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Apr 29, 2022
@rjbou rjbou force-pushed the install-formula branch from f4d9011 to ffa1a6a Compare April 29, 2022 16:20
@rjbou rjbou force-pushed the install-formula branch from ffa1a6a to dd83694 Compare May 2, 2022 10:15
AltGr and others added 8 commits May 3, 2022 18:22
Using the same scheme as for enforcing the invariant (but with a distinct path,
we don't want the two to get confused in messages, etc.)

This is only in the internal API at the moment. The `request` type is extended
to allow a formula (instead of an atom list) in its `wish_install` field.
Working around a set of bugs that were in the original version, but also
renouncing using the `--formula` code path because that was too limited for
using deps-only on multiple packages, and with unspecified versions.

This creates virtual packages at the client level that mimic the dependencies of
the specified packages (except for dependencies between them). This resolves a
few issues:

- these packages are always available (the original implem patched the packages
  but that could result in unavailable packages getting installed) ; they are
  however marked as avoid-version if unavailable as a cheap way to stay on the
  right version when possible.

- they exist in parallel to their "real" counterpart, so that these can still be
  installed if required through dependency chains, keeping consistency of the
  solution.

- they forbid different versions of their "real" counterpart, to ensure
  consistency.

- the `deps-of-xxx` package are removed from the solution, but could appear in
  conflict messages. I'd argue that it should make things understandable even if
  it is an implementation detail.
…experimental

For printing of action reasons with `--formula`, unfortunately we have
to add yet another argument, because the `requested` for applying
`--with-test` and similar flags no longer matches the one used for
printing actions reasons: the former doesn't include packages specified
through `--formula` while the latter needs to.
Formula remains strict, the best effort only concerns the individual atoms.
@rjbou rjbou force-pushed the install-formula branch from dd83694 to 4254e78 Compare May 3, 2022 18:00
@rjbou rjbou merged commit 091b3d2 into ocaml:master May 3, 2022
LasseBlaauwbroek added a commit to LasseBlaauwbroek/opam that referenced this pull request Aug 26, 2022
In PR ocaml#4796 I fixed a problem with `--best-effort`. Subsequently ocaml#4975 seems to
have broken this again. Not sure if this is the correct fix. There may also be
additional places where this is going wrong...
kit-ty-kate pushed a commit to LasseBlaauwbroek/opam that referenced this pull request Aug 31, 2022
In PR ocaml#4796 I fixed a problem with `--best-effort`. Subsequently ocaml#4975 seems to
have broken this again. Not sure if this is the correct fix. There may also be
additional places where this is going wrong...
kit-ty-kate pushed a commit to LasseBlaauwbroek/opam that referenced this pull request Aug 31, 2022
In PR ocaml#4796 I fixed a problem with `--best-effort`. Subsequently ocaml#4975 seems to
have broken this again. Not sure if this is the correct fix. There may also be
additional places where this is going wrong...
kit-ty-kate added a commit that referenced this pull request Sep 1, 2022
Fix --best-effort regression introduced in #4975
Leonidas-from-XIV pushed a commit to Leonidas-from-XIV/opam that referenced this pull request Mar 10, 2023
In PR ocaml#4796 I fixed a problem with `--best-effort`. Subsequently ocaml#4975 seems to
have broken this again. Not sure if this is the correct fix. There may also be
additional places where this is going wrong...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

opam install --deps-only skips the installation reasons
3 participants