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

Only find candidates that can't be built if there is no solution #245

Conversation

Leonidas-from-XIV
Copy link
Member

We found this issue while pairing.

We should only look at rejected solutions when there is no selected solution. In a scenario where there is 1 selected candidate that builds with dune and all others don't build with dune this would produce a wrong error.

We found this issue while pairing that we should only look at rejected
solutions when there is no selected solution. In a scenario where there
is 1 selected candidate that builds with dune and all others don't build
with dune this would produce a wrong error.
Copy link
Contributor

@NathanReb NathanReb left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for fixing this!

I just made a suggestion to improve the changelog entry but this is otherwise good to go!

CHANGES.md Outdated Show resolved Hide resolved
Co-authored-by: Nathan Rebours <[email protected]>
@Leonidas-from-XIV
Copy link
Member Author

Yes, I think it is better. Writing changelogs from a user perspective is quite hard when you look too much at the code. Thanks for the improvement!

@Leonidas-from-XIV Leonidas-from-XIV merged commit 67a4d37 into tarides:main Jan 13, 2022
@Leonidas-from-XIV Leonidas-from-XIV deleted the find-candidates-only-without-solution branch January 13, 2022 14:34
NathanReb added a commit to NathanReb/opam-repository that referenced this pull request Apr 19, 2022
CHANGES:

### Added

- Add opam extensions `x-opam-monorepo-opam-repositories` and
  `x-opam-monorepo-global-opam-vars` to make `lock` fully reproducible.
  (tarides/opam-monorepo#250, tarides/opam-monorepo#253, @NathanReb)
- Show an error message when the solver can't find any version that satisfies
  the requested version constraint in the user's OPAM file (tarides/opam-monorepo#215, tarides/opam-monorepo#248,
  @Leonidas-from-XIV)
- Allow packages to be marked as being provided by Opam and not to be pulled by
  `opam-monorepo`. To control this a new optional Opam file field,
  `x-opam-monorepo-opam-provided` is introduced. Its value is a list of package
  names that are to be excluded from being pulled (tarides/opam-monorepo#234, @Leonidas-from-XIV)
- Show an error message when the OCaml version of the lock file does not match
  the OCaml version of the switch (tarides/opam-monorepo#267, tarides/opam-monorepo#268, @Leonidas-from-XIV)
- Generate a `duniverse/README.md` file to explain the basics of
  `opam-monorepo` in the vendored directory (tarides/opam-monorepo#272, tarides/opam-monorepo#274, @Leonidas-from-XIV)
- Add a `--prefer-cross-compile` flag for the solver to select cross-compiling
  versions of packages when available. This is determined through the presence
  of the `"cross-compile"` tag in the opam metadata.

### Changed

- Bump lockfile version to 0.3 (tarides/opam-monorepo#285, @NathanReb)
- Mark packages to be pulled by opam-monorepo with the `vendor` variable so
  using OPAM with `opam install --deps-only --locked .` will not install
  packages that will be installed with `opam-monorepo pull` (tarides/opam-monorepo#237,
  @Leonidas-from-XIV)

### Deprecated

### Fixed

- Fix a bug where a package which had a single version that built with dune and got selected by the solver
  would be reported has having no version building with dune. (tarides/opam-monorepo#245, @Leonidas-from-XIV)
- Fix the solver so it does not select beta versions of the compiler unless
  forced to by version constraints or `--ocaml-version`. (tarides/opam-monorepo#269, @NathanReb)

### Removed

- Drop support for lockfile versions 0.2 and lower (tarides/opam-monorepo#285, @NathanReb)

### Security
NathanReb added a commit to NathanReb/opam-repository that referenced this pull request Apr 20, 2022
CHANGES:

### Added

- Add opam extensions `x-opam-monorepo-opam-repositories` and
  `x-opam-monorepo-global-opam-vars` to make `lock` fully reproducible.
  (tarides/opam-monorepo#250, tarides/opam-monorepo#253, @NathanReb)
- Show an error message when the solver can't find any version that satisfies
  the requested version constraint in the user's OPAM file (tarides/opam-monorepo#215, tarides/opam-monorepo#248, tarides/opam-monorepo#290
  @Leonidas-from-XIV)
- Allow packages to be marked as being provided by Opam and not to be pulled by
  `opam-monorepo`. To control this a new optional Opam file field,
  `x-opam-monorepo-opam-provided` is introduced. Its value is a list of package
  names that are to be excluded from being pulled (tarides/opam-monorepo#234, @Leonidas-from-XIV)
- Show an error message when the OCaml version of the lock file does not match
  the OCaml version of the switch (tarides/opam-monorepo#267, tarides/opam-monorepo#268, @Leonidas-from-XIV)
- Generate a `duniverse/README.md` file to explain the basics of
  `opam-monorepo` in the vendored directory (tarides/opam-monorepo#272, tarides/opam-monorepo#274, @Leonidas-from-XIV)
- Add a `--prefer-cross-compile` flag for the solver to select cross-compiling
  versions of packages when available. This is determined through the presence
  of the `"cross-compile"` tag in the opam metadata.

### Changed

- Bump lockfile version to 0.3 (tarides/opam-monorepo#285, @NathanReb)
- Mark packages to be pulled by opam-monorepo with the `vendor` variable so
  using OPAM with `opam install --deps-only --locked .` will not install
  packages that will be installed with `opam-monorepo pull` (tarides/opam-monorepo#237,
  @Leonidas-from-XIV)

### Fixed

- Fix a bug where a package which had a single version that built with dune and got selected by the solver
  would be reported has having no version building with dune. (tarides/opam-monorepo#245, @Leonidas-from-XIV)
- Fix the solver so it does not select beta versions of the compiler unless
  forced to by version constraints or `--ocaml-version`. (tarides/opam-monorepo#269, @NathanReb)

### Removed

- Drop support for lockfile versions 0.2 and lower (tarides/opam-monorepo#285, @NathanReb)
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.

2 participants