diff mbox

[pacman-dev] src/pacman/query.c: do not exit -Qo with error if file does not exist

Message ID 20171112230031.4671-1-iff@escondida.tk
State Superseded, archived
Delegated to: Andrew Gregory
Headers show

Commit Message

Ivy Foster Nov. 12, 2017, 11 p.m. UTC
From: Ivy Foster <iff@escondida.tk>

Query operations act on the local db, not the filesystem. Also, a
valid use case for -Qo is to discover what package owns a deleted file
so it can be reinstalled.

Closes FS#55856.

Signed-off-by: Ivy Foster <iff@escondida.tk>
---
I've opted to simply remove the relevant error messages here, since
there's a good chance that the user is aware that the file is missing
if they're querying a missing file.

As a side note, this removes the sole usage of the translated error
"failed to find '%s' in PATH: %s\n". I have not deleted this string
from every .po file, but that option now exists.

 src/pacman/query.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

Comments

Eli Schwartz Nov. 13, 2017, midnight UTC | #1
On 11/12/2017 06:00 PM, iff@escondida.tk wrote:
> As a side note, this removes the sole usage of the translated error
> "failed to find '%s' in PATH: %s\n". I have not deleted this string
> from every .po file, but that option now exists.

That's okay, strings are never touched except right before a release,
when the strings are frozen, added/changed/deleted strings updated in
the .po files, and everything is shipped off to transifex for the
translation team to look at.
Andrew Gregory Nov. 13, 2017, 12:39 a.m. UTC | #2
On 11/12/17 at 05:00pm, iff@escondida.tk wrote:
> From: Ivy Foster <iff@escondida.tk>
> 
> Query operations act on the local db, not the filesystem. Also, a
> valid use case for -Qo is to discover what package owns a deleted file
> so it can be reinstalled.
> 
> Closes FS#55856.
> 
> Signed-off-by: Ivy Foster <iff@escondida.tk>
> ---
> I've opted to simply remove the relevant error messages here, since
> there's a good chance that the user is aware that the file is missing
> if they're querying a missing file.
> 
> As a side note, this removes the sole usage of the translated error
> "failed to find '%s' in PATH: %s\n". I have not deleted this string
> from every .po file, but that option now exists.
> 
>  src/pacman/query.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index 024d3e21..64c42f19 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c
> @@ -171,19 +171,9 @@ static int query_fileowner(alpm_list_t *targets)
>  			filename[len--] = '\0';
>  		}
>  
> -		if(lstat(filename, &buf) == -1) {
> -			/* if it is not a path but a program name, then check in PATH */
> -			if(strchr(filename, '/') == NULL) {
> -				if(search_path(&filename, &buf) == -1) {
> -					pm_printf(ALPM_LOG_ERROR, _("failed to find '%s' in PATH: %s\n"),
> -							filename, strerror(errno));
> -					goto targcleanup;
> -				}
> -			} else {
> -				pm_printf(ALPM_LOG_ERROR, _("failed to read file '%s': %s\n"),
> -						filename, strerror(errno));
> -				goto targcleanup;
> -			}
> +		/* if it is not a path but a program name, then check in PATH */
> +		if((lstat(filename, &buf) == -1) && (strchr(filename, '/') == NULL)) {
> +			search_path(&filename, &buf);
>  		}
>  
>  		if(!lrealpath(filename, rpath)) {
> -- 
> 2.15.0

This won't work for missing directories, which will be missing the
trailing /, or files whose parent directory no longer exists, which
will fail the call to lrealpath.

apg
Ivy Foster Nov. 14, 2017, 12:35 a.m. UTC | #3
On 12 Nov 2017, at  7:39 pm -0500, Andrew Gregory wrote:
> On 11/12/17 at 05:00pm, iff@escondida.tk wrote:
> > From: Ivy Foster <iff@escondida.tk>
> > diff --git a/src/pacman/query.c b/src/pacman/query.c
> > index 024d3e21..64c42f19 100644
> > --- a/src/pacman/query.c
> > +++ b/src/pacman/query.c
> > @@ -171,19 +171,9 @@ static int query_fileowner(alpm_list_t *targets)
> >                     filename[len--] = '\0';
> >             }
> >
> > -           if(lstat(filename, &buf) == -1) {
> > -                   /* if it is not a path but a program name, then check in PATH */
> > -                   if(strchr(filename, '/') == NULL) {
> > -                           if(search_path(&filename, &buf) == -1) {
> > -                                   pm_printf(ALPM_LOG_ERROR, _("failed to find '%s' in PATH: %s\n"),
> > -                                                   filename, strerror(errno));
> > -                                   goto targcleanup;
> > -                           }
> > -                   } else {
> > -                           pm_printf(ALPM_LOG_ERROR, _("failed to read file '%s': %s\n"),
> > -                                           filename, strerror(errno));
> > -                           goto targcleanup;
> > -                   }
> > +           /* if it is not a path but a program name, then check in PATH */
> > +           if((lstat(filename, &buf) == -1) && (strchr(filename, '/') == NULL)) {
> > +                   search_path(&filename, &buf);
> >             }
> >
> >             if(!lrealpath(filename, rpath)) {
> > --
> > 2.15.0

> This won't work for missing directories, which will be missing the
> trailing /, or files whose parent directory no longer exists, which
> will fail the call to lrealpath.

Ah, quite right. That didn't even occur to me. As always, thanks for
the feedback! I'll submit an improved patch tomorrow (Mon) or Tues.

Ivy
Ivy Foster Nov. 14, 2017, 12:35 a.m. UTC | #4
On 12 Nov 2017, at  7:00 pm -0500, Eli Schwartz wrote:
> On 11/12/2017 06:00 PM, iff@escondida.tk wrote:
> > As a side note, this removes the sole usage of the translated error
> > "failed to find '%s' in PATH: %s\n". I have not deleted this string
> > from every .po file, but that option now exists.

> That's okay, strings are never touched except right before a release,
> when the strings are frozen, added/changed/deleted strings updated in
> the .po files, and everything is shipped off to transifex for the
> translation team to look at.

That's really good to know, thanks!

Ivy
diff mbox

Patch

diff --git a/src/pacman/query.c b/src/pacman/query.c
index 024d3e21..64c42f19 100644
--- a/src/pacman/query.c
+++ b/src/pacman/query.c
@@ -171,19 +171,9 @@  static int query_fileowner(alpm_list_t *targets)
 			filename[len--] = '\0';
 		}
 
-		if(lstat(filename, &buf) == -1) {
-			/* if it is not a path but a program name, then check in PATH */
-			if(strchr(filename, '/') == NULL) {
-				if(search_path(&filename, &buf) == -1) {
-					pm_printf(ALPM_LOG_ERROR, _("failed to find '%s' in PATH: %s\n"),
-							filename, strerror(errno));
-					goto targcleanup;
-				}
-			} else {
-				pm_printf(ALPM_LOG_ERROR, _("failed to read file '%s': %s\n"),
-						filename, strerror(errno));
-				goto targcleanup;
-			}
+		/* if it is not a path but a program name, then check in PATH */
+		if((lstat(filename, &buf) == -1) && (strchr(filename, '/') == NULL)) {
+			search_path(&filename, &buf);
 		}
 
 		if(!lrealpath(filename, rpath)) {