refactor(opam_solver): no priority avoidance for avoid-version

[art-w]

More preparation for the on-demand loading of opam files.

The solver was sorting package versions such that the avoid-version ones would come last in the list used by the SAT solver to select from. In practice, this is unnecessary because avoid-version packages are first unselectable (we force the SAT solver to find a solution without them), then selectable if no solution exists without (and we minimize how many of them are selected)... so the end result is the same with sorting and without.

There are situations where this refactoring could change the exact solution picked (if avoid-version is unavoidable and multiple solutions exists with the same number of avoid selected), because it impacts the depth-first-search bias of the SAT solver... but I hope no one depends on this! (what we do today is unspecified anyway)

On the other end, pre-sorting with avoid-version costs a lot: we have to parse each opam file just to check for the existence of the flag for each version of every package.

[punchagan]

Thanks for working via small prep commits and the clear explanation of the change! I'm not familiar with this part of dune, but the change looks reasonable to me. (It might be useful to put the description into the commit message, though it should still be retained when merging.)

[Alizter]

Can we check that we are still respecting this invariant? I don't think we have a test directly checking this. Something like:

   Pinned packages ignore avoid-version flags.

     $ mkdir avoid-pin
     $ cd avoid-pin
     $ mkrepo
     $ add_mock_repo_if_needed
     $ mkpkg fallback
     $ make_dune_project 3.13
     $ mkdir _pinned
     $ cat >_pinned/pinned.opam <<EOF
     > opam-version: "2.0"
     > flags: [avoid-version]
     > EOF
     $ cat >root.opam <<EOF
     > opam-version: "2.0"
     > depends: [ "pinned" | "fallback" ]
     > pin-depends: [ "pinned.1.0.0" "file://$PWD/_pinned" ]
     > EOF

     $ dune_pkg_lock_normalized
     Solution for dune.lock:
     - pinned.1.0.0 (this version should be avoided)

[art-w]

Ha good catch, I added the test and it failed :P Now also fixed

[Alizter]

From what I can tell, with this change, avoided versions will now not be put at the end of the queue of versions to try. This means that we do more work when there are a few avoid-versions in a list.

Here is a test to demonstrate:

File "test/blackbox-tests/test-cases/pkg/avoid-version-sat-work.t", line 1, characters 0-0:
------ test/blackbox-tests/test-cases/pkg/avoid-version-sat-work.t
++++++ test/blackbox-tests/test-cases/pkg/avoid-version-sat-work.t.corrected
File "test/blackbox-tests/test-cases/pkg/avoid-version-sat-work.t", line 17, characters 0-1:
 |
 |  $ mkrepo
 |  $ mkpkg p 1
 |  $ for version in 2 3 4 5; do
 |  >   mkpkg p $version <<EOF
 |  > flags: [avoid-version]
 |  > EOF
 |  > done
 |
 |  $ export DUNE_TRACE=+sat
 |  $ solve p >/dev/null
 |  $ dune trace cat | jq -s 'include "dune"; [ .[] | satSolveEvents | .args ]'
 |  [
 |    {
 |      "num_variables": 31,
 |      "num_clauses": 44,
-|      "num_decisions": 1,
+|      "num_decisions": 4,
-|      "num_conflicts": 0
+|      "num_conflicts": 4,
+|      "num_opam_files": 5
 |    }
 |  ]

I think this is fine in practice, if we are just going to delay the parsing. avoid-version is typically used for older versions anyway so this example is quite artificial, here we use it on the new versions. I would however prefer that we observe this change in this PR.