Bug 45625

Created attachment 56602 [details] [review]
[PATCH] cnf: Use local variables in the shell handler


Since this is a bash specific feature, we can use local variables to
not pollute the user's environment.
---
 contrib/command-not-found/PackageKit.sh.in |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Created attachment 56603 [details] [review]
[PATCH] cnf: Quote the arguments passed to pk-command-not-found


"$@" is a special variable in bash that expands to all the positional
parameters properly quoted. Essentially it means that any parameters
with spaces will be passed correctly as a single argument.
---
 contrib/command-not-found/PackageKit.sh.in |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Created attachment 56604 [details] [review]
[PATCH] cnf: Inform the user of the missing command from the shell


The shell missing command handler is the correct place to inform the
user of the missing command, and not from the tool doing the
replacement lookup.

This also has the benefit that the message can exactly match what bash
itself would have sent. It uses the actual interpreter name from $0
and the existing bash translations. Here's an example showing the
handler vs bare bash:

[dan@buster ~] su -
Password:
[root@buster ~]# LANG=de_DE
[root@buster ~]# blah
-bash: blah: Kommando nicht gefunden.
[root@buster ~]# unset -f command_not_found_handle
[root@buster ~]# blah
-bash: blah: Kommando nicht gefunden.
---
 contrib/command-not-found/PackageKit.sh.in       |    7 +++++--
 contrib/command-not-found/pk-command-not-found.c |    5 -----
 2 files changed, 5 insertions(+), 7 deletions(-)

Created attachment 56605 [details] [review]
[PATCH] cnf: Only search for packages when shell is interactive


If we're inside a script, we can't ask a user to install a package, so
just return with an error. Example:

$ banshee
-bash: banshee: command not found
Install package 'banshee' to provide command 'banshee'? [N/y]
$ bash -c '. /etc/profile.d/PackageKit.sh; banshee'
bash: banshee: command not found
---
 contrib/command-not-found/PackageKit.sh.in |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Created attachment 56606 [details] [review]
[PATCH] cnf: Return errors immediately instead of storing return values


Instead of storing return values and logic in local variables, just
return immediately if we can't search the database. If we can run the
search command, let its exit code be the return code for the function.
---
 contrib/command-not-found/PackageKit.sh.in |   19 +++++--------------
 1 files changed, 5 insertions(+), 14 deletions(-)

Created attachment 56607 [details] [review]
[PATCH] cnf: Ask the user before searching the package database


Often the user has simply mistyped the command when the shell can't
find it and would rather not wait while PackageKit roots around
looking for the missing package. Ask the user to continue before
searching the package database. This defaults to no and will assume no
after a 5 second timeout. If the user doesn't want to be bothered, they
can just hit return to get back to their prompt.
---
 contrib/command-not-found/PackageKit.sh.in |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

I've started getting spurious attempts of package installation after upgrading to GNOME 3.12 (unnoficial packages from http://copr.fedoraproject.org/coprs/rhughes/f20-gnome-3-12/) and getting a PackageKit update coupled with it.

I get installation prompts even for non-interactive shells, which is very annoying. It seems Dan Nicholson's patches improve the situation significantly, so applying them would be very helpful from an UX perspective.

Comment on attachment 56602 [details] [review]
[PATCH] cnf: Use local variables in the shell handler

Review of attachment 56602 [details] [review]:
-----------------------------------------------------------------

Looks good, pushed; thanks.

Comment on attachment 56603 [details] [review]
[PATCH] cnf: Quote the arguments passed to pk-command-not-found

Review of attachment 56603 [details] [review]:
-----------------------------------------------------------------

Looks good, pushed; thanks.

Comment on attachment 56605 [details] [review]
[PATCH] cnf: Only search for packages when shell is interactive

Review of attachment 56605 [details] [review]:
-----------------------------------------------------------------

Looks good, pushed; thanks.

I've pushed 1,2,4 -- but I'm not so sure about the rest. I'll review more when I get home. Thanks.

We moved the upstream bugtracker to GitHub a long time ago. If this issue still affects you please re-create the issue here: https://github.com/hughsie/PackageKit/issues
 
Sorry for the impersonal message, and fingers crossed your issue no longer happens. Thanks.