topgrade icon indicating copy to clipboard operation
topgrade copied to clipboard

feat: allow to configure the apt upgrade method on debian

Open thvitt opened this issue 1 year ago • 6 comments

dist-upgrade, which was the hard-coded option, is allowed to remove packages in order to resolve dependency conflicts. This might cause quite a mess ... it is left as the default, but can be modified by the option apt_command.

Standards checklist:

  • [x] The PR title is descriptive.
  • [x] I have read CONTRIBUTING.md
  • [x] The code compiles (cargo build)
  • [x] The code passes rustfmt (cargo fmt)
  • [x] The code passes clippy (cargo clippy)
  • [x] The code passes tests (cargo test)
  • [x] Optional: I have tested the code myself

For new steps

  • [ ] Optional: Topgrade skips this step where needed
  • [ ] Optional: The --dry-run option works with this step
  • [ ] Optional: The --yes option works with this step if it is supported by the underlying command

If you developed a feature or a bug fix for someone else and you do not have the means to test it, please tag this person here.

thvitt avatar Mar 23 '24 18:03 thvitt

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 4.98%. Comparing base (373cd3b) to head (eed298c).

Files Patch % Lines
src/config.rs 0.00% 9 Missing :warning:
src/steps/os/linux.rs 0.00% 2 Missing :warning:
Additional details and impacted files
@@           Coverage Diff            @@
##            main    #749      +/-   ##
========================================
- Coverage   5.03%   4.98%   -0.06%     
========================================
  Files         37      37              
  Lines      12085   12216     +131     
========================================
  Hits         609     609              
- Misses     11476   11607     +131     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 23 '24 18:03 codecov[bot]

Hi, thanks for your interest in contributing to Topgrade!

The CI failures are not related, please rebase your branch (against main) to get them resolved:)

dist-upgrade, which was the hard-coded option, is allowed to remove packages in order to resolve dependency conflicts. This might cause quite a mess

I can see this disaster, in your opinion, what is the appropriate default command to use? And, I am thinking, it is suitable for most users, then we can actually replace dist-upgrade with it

SteveLauC avatar Mar 24 '24 03:03 SteveLauC

I can see this disaster, in your opinion, what is the appropriate default command to use? And, I am thinking, it is suitable for most users, then we can actually replace dist-upgrade with it

upgrade certainly is the safer variant, OTOH I am using topgrade --yes on Debian testing, and they are just migrating to a 64bit datetime type, so I shouldn’t be too surprised :)

My use case for topgrade is 'keep the system relatively up-to-date without too much hassle', so for that, upgrade would be the better default. BTW it looks like it’s the default for nala anyway (nala’s equivalent to apt-get dist-upgrade is nala full-upgrade)

thvitt avatar Mar 24 '24 13:03 thvitt

BTW it looks like it’s the default for nala anyway (nala’s equivalent to apt-get dist-upgrade is nala full-upgrade)

Thanks for the info!


I am thinking about:

  1. use apt upgrade as the default option (for apt-fast and apt-get)

  2. Make the subcommand for nala and apt-fast/apt-get configurable

    available options that can be specified in the config file:

    1. upgrade (can be used with nala/apt-fast/apt-get)
    2. dist-upgrade (ONLY available for apt-fast/apt-get)
    3. full-upgrade (ONLY available for nala)
  3. If an invalid option is specified in the config file, print an error message and skip the step during step execution (i.e., we don't do check if the config is valid while loading configuration)

  4. For the config entry name, we might want to use something like this, specifying:

    1. It is for Debian-like distros
    2. This is the "upgrade" sub-command
    # The upgrade subcommand to use on Debian-like distros
    # For `apt-get` and `apt-fast`, one can use:
    # 1. "upgrade"
    # 2. "dist-upgrade"
    #
    # For `nala`, available options:
    # 1. "upgrade"
    # 2. "full-upgrade"
    debian_like_apt_upgrade_cmd = "value"
    

I would like to hear your thoughts on this since I am not a Debian user:)

SteveLauC avatar Mar 25 '24 06:03 SteveLauC

The longer I think about it:

  • I agree that upgrade should be the default as safest option.
  • nala full-upgradeapt-get dist-upgrade --autoremove
  • You can, in theory, have apt, apt-fast, and nala installed in parallel and use a different one of them each day, since they all work on the same package store.

I think it is a little confusing to configure the subcommand by hand but let the package manager be autodetected. So maybe the most sensible variant is to have the auto-detected tool with upgrade as default and just offer one configuration option that can be set to the whole command line, like debian_like_upgrade_cmdline = "apt-fast full-upgrade --autoremove"? The update command could just take the first word of this and run it with update, since that is all the same.

Alternatively, maybe a boolean option debian_like_dist_upgrade that, when true, selects dist-upgrade or full-upgrade depending on the detected package manager.

thvitt avatar Apr 02 '24 10:04 thvitt

I think it is a little confusing to configure the subcommand by hand but let the package manager be autodetected. So maybe the most sensible variant is to have the auto-detected tool with upgrade as default and just offer one configuration option that can be set to the whole command line,

Yeah, I agree that it feels weird

like debian_like_upgrade_cmdline = "apt-fast full-upgrade --autoremove"?

Though I feel Topgrade should not do such things (let the user write the whole script) either

SteveLauC avatar Apr 13 '24 13:04 SteveLauC