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

Support skip-broken option for the upgrade command #1036

Merged
merged 4 commits into from
Nov 29, 2023

Conversation

jan-kolarik
Copy link
Member

@jan-kolarik jan-kolarik commented Nov 22, 2023

  • Document dropped support of the --skip-broken option from the upgrade command
  • Exclude the --skip-broken option from the global options documentation, restricting only to related commands
  • Add missing man pages documentation for options related to resolving missing dependencies

Closes #1033.

@jan-kolarik
Copy link
Member Author

jan-kolarik commented Nov 22, 2023

I'll also try to add the corresponding CI test.

@jan-kolarik
Copy link
Member Author

Hmm, although I've added the option to the upgrade command, upon inspecting the Goal class, it appears that skip_broken is ignored for the upgrade action. Instead, the best option seems to handle the decision regarding missing dependencies in this case. This behavior aligns with the dnf documentation, and I assume it's intentional. Therefore, it seems unnecessary to include the option for the upgrade command, and we can consider removing it from the group upgrade command as well. Alternatively, we could leave it there for compatibility reasons, even though it won't affect behavior. What are your thoughts, @j-mracek? 🙂

@ppisar
Copy link
Contributor

ppisar commented Nov 22, 2023

Interesting, but it makes sense. Do I understand correctly that --best is dual to --skip-broken now? If so we should amend compatibility.conf to prevent people from reporting issues like #1033.

According to the linked document, --skip-broken is default behavior now. However, right now I observe a different behavior: Fedora 40 has a broken dependency like this:

# dnf5 --disable-repo=rawhide --enable-repo=f40-build upgrade 
Updating and loading repositories:
Repositories loaded.
Failed to resolve the transaction:
Problem: package rpmlint-2.5.0-1.fc40.noarch requires python3.12dist(python-magic), but none of the providers can be installed
  - package python3-file-magic-5.45-1.fc40.noarch conflicts with python3-magic < 5.45-1.fc40 provided by python3-magic-0.4.27-5.fc39.noarch
  - installed package python3-file-magic-5.45-1.fc40.noarch obsoletes python3-magic < 5.45-1.fc40 provided by python3-magic-0.4.27-5.fc39.noarch
  - cannot install the best update candidate for package rpmlint-2.4.0-11.fc40.noarch
  - cannot install the best update candidate for package python3-file-magic-5.45-1.fc40.noarch

Adding --no-best reveals many independent updates:

# dnf5 --disable-repo=rawhide --enable-repo=f40-build --no-best upgrade
Updating and loading repositories:
Repositories loaded.
Problem: package rpmlint-2.5.0-1.fc40.noarch requires python3.12dist(python-magic), but none of the providers can be installed
  - package python3-file-magic-5.45-1.fc40.noarch conflicts with python3-magic < 5.45-1.fc40 provided by python3-magic-0.4.27-5.fc39.noarch
  - installed package python3-file-magic-5.45-1.fc40.noarch obsoletes python3-magic < 5.45-1.fc40 provided by python3-magic-0.4.27-5.fc39.noarch
  - cannot install the best update candidate for package rpmlint-2.4.0-11.fc40.noarch
  - cannot install the best update candidate for package python3-file-magic-5.45-1.fc40.noarch

Package                                   Arch    Version                        Repository        Size
Upgrading:
 binutils                                 x86_64  2.41-14.fc40                   f40-build     27.2 MiB
[...]
Transaction Summary:
 Upgrading:        36 packages
 Replacing:        36 packages

A config dump confims --best is in effect:

# dnf5 --disable-repo=rawhide --enable-repo=f40-build --dump-main-config  | grep -E 'best|skip|strict'
best = 1
multilib_policy = best
skip_broken = 0
skip_if_unavailable = 0
skip_unavailable = 0
strict = 1

I don't have any of them defined in /etc and /usr configuration.

@j-mracek
Copy link
Contributor

Hmm, although I've added the option to the upgrade command, upon inspecting the Goal class, it appears that skip_broken is ignored for the upgrade action. Instead, the best option seems to handle the decision regarding missing dependencies in this case. This behavior aligns with the dnf documentation, and I assume it's intentional. Therefore, it seems unnecessary to include the option for the upgrade command, and we can consider removing it from the group upgrade command as well. Alternatively, we could leave it there for compatibility reasons, even though it won't affect behavior. What are your thoughts, @j-mracek? 🙂

This is tough question. From libsolv implementation - upgrade job is always optional. If a package is installed then upgrade don't need to be performed. upgrade job is weaker then install job. To make upgrade job strict, it requires --best to force solver to pick the best candidates for operation.

Install acpi

sudo dnf shell --disablerepo=* --enablerepo=updates
> install acpi
> remove acpi
> run

is failing
vs:

sudo dnf shell --disablerepo=* --enablerepo=updates
> upgrade acpi
> remove acpi
> run

is removing acpi

I would suggest to not add the option to command because it does not make any difference - it would be there only for compatibility reason and does nothing. But it requires a good documentation in DNF4 vs DNF5 section. This approach is painful. If the option will be implemented then we have to mark it as dummy option that is added only for a compatibility reasons.

@j-mracek
Copy link
Contributor

@ppisar It might be surprising, but behavior of DNF and DNF5 does not different much for upgrade.

What you reported in #1036 (comment) is mostly related to difference in configuration files. DNF is shipped with https://github.com/rpm-software-management/dnf/blob/master/etc/dnf/dnf.conf (Fedora) or with https://github.com/rpm-software-management/dnf/blob/master/etc/dnf/dnf-strict.conf (RHEL). Without configuration overrides, DNF behaves something between (best is in code false). DNF5 is not shipped with any configuration overrides (Fedora 41) and behaves like with RHEL configuration.

The default setting for configuration option skip_if_unavailable and best is very important for distribution and I would suggest that we should open a FESCO ticket about defaults. Be honest I would prefer if configuration overrides will be delivered by distribution packages then DNF packages.

For security reasons we have a requirements to set best=true and also best=false. The first group argue that the transaction must fail to create attention to start resolve the issue. The second group thinks that partially updated system is better then not updated or broken packages might create a negative image for Fedora.

May be we can do something in the middle - when best=false, install possible updates, but when a warning is created return value could be not 0.

@ppisar
Copy link
Contributor

ppisar commented Nov 24, 2023

I see my confusion came from dnf.conf(5) which documents best=false, but DNF5 has best=truein code. Once we or Fedora deliver the override tobest=false`, the manual will agree with the user experience.

Question is whether the manual should document a program default, or the distribution override. Because that or the the other way the manual will disagree with the effective setting either on RHEL or on Fedora.

@jan-kolarik
Copy link
Member Author

jan-kolarik commented Nov 28, 2023

I've dropped the commit introducing --skip-broken for the upgrade command. The option was also removed from the group upgrade command. Documentation was added to the DNF4 vs DNF5 changes section (see here - although the introduced section links are only correctly visible in the generated HTML docs later).

@j-mracek j-mracek self-assigned this Nov 28, 2023
@j-mracek
Copy link
Contributor

LGTM

@ppisar
Copy link
Contributor

ppisar commented Nov 28, 2023

Thanks. The documentation is now much better.

@j-mracek j-mracek added this pull request to the merge queue Nov 29, 2023
Merged via the queue into main with commit 2fd3325 Nov 29, 2023
15 of 16 checks passed
@j-mracek j-mracek deleted the jkolarik/upgrade-skip-broken branch November 29, 2023 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

dnf5 upgrade does not recognize --skip-broken option
3 participants