pkg icon indicating copy to clipboard operation
pkg copied to clipboard

Silent exit instead of error message on DB access problem

Open nuclight opened this issue 1 year ago • 3 comments

If there is a problem with .sqlite in /var/db/pkg, then e.g. pkg upgrade pkg just silently exits without any error message (just $?=1), even with -dddd. Tracing problem with gdb found the cause:

pkgdb_check_access():
714                 retval = faccessat(dbdirfd, dbpath, R_OK|W_OK, AT_EACCESS);

And there is no sufficient checks for errno later.

How to reproduce, most easy way:

# chflags sunlnk,schg /var/db/pkg/repo-FreeBSD.sqlite
# pkg upgrade pkg

Solution is to add perror() (or whatever better style for pkg) later in this function in else branch for all unknown errno's.

nuclight avatar Jan 14 '24 17:01 nuclight

I think this behavior is intentional(?), as pkgdb_check_access() only returns error codes, and doesn't print standard error messages. This issue also exists in some other commands as well (see https://github.com/freebsd/pkg/issues/2211).

A simple "fix" can be done by adding a warning message (using warn(3) or perror(3)) whenever faccessat(3) fails.

root@freebsd:/home/rilysh/pkg # ./src/pkg install abc
pkg: faccessat: Operation not permitted
root@freebsd:/home/rilysh/pkg # echo $?
1

I'm not sure, if it's required in most cases, as you can always check for returned errors. Although I agree, having a clear error message is much better than just relying on returned values.

rilysh avatar Jan 18 '24 18:01 rilysh

Well, this can't be intentional, because it creates UX from hell for the user. And even if the user is not just user but sysadmin, then bare faccessat: Operation not permitted is stil not enough - yopu want to know which file caused the problem. So that it can be fixed without consulting pkg sources. Because user do not call routines to check for error codes, user calls e.g. pkg upgrade.

So. The only debatable thing here is where to insert reporting to user, and by which function - this is what pkg maintainers know better than bugreporter.

nuclight avatar Jan 20 '24 23:01 nuclight

Adding it in pkgdb_check_access() function is just fine. However, before any PR, it will be better to get some attention from maintainers with this issue.

rilysh avatar Jan 21 '24 04:01 rilysh