# [PATCH/RFC] Fix for default pager 29 messages from 2010-06-07 to 2010-06-16. Participants: Dario Rodriguez, Ben Walton, Jeff King, Johannes Sixt, Erik Faye-Lund, Andreas Ericsson, Tor Arntsen, Miles Bader, Ævar Arnfjörð Bjarmason, Junio C Hamano, Brandon Casey, Nazri Ramliy. Thread: https://gitlist.dev/t/24037 ## Dario Rodriguez, 2010-06-07 23:58 Subject: [PATCH/RFC] Fix for default pager Message-ID: <1275955088-32750-1-git-send-email-soft.d4rio@gmail.com> URL: https://gitlist.dev/e/1275955088-32750-1-git-send-email-soft.d4rio%40gmail.com ``` All commands using pager: Default pager was 'less' even when some systems such AIX and other basic or old systems do NOT have 'less' installed. In such case, git just does not display anything in pager-enabled functionalities such as 'git log' or 'git show', exiting with status 0. With this patch, git will not use DEFAULT_PAGER macro anymore, instead, git will look for 'less' and 'more' in the most common paths. If there is no pager, returns NULL as if it's 'cat'. Obviously, the commit message needs to be rewritten :p Signed-off-by: Dario Rodriguez --- pager.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 73 insertions(+), 7 deletions(-) diff --git a/pager.c b/pager.c index dac358f..3cfbd73 100644 --- a/pager.c +++ b/pager.c @@ -2,10 +2,19 @@ #include "run-command.h" #include "sigchain.h" -#ifndef DEFAULT_PAGER -#define DEFAULT_PAGER "less" +#ifndef GIT_PAGER_ENVIRONMENT +#define GIT_PAGER_ENVIRONMENT "GIT_PAGER" #endif +#ifndef GIT_PGR_LOOKUP_DBG_ENVIRONMENT +#define GIT_PGR_LOOKUP_DBG_ENVIRONMENT "GIT_PAGER_LOOKUP_DEBUG" +#endif + +#ifndef PAGER_ENVIRONMENT +#define PAGER_ENVIRONMENT "PAGER" +#endif + + /* * This is split up from the rest of git so that we can do * something different on Windows. @@ -48,23 +57,80 @@ static void wait_for_pager_signal(int signo) raise(signo); } -const char *git_pager(int stdout_is_tty) +static int is_executable(const char *name) +{ + struct stat st; + + if (stat(name, &st) || + !S_ISREG(st.st_mode)) + return 0; + +#ifdef WIN32 +{ /* cannot trust the executable bit, peek into the file instead */ + char buf[3] = { 0 }; + int n; + int fd = open(name, O_RDONLY); + st.st_mode &= ~S_IXUSR; + if (fd >= 0) { + n = read(fd, buf, 2); + if (n == 2) + /* DOS executables start with "MZ" */ + if (!strcmp(buf, "#!") || !strcmp(buf, "MZ")) + st.st_mode |= S_IXUSR; + close(fd); + } +} +#endif + return st.st_mode & S_IXUSR; +} + +const char *git_pager(int stdout_is_tty) { + static const char *pager_bins[] = + { "less", "more", NULL }; + static const char *common_binary_paths[] = + { "/bin/","/usr/bin/","/usr/local/bin/",NULL }; + const char *pager; + char **p1,**p2,*pager_heap; + char exe_name[255]; + int pager_lkp_dbg=0; if (!stdout_is_tty) return NULL; - pager = getenv("GIT_PAGER"); + if(getenv(GIT_PGR_LOOKUP_DBG_ENVIRONMENT)) + pager_lkp_dbg=1; + + memset( exe_name,0,255 ); + pager = getenv(GIT_PAGER_ENVIRONMENT); if (!pager) { if (!pager_program) git_config(git_default_config, NULL); pager = pager_program; } if (!pager) - pager = getenv("PAGER"); - if (!pager) - pager = DEFAULT_PAGER; + pager = getenv(PAGER_ENVIRONMENT); + if (!pager) { + + for (p1=(char**)pager_bins; (*p1)&&(!pager) + ;p1++) + for (p2=(char**)common_binary_paths; (*p2)&&(!pager) + ;p2++) { + sprintf( exe_name,"%s%s", + *p2,*p1 ); + if (is_executable(exe_name)) { + pager_heap = (char*) malloc(sizeof(exe_name)); + strcpy(pager_heap,exe_name); + pager = pager_heap; + } + } + + if (pager_lkp_dbg) + fprintf(stderr, "Debug: Lookup for existent pagers, got [%s]\n", + (pager==NULL)?"(null)":pager); + + } else if (!*pager || !strcmp(pager, "cat")) pager = NULL; -- 1.7.1.245.g7c42e.dirty ``` ## Ben Walton, 2010-06-08 00:04 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <1275955270-sup-2380@pinkfloyd.chass.utoronto.ca> URL: https://gitlist.dev/e/1275955270-sup-2380%40pinkfloyd.chass.utoronto.ca In-Reply-To: <1275955088-32750-1-git-send-email-soft.d4rio@gmail.com> ``` Excerpts from Dario Rodriguez's message of Mon Jun 07 19:58:08 -0400 2010: > Default pager was 'less' even when some systems such AIX and other > basic or old systems do NOT have 'less' installed. In such case, git > just does not display anything in pager-enabled functionalities such > as 'git log' or 'git show', exiting with status 0. Why not set a sensible DEFAULT_PAGER value for the system in your config.mak file instead? Just curious. Thanks -Ben -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 ``` ## Dario Rodriguez, 2010-06-08 02:14 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/AANLkTinydWk3GqGDww8FS7pmW16jAVazRkmT_GsRMIhy%40mail.gmail.com In-Reply-To: <1275955270-sup-2380@pinkfloyd.chass.utoronto.ca> ``` On Mon, Jun 7, 2010 at 9:04 PM, Ben Walton wrote: > > Why not set a sensible DEFAULT_PAGER value for the system in your > config.mak file instead? > > Just curious. > I was thinking about it before coding the patch, and found some consequences. First of all, the most important thing I should understand is that most users will install git from binary, precompiled packages instead of the good download and compile. So this is actually a good reason to not do it that way (config.mak)... Some users may download the compiled binary while it's actually calling it's default pager. But this is not the only reason. Let me give you an example: We develop (I'm actually working at Accenture) using several machines. When we need some tool, we compile it in our first machine, and install it for an specific user. In some other environments we cannot compile things (testing environments) so we transfer (FTP) those binary files. It's just another case, and the default pager could cause problems here (in fact, i experienced such problems). Another case (or something as the first one): If you installed less (via your package system), then git, and then you happen to uninstall less... should GIT be uninstalled in absence of it's default pager? I think GIT must auto-detect it's pager based in what is on the system at runtime, don't you think so? ``` ## Jeff King, 2010-06-08 05:29 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <20100608052929.GA15156@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100608052929.GA15156%40coredump.intra.peff.net In-Reply-To: <1275955088-32750-1-git-send-email-soft.d4rio@gmail.com> ``` On Mon, Jun 07, 2010 at 08:58:08PM -0300, Dario Rodriguez wrote: > Default pager was 'less' even when some systems such AIX and other basic > or old systems do NOT have 'less' installed. In such case, git just > does not display anything in pager-enabled functionalities such as 'git log' > or 'git show', exiting with status 0. > > With this patch, git will not use DEFAULT_PAGER macro anymore, instead, > git will look for 'less' and 'more' in the most common paths. > If there is no pager, returns NULL as if it's 'cat'. Run-time pager detection seems like a reasonable goal, I guess, but... > -const char *git_pager(int stdout_is_tty) > +static int is_executable(const char *name) > +{ > + struct stat st; > + > + if (stat(name, &st) || > + !S_ISREG(st.st_mode)) > + return 0; > + > +#ifdef WIN32 > +{ /* cannot trust the executable bit, peek into the file instead */ > + char buf[3] = { 0 }; > + int n; > + int fd = open(name, O_RDONLY); > + st.st_mode &= ~S_IXUSR; > + if (fd >= 0) { > + n = read(fd, buf, 2); > + if (n == 2) > + /* DOS executables start with "MZ" */ > + if (!strcmp(buf, "#!") || !strcmp(buf, "MZ")) > + st.st_mode |= S_IXUSR; > + close(fd); > + } > +} > +#endif > + return st.st_mode & S_IXUSR; > +} > + > +const char *git_pager(int stdout_is_tty) > { > + static const char *pager_bins[] = > + { "less", "more", NULL }; > + static const char *common_binary_paths[] = > + { "/bin/","/usr/bin/","/usr/local/bin/",NULL }; ...must we really add code with such ugliness as magic PATHs and DOS magic numbers? Right now we fall back to just exec-ing "less". Could we instead just try to exec "less", if that fails then "more", and then finally "cat"? That would have almost the same effect and would be much simpler, wouldn't it? The exceptions I can think of are: - we would actually run "cat" in the final case, instead of optimizing it out. - "git var GIT_PAGER" wouldn't handle this automatically -Peff ``` ## Jeff King, 2010-06-08 05:35 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <20100608053507.GB15156@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100608053507.GB15156%40coredump.intra.peff.net In-Reply-To: ``` On Mon, Jun 07, 2010 at 11:14:58PM -0300, Dario Rodriguez wrote: > On Mon, Jun 7, 2010 at 9:04 PM, Ben Walton wrote: > > > > Why not set a sensible DEFAULT_PAGER value for the system in your > > config.mak file instead? > > > > Just curious. > > > > I was thinking about it before coding the patch, and found some > consequences. First of all, the most important thing I should > understand is that most users will install git from binary, > precompiled packages instead of the good download and compile. So this > is actually a good reason to not do it that way (config.mak)... Some > users may download the compiled binary while it's actually calling > it's default pager. If you are downloading a binary, the package compiler should do one of two things: 1. indicate a package dependency on 'less' 2. set DEFAULT_PAGER to 'more' (or whatever is appropriate for your system) Yes, auto-detection means we can more flexibly "upgrade" to less when the package suddenly appears. But if you really care about your pager, why not just set $PAGER? The most important thing is that users who _don't_ care don't see something broken, but the rules above already cover that with current git. > But this is not the only reason. Let me give you an example: We > develop (I'm actually working at Accenture) using several machines. > When we need some tool, we compile it in our first machine, and > install it for an specific user. In some other environments we cannot > compile things (testing environments) so we transfer (FTP) those > binary files. It's just another case, and the default pager could > cause problems here (in fact, i experienced such problems). Then set DEFAULT_PAGER to 'more' (or 'cat' for that matter), and use $PAGER on machines that are more capable. -Peff ``` ## Johannes Sixt, 2010-06-08 06:12 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <4C0DDF53.5090805@viscovery.net> URL: https://gitlist.dev/e/4C0DDF53.5090805%40viscovery.net In-Reply-To: <20100608052929.GA15156@coredump.intra.peff.net> ``` Am 6/8/2010 7:29, schrieb Jeff King: > On Mon, Jun 07, 2010 at 08:58:08PM -0300, Dario Rodriguez wrote: >> +#ifdef WIN32 >> +{ /* cannot trust the executable bit, peek into the file instead */ >> + char buf[3] = { 0 }; >> + int n; >> + int fd = open(name, O_RDONLY); >> + st.st_mode &= ~S_IXUSR; >> + if (fd >= 0) { >> + n = read(fd, buf, 2); >> + if (n == 2) >> + /* DOS executables start with "MZ" */ >> + if (!strcmp(buf, "#!") || !strcmp(buf, "MZ")) >> + st.st_mode |= S_IXUSR; >> + close(fd); >> + } >> +} >> +#endif >> + return st.st_mode & S_IXUSR; >> +} >> + >> +const char *git_pager(int stdout_is_tty) >> { >> + static const char *pager_bins[] = >> + { "less", "more", NULL }; >> + static const char *common_binary_paths[] = >> + { "/bin/","/usr/bin/","/usr/local/bin/",NULL }; > > ...must we really add code with such ugliness as magic PATHs and DOS > magic numbers? Yes and no. Coding DOS magic numbers is bad because you could wrapped a real pager in a shell script (even on Windows). These days, start_command() is able to report ENOENT if it cannot run a program because it does not exist. However, right in the case of git_pager() this capability is disabled because it sets this magic preexec_cb that waits for data in the child process before it execs the real pager. If we could get rid of preexec_cb, we could make this work much more pleasantly. pager_preexec waits for data on stdin; Linus added it to work around a faulty 'less'. Do we still need it? -- Hannes ``` ## Dario Rodriguez, 2010-06-08 12:24 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/AANLkTinxcrIV2TM966EkOC_crR0bHdNllEIdibz4gGjd%40mail.gmail.com In-Reply-To: <20100608052929.GA15156@coredump.intra.peff.net> ``` On Tue, Jun 8, 2010 at 2:29 AM, Jeff King wrote: > On Mon, Jun 07, 2010 at 08:58:08PM -0300, Dario Rodriguez wrote: > >> Default pager was 'less' even when some systems such AIX and other basic >> or old systems do NOT have 'less' installed. In such case, git just >> does not display anything in pager-enabled functionalities such as 'git log' >> or 'git show', exiting with status 0. >> >> With this patch, git will not use DEFAULT_PAGER macro anymore, instead, >> git will look for 'less' and 'more' in the most common paths. >> If there is no pager, returns NULL as if it's 'cat'. > > Run-time pager detection seems like a reasonable goal, I guess, but... > >> -const char *git_pager(int stdout_is_tty) >> +static int is_executable(const char *name) >> +{ >> +     struct stat st; >> + >> +     if (stat(name, &st) || >> +         !S_ISREG(st.st_mode)) >> +             return 0; >> + >> +#ifdef WIN32 >> +{    /* cannot trust the executable bit, peek into the file instead */ >> +     char buf[3] = { 0 }; >> +     int n; >> +     int fd = open(name, O_RDONLY); >> +     st.st_mode &= ~S_IXUSR; >> +     if (fd >= 0) { >> +             n = read(fd, buf, 2); >> +             if (n == 2) >> +                     /* DOS executables start with "MZ" */ >> +                     if (!strcmp(buf, "#!") || !strcmp(buf, "MZ")) >> +                             st.st_mode |= S_IXUSR; >> +             close(fd); >> +     } >> +} >> +#endif >> +     return st.st_mode & S_IXUSR; >> +} >> + >> +const char *git_pager(int stdout_is_tty) >>  { >> +     static const char *pager_bins[] = >> +             { "less", "more", NULL }; >> +     static const char *common_binary_paths[] = >> +             { "/bin/","/usr/bin/","/usr/local/bin/",NULL }; > > ...must we really add code with such ugliness as magic PATHs and DOS > magic numbers? > I copied the function 'is_executable' from 'help.c' so we already have such code... :p > Right now we fall back to just exec-ing "less". Could we instead just > try to exec "less", if that fails then "more", and then finally "cat"? > is such a good idea but right now, 'git_pager' is not exec-ing, it's just setting up a pager. If you set-up the pager based on wich one fails in it's execution, you must avoid usage of this function, since it will always return 'less' (or 'more...). What I posted is transparent to any other function; 'git_pager' will be called returning an existent, working pager, so the flow is the same, however I like your proposal too, and should be considered. > That would have almost the same effect and would be much simpler, > wouldn't it? The exceptions I can think of are: > >  - we would actually run "cat" in the final case, instead of optimizing >    it out. > Actually pager is being set to NULL if it's 'cat'... what's git doing with a NULL pager? >  - "git var GIT_PAGER" wouldn't handle this automatically > > -Peff > Blessing, Dario ``` ## Dario Rodriguez, 2010-06-08 13:49 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/AANLkTilvvpy4TBQF6g8boQL87FRB7kFDrVfYiHvOv6xu%40mail.gmail.com In-Reply-To: <20100608053507.GB15156@coredump.intra.peff.net> ``` On Tue, Jun 8, 2010 at 2:35 AM, Jeff King wrote: > If you are downloading a binary, the package compiler should do one of > two things: > >  1. indicate a package dependency on 'less' > but... git will be uninstalled if you happen to uninstall less. >  2. set DEFAULT_PAGER to 'more' (or whatever is appropriate for your >     system) > and I know it's a very nice fallback default, but you still depending on the pager, and will display nothing and return 0 if 'more' fails to execute. Something needs to be changed... if the sane way is to keep default pager (almost setting 'more') instead of auto-detection... then we must say something to the user when the pager fails to execute. (*) Actually 'git' display nothing... and returns 0. > Yes, auto-detection means we can more flexibly "upgrade" to less when > the package suddenly appears. But if you really care about your pager, > why not just set $PAGER? > good point, I agree. But again, back to (*), we must correct something... the other way is that if pager fails to execute, we cannot simply return 0. I could submit a patch for this, may be is a little bit better (or simpler). ``` ## Erik Faye-Lund, 2010-06-08 14:04 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/AANLkTikoHwupz8ZycLKko_gaZcsAnnXjlWIU4qpt-_9T%40mail.gmail.com In-Reply-To: ``` On Tue, Jun 8, 2010 at 2:24 PM, Dario Rodriguez wrote: > On Tue, Jun 8, 2010 at 2:29 AM, Jeff King wrote: >> On Mon, Jun 07, 2010 at 08:58:08PM -0300, Dario Rodriguez wrote: >> >>> Default pager was 'less' even when some systems such AIX and other basic >>> or old systems do NOT have 'less' installed. In such case, git just >>> does not display anything in pager-enabled functionalities such as 'git log' >>> or 'git show', exiting with status 0. >>> >>> With this patch, git will not use DEFAULT_PAGER macro anymore, instead, >>> git will look for 'less' and 'more' in the most common paths. >>> If there is no pager, returns NULL as if it's 'cat'. >> >> Run-time pager detection seems like a reasonable goal, I guess, but... >> >>> -const char *git_pager(int stdout_is_tty) >>> +static int is_executable(const char *name) >>> +{ >>> +     struct stat st; >>> + >>> +     if (stat(name, &st) || >>> +         !S_ISREG(st.st_mode)) >>> +             return 0; >>> + >>> +#ifdef WIN32 >>> +{    /* cannot trust the executable bit, peek into the file instead */ >>> +     char buf[3] = { 0 }; >>> +     int n; >>> +     int fd = open(name, O_RDONLY); >>> +     st.st_mode &= ~S_IXUSR; >>> +     if (fd >= 0) { >>> +             n = read(fd, buf, 2); >>> +             if (n == 2) >>> +                     /* DOS executables start with "MZ" */ >>> +                     if (!strcmp(buf, "#!") || !strcmp(buf, "MZ")) >>> +                             st.st_mode |= S_IXUSR; >>> +             close(fd); >>> +     } >>> +} >>> +#endif >>> +     return st.st_mode & S_IXUSR; >>> +} >>> + >>> +const char *git_pager(int stdout_is_tty) >>>  { >>> +     static const char *pager_bins[] = >>> +             { "less", "more", NULL }; >>> +     static const char *common_binary_paths[] = >>> +             { "/bin/","/usr/bin/","/usr/local/bin/",NULL }; >> >> ...must we really add code with such ugliness as magic PATHs and DOS >> magic numbers? >> > > I copied the function 'is_executable' from 'help.c' so we already have > such code... :p > How about just un-staticifying the version in help.c instead of duplicating it, then? That way we'd have half the ugliness for this purpose... -- Erik "kusma" Faye-Lund ``` ## Johannes Sixt, 2010-06-08 14:17 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <4C0E5103.7030501@viscovery.net> URL: https://gitlist.dev/e/4C0E5103.7030501%40viscovery.net In-Reply-To: ``` Am 6/8/2010 15:49, schrieb Dario Rodriguez: > we must correct > something... the other way is that if pager fails to execute, we > cannot simply return 0. But we do not return 0: $ GIT_PAGER=/is/not/there git log $ echo $? 141 That's SIGPIPE, just as I would expect. And with this change diff --git a/pager.c b/pager.c index dac358f..86519cc 100644 --- a/pager.c +++ b/pager.c @@ -89,9 +89,6 @@ void setup_pager(void) static const char *env[] = { "LESS=FRSX", NULL }; pager_process.env = env; } -#ifndef WIN32 - pager_process.preexec_cb = pager_preexec; -#endif if (start_command(&pager_process)) return; I get: $ GIT_PAGER=/is/not/there ./git log -1 --oneline error: cannot run /is/not/there: No such file or directory 1a16cee merge-recursive: demonstrate an incorrect conflict with submodule -- Hannes ``` ## Dario Rodriguez, 2010-06-08 14:39 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/AANLkTilWg8hw5j20o-xGsVO-q_OeSmtKEKAO6O416qvH%40mail.gmail.com In-Reply-To: <4C0E5103.7030501@viscovery.net> ``` On Tue, Jun 8, 2010 at 11:17 AM, Johannes Sixt wrote: > Am 6/8/2010 15:49, schrieb Dario Rodriguez: >> we must correct >> something... the other way is that if pager fails to execute, we >> cannot simply return 0. > > But we do not return 0: > >  $ GIT_PAGER=/is/not/there git log >  $ echo $? >  141 > > That's SIGPIPE, just as I would expect. > As I said in the original thread... $ PAGER=/nothing/here ../git log $ echo $? 0 $ GIT_PAGER=/nothing/here ../git log $ echo $? 0 That's on AIX 5.2 > And with this change > > diff --git a/pager.c b/pager.c > index dac358f..86519cc 100644 > --- a/pager.c > +++ b/pager.c > @@ -89,9 +89,6 @@ void setup_pager(void) >                static const char *env[] = { "LESS=FRSX", NULL }; >                pager_process.env = env; >        } > -#ifndef WIN32 > -       pager_process.preexec_cb = pager_preexec; > -#endif >        if (start_command(&pager_process)) >        return; > > > I get: > >  $ GIT_PAGER=/is/not/there ./git log -1 --oneline >  error: cannot run /is/not/there: No such file or directory >  1a16cee merge-recursive: demonstrate an incorrect conflict with submodule > Curious... I patched it on AIX and I get: $ GIT_PAGER=/nothing/here ../git log error: cannot run /nothing/here: No such file or directory commit 3274a12f940680612e3bfd3d022a0eab460c0f1f Author: usuario ####### <#######@Maquina01.(none)> Date: Thu Jun 3 20:02:23 2010 +0200 OtherCom commit acf110f7c878a37e4a5af8499134df28da0e8ab3 Author: usuario ####### <#######@Maquina01.(none)> Date: Thu Jun 3 20:01:37 2010 +0200 inicial However, the patch must delete the pager_preexec definition too... but I wonder, do somebody still need it? btw: I still think 'more' is much more sane fallback default than 'less'... look (with your patch applied): $ ../git log error: cannot run less: No such file or directory commit 3274a12f940680612e3bfd3d022a0eab460c0f1f Author: ####### <#######@Maquina01.(none)> Date: Thu Jun 3 20:02:23 2010 +0200 OtherCom commit acf110f7c878a37e4a5af8499134df28da0e8ab3 Author: ####### <#######@Maquina01.(none)> Date: Thu Jun 3 20:01:37 2010 +0200 inicial ``` ## Johannes Sixt, 2010-06-08 15:56 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <4C0E6810.3070301@viscovery.net> URL: https://gitlist.dev/e/4C0E6810.3070301%40viscovery.net In-Reply-To: ``` Am 08.06.2010 16:39, schrieb Dario Rodriguez: > On Tue, Jun 8, 2010 at 11:17 AM, Johannes Sixt wrote: >> $ GIT_PAGER=/is/not/there git log >> $ echo $? >> 141 >> >> That's SIGPIPE, just as I would expect. > > As I said in the original thread... > > $ PAGER=/nothing/here ../git log > $ echo $? > 0 That's no surprise with your toy repository: git-log has run to completion (without overrunning the pipe buffer) before the pager process that it forked can even execute its first instruction. > btw: I still think 'more' is much more sane fallback default than > 'less'... look (with your patch applied): > > $ ../git log > error: cannot run less: No such file or directory > commit 3274a12f940680612e3bfd3d022a0eab460c0f1f > Author: #######<#######@Maquina01.(none)> > Date: Thu Jun 3 20:02:23 2010 +0200 > > OtherCom > > commit acf110f7c878a37e4a5af8499134df28da0e8ab3 > Author: #######<#######@Maquina01.(none)> > Date: Thu Jun 3 20:01:37 2010 +0200 > > inicial > How is this an argument for 'more'? (Just asking; I don't see your point.) -- Hannes ``` ## Dario Rodriguez, 2010-06-08 17:28 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/AANLkTinZSuXJEXzpvEavYNLSyqUlx8qzWlrbtIH6q6fx%40mail.gmail.com In-Reply-To: <4C0E6810.3070301@viscovery.net> ``` On Tue, Jun 8, 2010 at 12:56 PM, Johannes Sixt wrote: > Am 08.06.2010 16:39, schrieb Dario Rodriguez: >> >> On Tue, Jun 8, 2010 at 11:17 AM, Johannes Sixt >>  wrote: >>> >>>  $ GIT_PAGER=/is/not/there git log >>>  $ echo $? >>>  141 >>> >>> That's SIGPIPE, just as I would expect. >> >> As I said in the original thread... >> >> $ PAGER=/nothing/here ../git log >> $ echo $? >> 0 > > That's no surprise with your toy repository: git-log has run to completion > (without overrunning the pipe buffer) before the pager process that it > forked can even execute its first instruction. > I cannot understand what's the point... I'm running git without installing it, but why do you say "toy repository"?... >> btw: I still think 'more' is much more sane fallback default than >> 'less'... look (with your patch applied): >> >> $ ../git log >> error: cannot run less: No such file or directory >> commit 3274a12f940680612e3bfd3d022a0eab460c0f1f >> Author: #######<#######@Maquina01.(none)> >> Date:   Thu Jun 3 20:02:23 2010 +0200 >> >>     OtherCom >> >> commit acf110f7c878a37e4a5af8499134df28da0e8ab3 >> Author: #######<#######@Maquina01.(none)> >> Date:   Thu Jun 3 20:01:37 2010 +0200 >> >>     inicial >> > > How is this an argument for 'more'? (Just asking; I don't see your point.) > No problem, let me explain: 'more' is older and standard while (correct me if not true) 'less' is not in POSIX:2008. I work in a lot of Unix-like systems and I found 'more' as a standard, while less is just sometimes installed... my error running 'less' is such an argument for 'more' because: $ PAGER=more ../git log commit fccd640197e34dbff72954924ed5c76c42a4aac7 Author: ###### <######@Maquina01.(none)> Date: Tue Jun 8 19:23:23 2010 +0200 commit stdin: END 'more' is not a problem... and as a POSIX standard it's almost never a problem. ``` ## Johannes Sixt, 2010-06-08 18:59 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <4C0E932B.3010702@viscovery.net> URL: https://gitlist.dev/e/4C0E932B.3010702%40viscovery.net In-Reply-To: ``` Am 08.06.2010 19:28, schrieb Dario Rodriguez: > On Tue, Jun 8, 2010 at 12:56 PM, Johannes Sixt wrote: >> Am 08.06.2010 16:39, schrieb Dario Rodriguez: >>> >>> On Tue, Jun 8, 2010 at 11:17 AM, Johannes Sixt >>> wrote: >>>> >>>> $ GIT_PAGER=/is/not/there git log >>>> $ echo $? >>>> 141 >>>> >>>> That's SIGPIPE, just as I would expect. >>> >>> As I said in the original thread... >>> >>> $ PAGER=/nothing/here ../git log >>> $ echo $? >>> 0 >> >> That's no surprise with your toy repository: git-log has run to completion >> (without overrunning the pipe buffer) before the pager process that it >> forked can even execute its first instruction. >> > > I cannot understand what's the point... I'm running git without > installing it, but why do you say "toy repository"?... Your repository has only 2 commits and its git log output is less than 1kB, i.e., sufficiently small to fit in a pipe's buffer. git log calls start_command to fork() the pager. The OS's scheduler does not run the newly forked process immediately; rather, git log goes on with its own business, writing output to the pipe that connects to the pager. Because your repository is so small, git log never has to wait that the pager drains the pipe. git log finally reaches exit(0). At this time, an atexit() handler (wait_for_pager()) finally calls finish_command() to wait for the pager. This is the first time that the forked child process can run. Only now it turns out that the pager cannot be run. The child process closes the pipe and exits with an error, but it is too late: wait_for_pager() drops the error return code of finish_command() to the floor. The parent process (git log) can complete with the exit code that it was given earlier, 0. Repeat your experiment with ./git log in git.git itself to see the difference. -- Hannes ``` ## Dario Rodriguez, 2010-06-08 20:44 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/AANLkTinB_SBilMOfgnHtDrQS-NBOLF4yY5NaP7ZvN9rK%40mail.gmail.com In-Reply-To: <4C0E932B.3010702@viscovery.net> ``` On Tue, Jun 8, 2010 at 3:59 PM, Johannes Sixt wrote: > Your repository has only 2 commits and its git log output is less than 1kB, > i.e., sufficiently small to fit in a pipe's buffer. > > git log calls start_command to fork() the pager. The OS's scheduler does not > run the newly forked process immediately; rather, git log goes on with its > own business, writing output to the pipe that connects to the pager. Because > your repository is so small, git log never has to wait that the pager drains > the pipe. git log finally reaches exit(0). At this time, an atexit() handler > (wait_for_pager()) finally calls finish_command() to wait for the pager. > > This is the first time that the forked child process can run. Only now it > turns out that the pager cannot be run. The child process closes the pipe > and exits with an error, but it is too late: wait_for_pager() drops the > error return code of finish_command() to the floor. The parent process (git > log) can complete with the exit code that it was given earlier, 0. > > Repeat your experiment with ./git log in git.git itself to see the > difference. > > -- Hannes > Capisco & touché, with much more than 1k of info, git show ends with a "Broken Pipe"... seems hard to detect for little, recently started projects since I added more than 60k of scripts and I need to do 'git show' to understand that the problem is a broken pipe. Now, let me think about it... do we need the pager_preexec function? I mean... it works fine without it, and the function is there because of a faulty 'less'. My problem is obvioulsly solved by adding PAGER=more in my default environment, but I think this could be a litle bit embarrassing for a new user, mostly in environments such this AIX :P Cheers, Dario ``` ## Andreas Ericsson, 2010-06-08 21:33 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <4C0EB741.9020905@op5.se> URL: https://gitlist.dev/e/4C0EB741.9020905%40op5.se In-Reply-To: ``` On 06/08/2010 10:44 PM, Dario Rodriguez wrote: > On Tue, Jun 8, 2010 at 3:59 PM, Johannes Sixt wrote: >> Your repository has only 2 commits and its git log output is less than 1kB, >> i.e., sufficiently small to fit in a pipe's buffer. >> >> git log calls start_command to fork() the pager. The OS's scheduler does not >> run the newly forked process immediately; rather, git log goes on with its >> own business, writing output to the pipe that connects to the pager. Because >> your repository is so small, git log never has to wait that the pager drains >> the pipe. git log finally reaches exit(0). At this time, an atexit() handler >> (wait_for_pager()) finally calls finish_command() to wait for the pager. >> >> This is the first time that the forked child process can run. Only now it >> turns out that the pager cannot be run. The child process closes the pipe >> and exits with an error, but it is too late: wait_for_pager() drops the >> error return code of finish_command() to the floor. The parent process (git >> log) can complete with the exit code that it was given earlier, 0. >> >> Repeat your experiment with ./git log in git.git itself to see the >> difference. >> >> -- Hannes >> > > Capisco& touché, with much more than 1k of info, git show ends with a > "Broken Pipe"... seems hard to detect for little, recently started > projects since I added more than 60k of scripts and I need to do 'git > show' to understand that the problem is a broken pipe. > > Now, let me think about it... do we need the pager_preexec function? I > mean... it works fine without it, and the function is there because of > a faulty 'less'. > > My problem is obvioulsly solved by adding PAGER=more in my default > environment, but I think this could be a litle bit embarrassing for a > new user, mostly in environments such this AIX :P > Catering to AIX by default seems stupid beyond belief. AIX users today are, without fail, accustomed to having to tweak more or less everything to make the system run smoothly with modern applications (where "modern" is a generous term, including everything that's been written in the last 10 or so years). -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 Considering the successes of the wars on alcohol, poverty, drugs and terror, I think we should give some serious thought to declaring war on peace. ``` ## Tor Arntsen, 2010-06-09 09:08 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/AANLkTinAO5empFix9W_rbtU3Vv4O73OsJBtA1stb66DS%40mail.gmail.com In-Reply-To: <4C0EB741.9020905@op5.se> ``` On Tue, Jun 8, 2010 at 23:33, Andreas Ericsson wrote: > Catering to AIX by default seems stupid beyond belief. AIX users today > are, without fail, accustomed to having to tweak more or less everything > to make the system run smoothly with modern applications (where "modern" > is a generous term, including everything that's been written in the last > 10 or so years). AIX doesn't come with 'less', because it doesn't really need one. 'more' on AIX can page backwards/forwards in piped data (unlike 'more' on Linux etc.), thus negating the most common need for installing 'less' elsewhere. (The other big feature of less, that it by default clears the screen just as you had found what you wanted to read and were about to apply it to the command line before 'poof' - gone) is so annoying to me that I routinely sets PAGER=more before using Git. If it's true that Git should demand 'less' to be installed before being usable out of the box.. well, that's just plain silly. -Tor ``` ## Miles Bader, 2010-06-09 09:29 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/buo4ohcsgdu.fsf%40dhlpc061.dev.necel.com In-Reply-To: ``` Tor Arntsen writes: > AIX doesn't come with 'less', because it doesn't really need one. > 'more' on AIX can page backwards/forwards in piped data (unlike 'more' > on Linux etc.), thus negating the most common need for installing > 'less' elsewhere. Less has about a zillion features that more [traditionally] doesn't have, and even some of the more esoteric ones can be quite useful for git. For instance you can determine in a great deal of detail exactly how/when/if it clears the screen/shows a prompt/waits for user input before exiting, etc. -Miles -- (\(\ (^.^) (")") *This is the cute bunny virus, please copy this into your sig so it can spread. ``` ## Ævar Arnfjörð Bjarmason, 2010-06-09 18:57 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/AANLkTikqnMGviOOrTWDUA82u3pwfWnunzj8KxQkRcLsm%40mail.gmail.com In-Reply-To: <4C0EB741.9020905@op5.se> ``` On Tue, Jun 8, 2010 at 21:33, Andreas Ericsson wrote: > Catering to AIX by default seems stupid beyond belief. AIX users today > are, without fail, accustomed to having to tweak more or less everything > to make the system run smoothly with modern applications (where "modern" > is a generous term, including everything that's been written in the last > 10 or so years). BSD users are also accustomed to that to a smaller degree. But as long as they keep submitting portability patches to keep their ports up to date, I don't see why those patches shouldn't be accepted. If they're otherwise fit for inclusion that is. ``` ## Jeff King, 2010-06-10 08:29 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <20100610082916.GA5559@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100610082916.GA5559%40coredump.intra.peff.net In-Reply-To: ``` On Wed, Jun 09, 2010 at 11:08:49AM +0200, Tor Arntsen wrote: > If it's true that Git should demand 'less' to be installed before > being usable out of the box.. well, that's just plain silly. It depends on how you define "out of the box". The person compiling it just needs to set DEFAULT_PAGER appropriately for their system. "less" is a sane choice for most modern systems. But we can make it even easier on AIX people with: diff --git a/Makefile b/Makefile index 34b7dd5..6ad0aca 100644 --- a/Makefile +++ b/Makefile @@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD) HAVE_PATHS_H = YesPlease endif ifeq ($(uname_S),AIX) + DEFAULT_PAGER = more NO_STRCASESTR=YesPlease NO_MEMMEM = YesPlease NO_MKDTEMP = YesPlease That won't do automagic run-time detection if you have "less" installed, but given your claim that AIX's "more" actually doesn't suck, it's probably a good default. People who care can set their PAGER environment variable. -Peff ``` ## Tor Arntsen, 2010-06-10 08:48 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/AANLkTinLt3p0q-q5oDFk5CWzdhqQ2lwkWuvpdPzKZvYe%40mail.gmail.com In-Reply-To: <20100610082916.GA5559@coredump.intra.peff.net> ``` On Thu, Jun 10, 2010 at 10:29, Jeff King wrote: > On Wed, Jun 09, 2010 at 11:08:49AM +0200, Tor Arntsen wrote: > >> If it's true that Git should demand 'less' to be installed before >> being usable out of the box.. well, that's just plain silly. > > It depends on how you define "out of the box". The person compiling it > just needs to set DEFAULT_PAGER appropriately for their system. "less" > is a sane choice for most modern systems. But we can make it even easier > on AIX people with: > > diff --git a/Makefile b/Makefile > index 34b7dd5..6ad0aca 100644 > --- a/Makefile > +++ b/Makefile > @@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD) >        HAVE_PATHS_H = YesPlease >  endif >  ifeq ($(uname_S),AIX) > +       DEFAULT_PAGER = more >        NO_STRCASESTR=YesPlease >        NO_MEMMEM = YesPlease >        NO_MKDTEMP = YesPlease That looks good to me. > That won't do automagic run-time detection if you have "less" installed, > but given your claim that AIX's "more" actually doesn't suck, it's > probably a good default. It does the paging just as 'less', yes. I can page 'git log' or any other output. backwards/forwards as I wish. I'm not aware of any 'more' other than the AIX one that does this. Even the archaic (pre-AIX5) versions did. The AIX 'more' also have several more in-buffer options compared to e.g. Linux 'more'. It sits somewhere between traditional 'more' and 'less', feature-wise. (The AIX 6 version even does the, to me, annoying less-style erase-screen, unless turned off by an option or environment variable.) > People who care can set their PAGER environment > variable. -Tor ``` ## Jeff King, 2010-06-10 08:59 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <20100610085952.GA8269@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100610085952.GA8269%40coredump.intra.peff.net In-Reply-To: ``` On Thu, Jun 10, 2010 at 10:48:31AM +0200, Tor Arntsen wrote: > That looks good to me. OK, here it is with a commit message. Other systems might want the same, I guess (Solaris, IRIX?). I'm cc'ing Brandon, who might have some input. Note that this is completely untested by me, as all of my AIX boxen have gone away in the past few months (yay!). -- >8 -- Subject: [PATCH] Makefile: default pager on AIX to "more" AIX doesn't ship with "less" by default, and their "more" is more featureful than average, so the latter is a more sensible choice. People who really want less can set the compile-time option themselves, or users can set $PAGER. Signed-off-by: Jeff King --- Makefile | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/Makefile b/Makefile index 34b7dd5..6ad0aca 100644 --- a/Makefile +++ b/Makefile @@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD) HAVE_PATHS_H = YesPlease endif ifeq ($(uname_S),AIX) + DEFAULT_PAGER = more NO_STRCASESTR=YesPlease NO_MEMMEM = YesPlease NO_MKDTEMP = YesPlease -- 1.7.1.514.g71ed8 ``` ## Tor Arntsen, 2010-06-10 09:24 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <4C10AF43.5050708@spacetec.no> URL: https://gitlist.dev/e/4C10AF43.5050708%40spacetec.no In-Reply-To: <20100610085952.GA8269@coredump.intra.peff.net> ``` On 06/10/10 10:59, Jeff King wrote: > On Thu, Jun 10, 2010 at 10:48:31AM +0200, Tor Arntsen wrote: > >> That looks good to me. > > OK, here it is with a commit message. Other systems might want the same, > I guess (Solaris, IRIX?). I'm cc'ing Brandon, who might have some input. > > Note that this is completely untested by me, as all of my AIX boxen have > gone away in the past few months (yay!). I did a re-build and test. Everything works fine. Tested-by: Tor Arntsen -Tor -- >8 -- Subject: [PATCH] Makefile: default pager on AIX to "more" AIX doesn't ship with "less" by default, and their "more" is more featureful than average, so the latter is a more sensible choice. People who really want less can set the compile-time option themselves, or users can set $PAGER. Signed-off-by: Jeff King --- Makefile | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/Makefile b/Makefile index 34b7dd5..6ad0aca 100644 --- a/Makefile +++ b/Makefile @@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD) HAVE_PATHS_H = YesPlease endif ifeq ($(uname_S),AIX) + DEFAULT_PAGER = more NO_STRCASESTR=YesPlease NO_MEMMEM = YesPlease NO_MKDTEMP = YesPlease -- 1.7.1.514.g71ed8 ``` ## Dario Rodriguez, 2010-06-10 11:31 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/AANLkTimI0DAHWzAPYRcui3h7KDvf1NFQBx0p4LdlYHTJ%40mail.gmail.com In-Reply-To: <4C10AF43.5050708@spacetec.no> ``` On Thu, Jun 10, 2010 at 6:24 AM, Tor Arntsen wrote: > On 06/10/10 10:59, Jeff King wrote: >> On Thu, Jun 10, 2010 at 10:48:31AM +0200, Tor Arntsen wrote: >> >>> That looks good to me. >> >> OK, here it is with a commit message. Other systems might want the same, >> I guess (Solaris, IRIX?). I'm cc'ing Brandon, who might have some input. >> >> Note that this is completely untested by me, as all of my AIX boxen have >> gone away in the past few months (yay!). > > I did a re-build and test. Everything works fine. > > Tested-by: Tor Arntsen > > -Tor > > -- >8 -- > Subject: [PATCH] Makefile: default pager on AIX to "more" > > AIX doesn't ship with "less" by default, and their "more" is > more featureful than average, so the latter is a more > sensible choice.  People who really want less can set the > compile-time option themselves, or users can set $PAGER. > > Signed-off-by: Jeff King > --- >  Makefile |    1 + >  1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/Makefile b/Makefile > index 34b7dd5..6ad0aca 100644 > --- a/Makefile > +++ b/Makefile > @@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD) >        HAVE_PATHS_H = YesPlease >  endif >  ifeq ($(uname_S),AIX) > +       DEFAULT_PAGER = more >        NO_STRCASESTR=YesPlease >        NO_MEMMEM = YesPlease >        NO_MKDTEMP = YesPlease > -- 1.7.1.514.g71ed8 > It's ok for me, I will test it on AIX today, if I've some time. Cheers, Dario ``` ## Junio C Hamano, 2010-06-10 14:55 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <7vy6enlyy6.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vy6enlyy6.fsf%40alter.siamese.dyndns.org In-Reply-To: <20100610085952.GA8269@coredump.intra.peff.net> ``` Jeff King writes: > Subject: [PATCH] Makefile: default pager on AIX to "more" > > AIX doesn't ship with "less" by default, and their "more" is > more featureful than average, so the latter is a more > sensible choice. People who really want less can set the > compile-time option themselves, or users can set $PAGER. > > Signed-off-by: Jeff King > --- > Makefile | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/Makefile b/Makefile > index 34b7dd5..6ad0aca 100644 > --- a/Makefile > +++ b/Makefile > @@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD) > HAVE_PATHS_H = YesPlease > endif > ifeq ($(uname_S),AIX) > + DEFAULT_PAGER = more > NO_STRCASESTR=YesPlease > NO_MEMMEM = YesPlease > NO_MKDTEMP = YesPlease A very sensible approach. Thanks. ``` ## Brandon Casey, 2010-06-15 16:11 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/gJV0lM_e77LzoiHR7moWdAApSZ7yI38lZ-w8kZwc97unWqtBc94nfg%40cipher.nrlssc.navy.mil In-Reply-To: <20100610085952.GA8269@coredump.intra.peff.net> ``` On 06/10/2010 03:59 AM, Jeff King wrote: > On Thu, Jun 10, 2010 at 10:48:31AM +0200, Tor Arntsen wrote: > >> That looks good to me. > > OK, here it is with a commit message. Other systems might want the same, > I guess (Solaris, IRIX?). I'm cc'ing Brandon, who might have some input. Yes, I currently set DEFAULT_PAGER to 'more' in my config.mak file on both of these platforms. The 'more' on IRIX is decent (it can go backwards), but the 'more' on Solaris sucks. I've seen 'less' on some newer versions of Solaris. Is it a standard component yet? So, I think it's appropriate to set DEFAULT_PAGER on IRIX. There can't be many users anyway. It's probably appropriate to set it on Solaris too, if 'less' is not a commonly installed component on modern systems. I wonder how surprised existing git users will be, for those on Solaris platforms that have 'less' installed, when Solaris's crappy 'more' becomes their pager. Actually, there is a 'more' in /usr/xpg4/bin that is much better, but it is not being used when DEFAULT_PAGER is set to 'more'. Junio created the SANE_TOOL_PATH hack to add this additional path to the search path, but it is only implemented in git-sh-setup, so it only has effect for git scripts. Maybe it should be added to setup_path(). But, I also think it would be nice if git fell back to the 'cat' behavior when it fails to spawn the pager, because the following error is not very informative: casey@ # git log sh: less: not found Broken Pipe -brandon > Note that this is completely untested by me, as all of my AIX boxen have > gone away in the past few months (yay!). > > -- >8 -- > Subject: [PATCH] Makefile: default pager on AIX to "more" > > AIX doesn't ship with "less" by default, and their "more" is > more featureful than average, so the latter is a more > sensible choice. People who really want less can set the > compile-time option themselves, or users can set $PAGER. > > Signed-off-by: Jeff King > --- > Makefile | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/Makefile b/Makefile > index 34b7dd5..6ad0aca 100644 > --- a/Makefile > +++ b/Makefile > @@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD) > HAVE_PATHS_H = YesPlease > endif > ifeq ($(uname_S),AIX) > + DEFAULT_PAGER = more > NO_STRCASESTR=YesPlease > NO_MEMMEM = YesPlease > NO_MKDTEMP = YesPlease ``` ## Tor Arntsen, 2010-06-15 16:32 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <4C17AB38.3060705@spacetec.no> URL: https://gitlist.dev/e/4C17AB38.3060705%40spacetec.no In-Reply-To: ``` On 06/15/10 18:11, Brandon Casey wrote: > Yes, I currently set DEFAULT_PAGER to 'more' in my config.mak file > on both of these platforms. The 'more' on IRIX is decent (it can go > backwards), but the 'more' on Solaris sucks. I've seen 'less' on some > newer versions of Solaris. Is it a standard component yet? 'less' appears to be standard on Solaris 10 (5.10). My older 5.8 box is offline so I can't check that one. AFAIK the difference between Solaris 8 and Solaris 10 is pretty big so I suspect there's no 'less' there but I can't say for certain. I could try to fire up that old box and check though. -Tor ``` ## Nazri Ramliy, 2010-06-16 01:34 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: URL: https://gitlist.dev/e/AANLkTinb7eRrRRYxxRd3BbNfBwRYAVFTNJ7z8oklNvIs%40mail.gmail.com In-Reply-To: ``` On Wed, Jun 16, 2010 at 12:11 AM, Brandon Casey wrote: > But, I also think it would be nice if git fell back to the 'cat' > behavior when it fails to spawn the pager, because the following error > is not very informative: Falling back to 'cat' is nice except when you are on a remote machine with very bad latency. nazri ``` ## Jeff King, 2010-06-16 06:28 Subject: Re: [PATCH/RFC] Fix for default pager Message-ID: <20100616062814.GA13481@sigill.intra.peff.net> URL: https://gitlist.dev/e/20100616062814.GA13481%40sigill.intra.peff.net In-Reply-To: ``` On Tue, Jun 15, 2010 at 11:11:35AM -0500, Brandon Casey wrote: > So, I think it's appropriate to set DEFAULT_PAGER on IRIX. There can't > be many users anyway. It's probably appropriate to set it on Solaris > too, if 'less' is not a commonly installed component on modern systems. > I wonder how surprised existing git users will be, for those on Solaris > platforms that have 'less' installed, when Solaris's crappy 'more' > becomes their pager. I'm a little worried about that, too. On the other hand, wouldn't people who actually care about less have set PAGER already, to use it for things like "man"? > But, I also think it would be nice if git fell back to the 'cat' > behavior when it fails to spawn the pager, because the following error > is not very informative: > > casey@ # git log > sh: less: not found > Broken Pipe The pager command is executed by the shell these days. Perhaps we should simply set DEFAULT_PAGER on these platforms to "less || more || cat", which seems to work from my simple tests. If that is too hack-ish (e.g., we really care about "does less exist", not "did it fail"), we can do a more invasive patch (or even provide a "git-pager" shell script helper to do a more thorough job). -Peff ```