search

actionless / pikaur

AUR helper with minimal dependencies. Review PKGBUILDs all in once, next build them all without user interaction.
GNU General Public License v3.0
868 stars 87 forks source link

Handling mailformed `depends` field #280

Closed yagebu closed 4 years ago

yagebu commented 6 years ago 
pikaur -Vq
Pikaur v1.2.23
Pacman v5.1.1 - libalpm v11.0.1
Attached log:
pikaur -S ocrmypdf
Reading repository package databases...
Reading local package database...
Resolving AUR dependencies...
Traceback (most recent call last):
  File "/usr/bin/pikaur", line 9, in <module>
    main()
  File "/usr/lib/python3.7/site-packages/pikaur/main.py", line 322, in main
    cli_entry_point()
  File "/usr/lib/python3.7/site-packages/pikaur/main.py", line 235, in cli_entry_point
    run_with_sudo_loop(pikaur_operation)
  File "/usr/lib/python3.7/site-packages/pikaur/core.py", line 265, in run_with_sudo_loop
    raise catched_exc  # pylint: disable=raising-bad-type
  File "/usr/lib/python3.7/site-packages/pikaur/core.py", line 259, in run_with_sudo_loop
    result = main_thread.get()
  File "/usr/lib/python3.7/multiprocessing/pool.py", line 657, in get
    raise self._value
  File "/usr/lib/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/usr/lib/python3.7/site-packages/pikaur/main.py", line 100, in cli_install_packages
    InstallPackagesCLI()
  File "/usr/lib/python3.7/site-packages/pikaur/install_cli.py", line 160, in __init__
    self.get_all_packages_info()
  File "/usr/lib/python3.7/site-packages/pikaur/install_cli.py", line 212, in get_all_packages_info
    manually_excluded_packages_names=self.manually_excluded_packages_names,
  File "/usr/lib/python3.7/site-packages/pikaur/install_info_fetcher.py", line 55, in __init__
    self.get_all_packages_info()
  File "/usr/lib/python3.7/site-packages/pikaur/install_info_fetcher.py", line 117, in get_all_packages_info
    self.get_aur_deps_info()
  File "/usr/lib/python3.7/site-packages/pikaur/install_info_fetcher.py", line 342, in get_aur_deps_info
    self.aur_deps_relations = find_aur_deps(all_aur_pkgs)
  File "/usr/lib/python3.7/site-packages/pikaur/aur_deps.py", line 244, in find_aur_deps
    results = request.get()
  File "/usr/lib/python3.7/multiprocessing/pool.py", line 657, in get
    raise self._value
  File "/usr/lib/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/usr/lib/python3.7/site-packages/pikaur/aur_deps.py", line 169, in find_missing_deps_for_aur_pkg
    source=PackageSource.REPO
  File "/usr/lib/python3.7/site-packages/pikaur/aur_deps.py", line 21, in check_deps_versions
    for dep_name in deps_pkg_names
  File "/usr/lib/python3.7/site-packages/pikaur/aur_deps.py", line 21, in <listcomp>
    for dep_name in deps_pkg_names
KeyError: ''
actionless commented 6 years ago

it seems what this package https://aur.archlinux.org/packages/python-pikepdf/ have a mailformed depends field (here is a right example of how to put depends field for such case: https://aur.archlinux.org/packages/xfe/)

however other AUR helpers which i tested are ignoring this problem, so i guess i'll implement the same

@Morganamilo do you have any opinion on this?

actionless commented 6 years ago

and extra proof: https://wiki.archlinux.org/index.php/PKGBUILD#depends

Morganamilo commented 6 years ago

Yeah bash arrays are whitespace separated, not comma separated. Still you could use a more user friendly error message.

actionless commented 6 years ago

mb it will be better to have validity check of aur metadata and if's not passes -- propose to edit PKGBUILD (and so read it and replace aur metadata retrieved from aur rpc with the .SRCINFO generated from freshly-edited PKGBUILD, the same as when editing PKGBUILDs before the build)

vasiliev-vb commented 5 years ago

Hello!

Got this call stack while performing 'pikaur -Su'. Seems like one of my installed packages has malformed 'depends' field, but I even can't get information which one.

I think in this case pikaur should report about problem in specific PKGBUILD and ignore the malformed package.

actionless commented 5 years ago

@vasiliev-vb i don't agree it should skip it, but indeed it would make sense to write in which exactly package that problem happened so user can ignore it or use [m]anual package selection

actionless commented 5 years ago

(i'll make a patch in git master later today so you could test it and let you know here)

vasiliev-vb commented 5 years ago

@vasiliev-vb i don't agree it should skip it, but indeed it would make sense to write in which exactly package that problem happened so user can ignore it or use [m]anual package selection

I think that would be nice solution. Waiting for patch.

actionless commented 5 years ago

sorry for the delay, here it is ^

vasiliev-vb commented 5 years ago

Works as intended. Thanks!

actionless commented 5 years ago

thanks for checking!

vasiliev-vb commented 5 years ago

Not sure about closing full issue because the original problem with malformed 'depends' field remains (by 'works as intended' I have meant printing out bad package name).

You can try yourself: at the moment package 'vimb' in the AUR has 'depends' field with commas.

actionless commented 5 years ago

what do you propose instead?

vasiliev-vb commented 5 years ago

Speaking about the original problem (pikaur -S <package_with_malformed_depends>) I don't think that printing python stacktrace in case of error is user-friendly.

I propose two possible solutions:

  1. Print message about malformed 'depends' field in the package and propose to edit PKGBUILD manually or exit.
  2. Try to automatically fix 'depends' field, removing commas. If problem remains after autofix, fallback to first solution.

Speaking about problems during pikaur -Su with already installed bad package, I think solution you proposed

it would make sense to write in which exactly package that problem happened so user can ignore it or use [m]anual package selection

is good, but at the moment pikaur doesn't work that way. After your fix pikaur prints message about malformed dependencies for package during 'pikaur -Su', but still prints out stacktrace, not proposing to ignore bad package or use [m]anual package selection.

actionless commented 5 years ago

but your proposal is just to guess which next syntax error in bash some aur packager gonna make, i see no point re-implementing shellcheck logic in pikaur

while "eating" any exception during aur deps resolution will make application harder to debug

so i still don't see any solution for this which would be either general or following some specific spec

however i am not saying what there is no such solution -- if you'll propose something reasonable (especially as a PR) -- why not (i'll reopen this again as a discussion)

mattia-b89 commented 5 years ago

I can't reproduce this issue, neither with pikaur -S vimb nor pikaur -S ocrmypdf, probably because they fixed the error upstream;
if you can provide an issued-case, it will be helpful for further debugging.

Talking about possible solutions proposed by @vasiliev-vb ,
I think n.2 (try to automatically fix the issue) is to avoid: we can never fix all error-case (today is a white-space, a comma, etc...) or at least it is time-expensive and pikaur users are computer-wise: if it's a problem they should be able to investigate and fix it by themselves!
I think the definitive solution is n.1: do not crash, inform the user about problem, try to give it the most accurately possible information, ask him to edit PKGBUILD, or where the issue is) and keep into account again, pikaur users are computer-wise: if it's a problem the should be able to investigate and fix it by themselves!

actionless commented 5 years ago

for reproducing the issue you could just edit any PKGBUILD and add commas between the items of depends array

as i already answered to this above: you can't know which part of PKGBUILD written wrong unless you do shellcheck on it (and i even not speaking about logical mistakes, which would pass through shellcheck easily)