Dependency provider selection result may not match user's intention

starrify commented 1 month ago

$ pikaur -Vq
Pikaur v1.25.3
Pacman v6.1.0 - libalpm v14.0.0 - pyalpm v0.10.6

Description:

The Choose a package provider for <dependency> prompt performs a superfluous round of sorting the options, leading to potential mismatch between the user's input and the perceived result.

Here is full logs attached (click to expand) for elaboration, where the user chooses 4 for nvidia-535xx-utils while pikaur perceives nvidia-430xx-utils instead.

``` $ pikaur nvidia-535xx-dkms Searching... [###] 1) aur/nvidia-535xx-dkms 535.183.01-2 (12, 2.94) NVIDIA drivers - module sources, 535 branch Please enter the number of the package(s) you want to install and press [Enter] (default=1): > 1 Reading repository package databases... Reading local package database... Resolving AUR dependencies... :: Choose a package provider for nvidia-utils: 1) aur nvidia-utils-beta 560.35.03-1 (346, 3.53) 2) aur nvidia-470xx-utils 470.256.02-6 (98, 2.01) 3) aur nvidia-390xx-utils 390.157-12 (57, 1.23) 4) aur nvidia-535xx-utils 535.183.01-2 (12, 2.94) 5) aur nvidia-vulkan-utils 535.43.09-1 [outofdate: 2023/10/05] (31, 0.00) 6) aur nvidia-340xx-utils 340.108-1 (23, 0.00) 7) aur nvidia-525xx-utils 525.147.05-3 (6, 0.02) 8) aur nvidia-510xx-utils 510.108.03-2 (4, 0.00) 9) aur nvidia-410xx-utils 410.93-1 (0, 0.00) 10) aur nvidia-430xx-utils 430.40-9 (0, 0.00) 11) aur nvidia-440xx-utils 440.100-1 (0, 0.00) 12) aur nvidia-550xx-utils 550.107.02-1 (0, 0.00) 13) aur nvidia-utils-tesla 550.90.07-1 (0, 0.00) :: [v]iew package details > 4 :: New dependency will be installed from repository: patchelf (for nvidia-535xx-dkms) -> 0.18.0-3 :: AUR package will be installed: nvidia-535xx-dkms -> 535.183.01-2 :: New dependency will be installed from AUR: nvidia-430xx-utils (for nvidia-535xx-dkms) 560.35.03-2 -> 430.40-9 :: Proceed with installation? [Y/n] :: [v]iew package details [m]anually select packages :: [r] show if packages are required by already installed packages >> ```

Analysis:

This is believed to happen within the Provider.choose method. While said method is expected to return an index of the given options (sample usage), it however:

Invokes print_package_search_results which reorders the options,
And does not make use of the returned list of options which is reordered.

Proposed fix:

It is assumed that this issue may be fixed by altering Provider.choose's behavior so that:

It no longer reorders the options upon presenting them to users (use a different print function, or add a do-not-sort parameter to print_package_search_results)
Or, it still presents to users sorted options via print_package_search_results but additionally matches the user input against the return value of print_package_search_results so that the right index may be returned.

actionless commented 1 month ago

And does not make use of the returned list of options which is reordered.

yup, this was the problem 👍

starrify commented 1 month ago

Hi, thanks for the quick fix! I took a quick look at d6aac5c though I'm sorry to say that I'm afraid that might not be a thourough fix.

d6aac5c fixes the cached dependency name (that is stored in cls.saved_providers) which is good, yet it does not affect the return value of Provide.choose which is still the user-provided index (based on the sorted prompt). Therefore I reckon that it is still the unexpected index that is used (example).

Update: I see that d6aac5c does fix this very use case (e.g. sample log attached above) and turns out here Provider.choose gets invoked twice and it is eventually the 2nd result to be used -- in this case the cached result in cls.saved_providers makes the difference.

actionless commented 1 month ago

great that i didn't made bugfix release yet 😺 yup, i think i'll change it to return the name instead of index

actionless commented 1 month ago

@starrify please check now

starrify commented 1 month ago

LGTM and thanks a lot! :smile_cat:

actionless / pikaur