[pacman-dev] Adds a check to -Qo, so that pacman warns the user if the file does not exits

Message ID 20200617000549.9017-1-pedrocga1.opensource@gmail.com
State Under Review
Headers show
Series [pacman-dev] Adds a check to -Qo, so that pacman warns the user if the file does not exits | expand

Commit Message

Pedro Victor June 17, 2020, 12:05 a.m. UTC
From: Pedro Victor <pedrocga1@gmail.com>

---
 src/pacman/query.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Eli Schwartz June 17, 2020, 12:26 a.m. UTC | #1
On 6/16/20 8:05 PM, Pedro Victor wrote:
> From: Pedro Victor <pedrocga1@gmail.com>
> 
> ---
>  src/pacman/query.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index 513ddb7a..784936b9 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c
> @@ -209,6 +209,11 @@ static int query_fileowner(alpm_list_t *targets)
>  			strcat(rpath + rlen, "/");
>  		}
>  
> +		if (access(rel_path, F_OK) == -1) {
> +			pm_printf(ALPM_LOG_ERROR, _("File does not exist: %s\n"), rel_path);
> +			goto targcleanup;
> +		}
> +

Note that the current behavior is, if a file has been deleted and a
warning is reported by 'pacman -Qkk', pacman -Qo /path/to/file will
still report which package is *supposed* to it.

>  		for(i = packages; i && (!found || is_dir); i = alpm_list_next(i)) {
>  			if(alpm_filelist_contains(alpm_pkg_get_files(i->data), rel_path)) {
>  				print_query_fileowner(rpath, i->data);
>
Pedro Victor June 17, 2020, 1:35 a.m. UTC | #2
That's true, thanks. I sent this patch because I mistyped a path in -Qo and
it only printed "file %mistyped_path is not owned by any package", without
warning me that the file did not exist.
So if this check is put somewhere after the file database query, it will
warn about mistyped paths to nonexistent files, right?

On Tue, Jun 16, 2020, 21:26 Eli Schwartz <eschwartz@archlinux.org> wrote:

> On 6/16/20 8:05 PM, Pedro Victor wrote:
> > From: Pedro Victor <pedrocga1@gmail.com>
> >
> > ---
> >  src/pacman/query.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/src/pacman/query.c b/src/pacman/query.c
> > index 513ddb7a..784936b9 100644
> > --- a/src/pacman/query.c
> > +++ b/src/pacman/query.c
> > @@ -209,6 +209,11 @@ static int query_fileowner(alpm_list_t *targets)
> >                       strcat(rpath + rlen, "/");
> >               }
> >
> > +             if (access(rel_path, F_OK) == -1) {
> > +                     pm_printf(ALPM_LOG_ERROR, _("File does not exist:
> %s\n"), rel_path);
> > +                     goto targcleanup;
> > +             }
> > +
>
> Note that the current behavior is, if a file has been deleted and a
> warning is reported by 'pacman -Qkk', pacman -Qo /path/to/file will
> still report which package is *supposed* to it.
>
> >               for(i = packages; i && (!found || is_dir); i =
> alpm_list_next(i)) {
> >
>  if(alpm_filelist_contains(alpm_pkg_get_files(i->data), rel_path)) {
> >                               print_query_fileowner(rpath, i->data);
> >
>
>
> --
> Eli Schwartz
> Bug Wrangler and Trusted User
>
>
Eli Schwartz June 18, 2020, 2:40 a.m. UTC | #3
On 6/16/20 9:35 PM, Pedro Victor wrote:
> That's true, thanks. I sent this patch because I mistyped a path in -Qo and
> it only printed "file %mistyped_path is not owned by any package", without
> warning me that the file did not exist.
> So if this check is put somewhere after the file database query, it will
> warn about mistyped paths to nonexistent files, right?

Yes, that seems reasonable. Can you submit an updated patch?

While you are at it, commit messages are usually in the imperative mood,
e.g. "add a check [...]" rather than "adds a check". See e.g.

https://git-scm.com/docs/SubmittingPatches#imperative-mood
https://chris.beams.io/posts/git-commit/#imperative

Patch

diff --git a/src/pacman/query.c b/src/pacman/query.c
index 513ddb7a..784936b9 100644
--- a/src/pacman/query.c
+++ b/src/pacman/query.c
@@ -209,6 +209,11 @@  static int query_fileowner(alpm_list_t *targets)
 			strcat(rpath + rlen, "/");
 		}
 
+		if (access(rel_path, F_OK) == -1) {
+			pm_printf(ALPM_LOG_ERROR, _("File does not exist: %s\n"), rel_path);
+			goto targcleanup;
+		}
+
 		for(i = packages; i && (!found || is_dir); i = alpm_list_next(i)) {
 			if(alpm_filelist_contains(alpm_pkg_get_files(i->data), rel_path)) {
 				print_query_fileowner(rpath, i->data);