core: Use SOLVER_FAVOR in relaxed mode

[jlebon]

This closes the loop on the "developer experience" when composing with
lockfiles. We want to remain flexible and allow adding/removing packages
from the manifest without editing lockfiles. But we still want libsolv
to give preference to lockfile packages when evaluating alternatives.

For example, if the manifest has a request for a foobar Provides which
both foo and bar provide and foo is in the lockfile, but not
bar, we want libsolv to give priority to foo. IOW, the presence of a
package in a lockfile is a signal that we expect the Provides to be met
by that package.

Use libsolv's SOLVER_FAVOR for this.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlebon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [jlebon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[jlebon]

Requires: #2058
Requires: rpm-software-management/libdnf#937

[jlebon]

The feel I'm going for here is to match the UX from cargo. Reality is not so simple however, because there are (unsurprisingly) major differences between Rust packages and RPM packages. Notably:

• AFAIK, Rust packages don't use Provides and so the crate name you write in Cargo.toml is the crate name cargo downloads and locks. OTOH, treefile package entries aren't actual package names necessarily and can be fulfilled by packages named entirely different things. (I actually debated whether we should make this more strict so that we always require pure package names only... though that's a compatibility break, and it takes us farther away from dnf parity).
• cargo is in charge of doing its own depsolving, whereas we're delegating to another library. So we inherently have less control on how depsolving occurs.

That said, I think overall this matches what cargo does in spirit. See e.g.: https://github.com/rust-lang/cargo/blob/74e3a7d5b756d7c0e94399fc29fcd154e792c22a/src/cargo/ops/resolve.rs#L235-L249

[jlebon]

A real example of what this fixes here: coreos/coreos-installer#222.

[jlebon]

OK, rebased this now and added a test! But there's an unrelated regression in libdnf: rpm-software-management/libdnf#946 (should be an easy review!).

[jlebon]

OK, this is ready to go now!

[jlebon]

symbol lookup error: /home/jenkins/agent/workspace/i_coreos_rpm-ostree_PR-2059-ZZV5BSNPBI7KHN6TOD4VPII6AMCVRJEJ5SWWOOQV6BF6COOJMIQQ/packaging/rpm-ostree-2020.1.89.g430653d1/.libs/librpmostree-1.so.1: undefined symbol: hy_goal_favor

Hmm, weird. Not sure why it's not seeing it even though it seems like it is checking out and building the right version of libdnf. Prow for example compiles it fine.

[cgwalters]

Maybe
INTROSPECTION_SCANNER_ENV = env LD_LIBRARY_PATH=$(top_builddir)/libdnf-build/libdnf started failing for some reason?

[cgwalters]

LGTM, I'm in favor of this.

[jlebon]

/hold

[jlebon]

Still no idea what's going on with this, but you're right it's related to our old friend g-ir-scanner. I haven't been able to reproduce this locally. Will dig in some more tomorrow.

[jlebon]

OK, I finally got to the bottom of the CI failure here. Debugging this was way more involved than I expected though I did learn a bunch along the way! Anyway, split that out into #2089.

As for this PR, I think I need to tweak things further here before this is ready to go. Specifically, I think this needs to go together with the tweaks needed for libsolv to favor packages we have already cached all else equal (see coreos/coreos-assembler#1431 (comment)).

/hold

[openshift-ci-robot]

@jlebon: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[openshift-ci]

@jlebon: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/unit	605258a	link	/test unit
ci/prow/build	605258a	link	/test build
ci/prow/fcos-e2e	605258a	link	true	/test fcos-e2e
ci/prow/images	605258a	link	true	/test images
ci/prow/clang-analyzer	605258a	link	true	/test clang-analyzer
ci/prow/build-clang	605258a	link	true	/test build-clang
ci/prow/kola-upgrade	605258a	link	true	/test kola-upgrade

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[jlebon]

I think this would still be nice to have but it's subtle to get right and hasn't seemed to come up as much as expected in CoreOS development, so dropping.