perf uses less at the default pager, but the linux-tools packages have no dependency for it

This bug is missing log files that will aid in diagnosing the problem. From a terminal window please run:

apport-collect 833101

and then change the status of the bug to 'Confirmed'.

If, due to the nature of the issue you have encountered, you are unable to run this command, please add a comment stating that fact and change the bug status to 'Confirmed'.

This change has been made by an automated script, maintained by the Ubuntu Kernel Team.

This issue only affects lightweight images which (coincidentally) don't have apport.

I believe this problem should be self-explanatory, even without additional logs.

Still present in linux-linaro-tools-3.0.0-1006

Avik, can you check if this is still an issue with our 11.11 images? Thanks.

Yes, this issue is still present in 11.11 nano image.

Thanks for confirming the issue. Will let the bug assigned to you then.

Avik, any update on this one? If not yet fixed, please move it to 12.01. Thanks.

No update as such. Moving it to 12.01.

The attached patch seems to fix the issue.

Patch not merged yet, moving it to 12.02

New patch accepted: http://git.linaro.org/gitweb?p=people/ynk/linux-linaro-tracking.git;a=commit;h=49561d58a14351d136518f25bd686f9a6ca41b69

Granted the code in perf/util/pager.c looks borked in that "else if" after "if (!pager)" doesn't look like it will ever get executed. However, if you just set your PAGER env variable to either "more" or "cat" should do the job.

Brad, does the patch in comment #11 look good here?

@Avik,

If it's as simple as setting the PAGER environment variable, why do I want to apply the patch?

If you think it's the right thing to do however, I'd suggest trying to push it upstream.

PAGER is for customisation. A package which might not work if PAGER is not set is a broken package.

However, the proposed change (http://git.linaro.org/gitweb?p=people/ynk/linux-linaro-tracking.git;a=commit;h=49561d58a14351d136518f25bd686f9a6ca41b69) has some problems. In particular, it will mess up PATH in the environment, because strtok() causes the string it is used on to be modified. Since perf runs other subprocesses, this may lead to problems.

Why don't we just use /usr/bin/pager as the default? This is the whole point of the Debian alternatives system.

So:

If PAGER is set, try use its value. Don't bother to search: it's up to the user to specify sensible preferences.

Otherwise, if /usr/bin/pager exists, use that.

Otherwise, fall back to the legacy behaviour (i.e., try to run "less"). This should work just fine for the non-Debian folks.

Cheers
---Dave

@Dave,

I could live with something like that.

Does this patch look OK?

diff --git a/tools/perf/util/pager.c b/tools/perf/util/pager.c
index 1915de2..d6f23de 100644
--- a/tools/perf/util/pager.c
+++ b/tools/perf/util/pager.c
@@ -57,8 +57,12 @@ void setup_pager(void)
        }
        if (!pager)
                pager = getenv("PAGER");
- if (!pager)
- pager = "less";
+ if (!pager) {
+ if (!access("/usr/bin/pager", X_OK))
+ pager = "/usr/bin/pager";
+ else
+ pager = "less";
+ }
        else if (!*pager || !strcmp(pager, "cat"))
                return;

On Thu, May 10, 2012 at 12:30:52PM -0000, Avik Sil wrote:
> Does this patch look OK?
>
> diff --git a/tools/perf/util/pager.c b/tools/perf/util/pager.c
> index 1915de2..d6f23de 100644
> --- a/tools/perf/util/pager.c
> +++ b/tools/perf/util/pager.c
> @@ -57,8 +57,12 @@ void setup_pager(void)
> }
> if (!pager)
> pager = getenv("PAGER");
> - if (!pager)
> - pager = "less";
> + if (!pager) {
> + if (!access("/usr/bin/pager", X_OK))
> + pager = "/usr/bin/pager";
> + else
> + pager = "less";

To conform to the existing code style, you might want to write the code
as follows:

+ if (!pager) {
+ if (!access("/usr/bin/pager", X_OK))
+ pager = "/usr/bin/pager";
+ }
+ if (!pager)
+ pager = "less";

Functionally, the code looks OK to me either way.

Cheers
---Dave

Packaging bug, won't fix in the kernel team.